-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Have the per-query caches store the results on arenas #70674
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
17fb3fd
to
5196dcc
Compare
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 |
☔ The latest upstream changes (presumably #70692) made this pull request unmergeable. Please resolve the merge conflicts. |
828930c
to
148b529
Compare
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 @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit c9f013d922cee0d0fefa70bac5de4d8427ad7340 with merge 5645cd25e3c73522f0b1ab615facc32c8537f062... |
☀️ Try build successful - checks-azure |
Queued 5645cd25e3c73522f0b1ab615facc32c8537f062 with parent 1b521f5, future comparison URL. |
This appears to be a slight perf regression. |
Indeed, and I have no idea why. |
@cjgillot any updates on this? |
c9f013d
to
8fd36da
Compare
☔ The latest upstream changes (presumably #71044) made this pull request unmergeable. Please resolve the merge conflicts. |
8fd36da
to
dc3a0e7
Compare
☔ The latest upstream changes (presumably #71215) made this pull request unmergeable. Please resolve the merge conflicts. |
dc3a0e7
to
4bcc53b
Compare
☔ The latest upstream changes (presumably #71292) made this pull request unmergeable. Please resolve the merge conflicts. |
4bcc53b
to
d7d2185
Compare
Rebased. Can I get another perf run? |
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit d7d2185 with merge 040acbcab104f84b25bd6be13b98182374fff2ab... |
☀️ Try build successful - checks-azure |
Queued 040acbcab104f84b25bd6be13b98182374fff2ab with parent fb5615a, future comparison URL. |
Finished benchmarking try commit 040acbcab104f84b25bd6be13b98182374fff2ab, comparison URL. |
Perf is neutral. |
@bors r+ |
📌 Commit d7d2185 has been approved by |
☀️ Test successful - checks-azure |
Remove QueryStorage::store_nocache This method was added in rust-lang#70674 but it doesn't seem to serve any purpose.
Remove QueryStorage::store_nocache This method was added in rust-lang/rust#70674 but it doesn't seem to serve any purpose.
This PR leverages the cache for each query to serve as storage area for the query results.
It introduces a new cache
ArenaCache
, which moves the result to an arena,and only stores the reference in the hash map.
This allows to remove a sizeable part of the usage of the global
TyCtxt
arena.I only migrated queries that already used arenas before.