-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Remove SimpleDefKind #94066
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
Remove SimpleDefKind #94066
Conversation
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue (But even without a performance impact, might be a nice small refactor) |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit d338fa7527725b766de357e37bd86bc2f2e21ef9 with merge d9c0847522681f2c45c673d80e2c90eb6138f1ad... |
☀️ Try build successful - checks-actions |
Queued d9c0847522681f2c45c673d80e2c90eb6138f1ad with parent 75d9a0a, future comparison URL. |
Finished benchmarking commit (d9c0847522681f2c45c673d80e2c90eb6138f1ad): comparison url. Summary: This benchmark run did not return any relevant results. 3 results were found to be statistically significant but too small to be relevant. 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 led to changes in compiler perf. @bors rollup=never |
Looks like no significant impact on overall runtimes, probably only worth it if it looks like a nice refactor regardless of bootstrap performance impact (which I suspect is there, just tiny). Might also try adding an inline(never) which could help. |
d338fa7
to
57e534e
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 57e534e93b29107438b4f0a7bbdb2a13de28dbad with merge 7dc78f414000d93ed6f88e572db2a4859cc59aa5... |
☀️ Try build successful - checks-actions |
Queued 7dc78f414000d93ed6f88e572db2a4859cc59aa5 with parent 930fc4f, future comparison URL. |
Finished benchmarking commit (7dc78f414000d93ed6f88e572db2a4859cc59aa5): comparison url. Summary: This benchmark run did not return any relevant results. 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 led to changes in compiler perf. @bors rollup=never |
|
57e534e
to
ddda851
Compare
FWIW, it looks like the only usage of SimpleDefKind is some slight adjustments to cycle diagnostics. I'm not personally convinced those are worth it -- but maybe they are. In any case, something to be considered separately I suppose, for now this just refactors to drop SimpleDefKind in favor of DefKind. @bors try @rust-timer queue I suspect this'll be largely performance neutral or a slight hit -- DefKind is likely larger by a few bytes at least -- but likely to not matter much. |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit ddda851 with merge b61621d972aab729bc51f3643e7381c0860332b9... |
☀️ Try build successful - checks-actions |
Queued b61621d972aab729bc51f3643e7381c0860332b9 with parent 30b3f35, future comparison URL. |
Finished benchmarking commit (b61621d972aab729bc51f3643e7381c0860332b9): comparison url. Summary: This benchmark run did not return any relevant results. 5 results were found to be statistically significant but too small to be relevant. 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 led to changes in compiler perf. @bors rollup=never |
@bors r+ |
📌 Commit ddda851 has been approved by |
⌛ Testing commit ddda851 with merge cdb64f7d343d11888d1c6e8f5e647c8f22b95cdd... |
💔 Test failed - checks-actions |
@bors retry aarch64 runner down? |
☀️ Test successful - checks-actions |
Finished benchmarking commit (026d8ce): comparison url. Summary: This benchmark run did not return any relevant results. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Now that rustc_query_system depends on rustc_hir, we can just directly make use of the regular DefKind.