-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Use a more lightweight cache for erase_regions #59505
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors try |
⌛ Trying commit 6b6cb87e9137e5d593e0f42b9af92a135e9e38f7 with merge 6400173020be5019c2bbcadabcabd413750de993... |
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 |
💔 Test failed - checks-travis |
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 d633fd98da7acf14eee5e4339b8909e986580687 with merge 6284bc1b5909f6452af037b507190bbabb1a4d4d... |
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 |
@rust-timer build 6284bc1b5909f6452af037b507190bbabb1a4d4d |
Success: Queued 6284bc1b5909f6452af037b507190bbabb1a4d4d with parent 237bf32, comparison URL. |
☀️ Try build successful - checks-travis |
Finished benchmarking try commit 6284bc1b5909f6452af037b507190bbabb1a4d4d |
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 |
I'd like to enable caches for dependency-less functions like this one but I'm not sure a debug assertion is enough. Can we instead of make sure at compile time that a function cannot have dependencies? One way to do that would be to not give it access to the |
I don't see how, given that we allow access to |
Yeah, that's true It would still be good to turn this into a generic, sanctioned pattern with safe-guards against foot gunning ourselves. Maybe:
The cc @nikomatsakis, who (I think) mentioned wanting to support dependenciless functions in Salsa too. |
☔ The latest upstream changes (presumably #59517) made this pull request unmergeable. Please resolve the merge conflicts. |
This is also the only thing using anon queries, which I'd also like to remove since it doesn't make sense to me. |
As I said, I'm not against doing something like this in principle. But this isn't a pressing issue (i.e. the current solution works and performance improvement nice but not stellar), so I'd prefer if we supported this pattern in a general way (along the lines I described above) instead of doing a one-off solution. |
ping from triage @michaelwoerister any updates on this? |
re: triage: My suggestions on how to move forward here are still up-to-date. |
No description provided.