Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 8 pull requests #109692

Merged
merged 25 commits into from
Mar 28, 2023
Merged

Rollup of 8 pull requests #109692

merged 25 commits into from
Mar 28, 2023

Conversation

Noratrieb
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

lcnr and others added 25 commits March 22, 2023 11:37
This updates object to 0.30 and fixes a bug where the symbol table
would be omitted for archives where there are object files yet none
that export any symbol. This bug could lead to linker errors for crates
like rustc_std_workspace_core which don't contain any code of their own
but exist solely for their dependencies. This is likely the cause of
the linker issues I was experiencing on Webassembly. It has been shown
to cause issues on other platforms too.

cc rust-lang/ar_archive_writer#5
…ze>`

A successful advance is now signalled by returning `0` and other values now represent the remaining number
of steps that couldn't be advanced as opposed to the amount of steps that have been advanced during a partial advance_by.

This simplifies adapters a bit, replacing some `match`/`if` with arithmetic. Whether this is beneficial overall depends
on whether `advance_by` is mostly used as a building-block for other iterator methods and adapters or whether
we also see uses by users where `Result` might be more useful.
socket ancillary data implementation for FreeBSD (from 13 and above).

introducing new build config as well.
…tmcm

Change advance(_back)_by to return the remainder instead of the number of processed elements

When advance_by can't advance the iterator by the number of requested elements it now returns the amount by which it couldn't be advanced instead of the amount by which it did.

This simplifies adapters like chain, flatten or cycle because the remainder doesn't have to be calculated as the difference between requested steps and completed steps anymore.

Additionally switching from `Result<(), usize>` to `Result<(), NonZeroUsize>` reduces the size of the result and makes converting from/to a usize representing the number of remaining steps cheap.
stop special-casing `'static` in evaluation

fixes rust-lang#102360

I have no idea whether this actually removed all places where `'static` matters. Without canonicalization it's very easy to accidentally rely on `'static` again. Blocked on changing the `order_dependent_trait_objects` future-compat lint to a hard error

r? `@nikomatsakis`
Use Rayon's TLV directly

This accesses Rayon's `TLV` thread local directly avoiding wrapper functions. This makes rustc work with rust-lang/rustc-rayon#10.

r? `@cuviper`
…ions, r=cjgillot

Erase impl regions when checking for impossible to eagerly monomorphize items

We were inserting `ReErased` for method substs, but not for impl substs, leading to the call for `subst_and_check_impossible_predicates` being a bit weaker than it should be (since it ignores predicates that need substitution -- incl early-bound regions).

Fixes rust-lang#109297
…d, r=jackh726

Correctly substitute GAT's type used in `normalize_param_env` in `check_type_bounds`

Given:

```rust
trait Foo {
    type Assoc<T>: PartialEq<Self::Assoc<i32>>;
}

impl Foo for () {
    type Assoc<T> = Wrapper<T>;
}

struct Wrapper<T>(T);

impl<T> PartialEq<Wrapper<i32>> for Wrapper<T> { }
```

We add an additional predicate in the `normalize_param_env` in `check_type_bounds` that is used to normalize the GAT's bounds to check them in the impl. Problematically, though, that predicate is constructed to be `for<^0> <() as Foo>::Assoc<^0> => Wrapper<T>`, instead of `for<^0> <() as Foo>::Assoc<^0> => Wrapper<^0>`.

That means `Self::Assoc<i32>` in the bounds that we're checking normalizes to `Wrapper<T>`, instead of `Wrapper<i32>`, and so the bound `Self::Assoc<T>: PartialEq<Self::Assoc<i32>>` normalizes to `Wrapper<T>: PartialEq<Wrapper<T>>`, which does not hold.

Fixes this by properly substituting the RHS of that normalizes predicate that we add to the `normalize_param_env`. That means the bound is properly normalized to `Wrapper<T>: PartialEq<Wrapper<i32>>`, which *does* hold.

---

The second commit in this PR just cleans up some substs stuff and some naming.

r? `@jackh726` cc rust-lang#87900
…=Mark-Simulacrum

Update ar_archive_writer to 0.1.3

This updates object to 0.30 and fixes a bug where the symbol table would be omitted for archives where there are object files yet none that export any symbol. This bug could lead to linker errors for crates like rustc_std_workspace_core which don't contain any code of their own but exist solely for their dependencies. This is likely the cause of the linker issues I was experiencing on Webassembly. It has been shown to cause issues on other platforms too.

cc rust-lang/ar_archive_writer#5
remove obsolete `givens` from regionck

Revives rust-lang#107376. The only change is the last commit (rust-lang@2a3177a) which should fix the regression.

Fixes rust-lang#106567

r? `@lcnr`
@Noratrieb
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Mar 28, 2023

📌 Commit 60ce19d has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 28, 2023
@bors
Copy link
Contributor

bors commented Mar 28, 2023

⌛ Testing commit 60ce19d with merge f9749c071bd4005257fd0e21efdc53ecf24e7c39...

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2023
@Noratrieb
Copy link
Member Author

@bors retry spurious

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2023
@bors
Copy link
Contributor

bors commented Mar 28, 2023

⌛ Testing commit 60ce19d with merge 478cbb4...

@bors
Copy link
Contributor

bors commented Mar 28, 2023

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing 478cbb4 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Mar 28, 2023

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing 478cbb4 to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (478cbb4): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.9% [0.2%, 9.2%] 31
Regressions ❌
(secondary)
1.7% [0.2%, 10.9%] 25
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.1% [-1.6%, -0.5%] 5
All ❌✅ (primary) 0.9% [0.2%, 9.2%] 31

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.8% [2.3%, 4.9%] 4
Improvements ✅
(primary)
-0.1% [-0.1%, -0.1%] 1
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) -0.1% [-0.1%, -0.1%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
4.4% [1.1%, 8.4%] 5
Regressions ❌
(secondary)
4.5% [2.2%, 9.5%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 4.4% [1.1%, 8.4%] 5

@rustbot rustbot added the perf-regression Performance regression. label Mar 28, 2023
@Noratrieb
Copy link
Member Author

oof

@Noratrieb
Copy link
Member Author

I would suspect #102472 as the regression is somewhere with obligation processing. But the unrolled builds don't seem to be done yet?

@Noratrieb
Copy link
Member Author

cc @lcnr as I suspect your PR to be there regression as the regression is in evaluate_obligation
I could be wrong or course

@lcnr
Copy link
Contributor

lcnr commented Mar 30, 2023

expect that to be the case, isn't there a way to check the perf of single prs in a rollup? cc @rust-lang/wg-compiler-performance

@Kobzol
Copy link
Contributor

Kobzol commented Mar 30, 2023

There is, but the comment with unrolled builds hasn't been posted on this PR for some reason. @Mark-Simulacrum is something in the logs?

@lqd
Copy link
Member

lqd commented Mar 30, 2023

Nils and I asked on zulip yesterday about the missing unrolled builds to help investigate this.

@lqd
Copy link
Member

lqd commented Mar 30, 2023

isn't there a way to check the perf of single prs in a rollup?

we can check perf on a revert and see if the regression disappears, so I've done that in #109780

@lqd
Copy link
Member

lqd commented Mar 31, 2023

It seems like it's one source of the regression here, #109780 (comment). We could land the revert to buy time, or @lcnr do you think there's an easy way to mitigate the regression ?
It doesn't seem to recover everything in this rollup's results though, so there may be more than one source.

@lcnr
Copy link
Contributor

lcnr commented Mar 31, 2023

We could land the revert to buy time, or @lcnr do you think there's an easy way to mitigate the regression ?

have some ideas but won't have a lot of time other the next few weeks, I would be alright with reverting for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.