-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Split out the various responsibilities of rustc_metadata::Lazy
#97291
Split out the various responsibilities of rustc_metadata::Lazy
#97291
Conversation
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
791765c
to
30323cc
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 30323cc40d0af9bd158a823d2a8818b26f8d269f with merge 48a331d69e217e5bec16e805669ffe759820ba8d... |
☀️ Try build successful - checks-actions |
Queued 48a331d69e217e5bec16e805669ffe759820ba8d with parent 32c8c5d, future comparison URL. |
Finished benchmarking commit (48a331d69e217e5bec16e805669ffe759820ba8d): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Footnotes |
Slight regression, but I think it's mild enough. Combined with the clarity wins and unblocking of future work, I think we just land. |
I'm gonna try to slap some |
30323cc
to
2b5e592
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 2b5e592 with merge a9dc794e2e93c93fb228ae43a6ca17eb4192ff7d... |
☀️ Try build successful - checks-actions |
Queued a9dc794e2e93c93fb228ae43a6ca17eb4192ff7d with parent ee160f2, future comparison URL. |
Finished benchmarking commit (a9dc794e2e93c93fb228ae43a6ca17eb4192ff7d): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Footnotes |
Okay, well fixing the incorrect size_hint implementation and adding some inlines did the trick I guess. |
@bors r+ |
📌 Commit 2b5e592 has been approved by |
☀️ Test successful - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Finished benchmarking commit (ee9726c): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Lazy<T>
actually acts like three different types -- a pointer in the crate metadata to a single value, a pointer to a list/array of values, and an indexable pointer of a list of values (a table).We currently overload
Lazy<T>
to work differently thanLazy<[T]>
and the same forLazy<Table<I, T>>
. All is well with some helper adapter traits such asLazyQueryDecodable
andEncodeContentsForLazy
.Well, changes in #97287 that make
Lazy
work with the now invariant lifetime'tcx
make these adapters fall apart because of coherence reasons. So we split out these three types and rework some of the helper traits so it's both 1. more clear to understand, and 2. compatible with the changes later in that PR.Split out from #97287 so it can be reviewed separately, since this PR stands on its own.