-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add a query cache for dense local DefIds #69303
Conversation
e4a34ca
to
948880c
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 948880c95b0e5c8501c490bf18b5f06ea557acc7 with merge 26c75c5e3b892d085b4815677fd07f9f84583c4c... |
☀️ Try build successful - checks-azure |
Queued 26c75c5e3b892d085b4815677fd07f9f84583c4c with parent de362d8, future comparison URL. |
Finished benchmarking try commit 26c75c5e3b892d085b4815677fd07f9f84583c4c, comparison URL. |
Looks like perf was regressed? |
948880c
to
9bac16a
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 9bac16a with merge bbed581aa2e500028b8ff65f2f35aa8a64c00948... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors try |
⌛ Trying commit 40567e6 with merge 62c144a0ca9aff8af0b2c73b944dd21cca2f0007... |
} | ||
|
||
#[derive(Default)] | ||
pub struct DefaultCache; | ||
pub struct DefaultCache<D>(PhantomData<D>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this generic parameter D
do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoids compiler errors for the pub type LocalDenseDefIdCacheSelector<V>
alias.
☀️ Try build successful - checks-azure |
Queued 62c144a0ca9aff8af0b2c73b944dd21cca2f0007 with parent 97eda01, future comparison URL. |
@Zoxc can you resolve conflicts? we can get this reviewed |
This comment has been minimized.
This comment has been minimized.
@eddyb Is this not what you had in mind? |
@Zoxc Oops, I failed to read which PR this was on, for some reason I thought this one landed. |
r=me after rebase |
Perf is still an issue with this one though =P |
@@ -75,6 +75,7 @@ rustc_queries! { | |||
// This can be conveniently accessed by methods on `tcx.hir()`. | |||
// Avoid calling this query directly. | |||
query hir_owner(key: DefId) -> &'tcx HirOwner<'tcx> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the query keys to LocalDefId
, then you won't have any is_local
checks, maybe that helps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are LocalDefId
in master
now. I don't expect removing a single branch to help much though.
Closing this pull request as Zoxc is stepping back from compiler development; see rust-lang/team#316. |
Waiting on #68988 to land for a perf run.
r? @eddyb