try and simplify some things in the query system #100436

jyn514 · 2022-08-12T03:29:49Z

No description provided.

rust-highfive · 2022-08-12T03:29:52Z

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

jyn514 · 2022-08-12T03:30:26Z

@bors try @rust-timer queue

rust-timer · 2022-08-12T03:30:27Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-08-12T03:30:36Z

⌛ Trying commit 09a56fd69bee9590d20526a447b8879c68a1b42e with merge b4e3257899c45300440fd556ce4d3bf0517ee057...

bors · 2022-08-12T04:54:36Z

☀️ Try build successful - checks-actions
Build commit: b4e3257899c45300440fd556ce4d3bf0517ee057 (b4e3257899c45300440fd556ce4d3bf0517ee057)

rust-timer · 2022-08-12T04:54:37Z

Queued b4e3257899c45300440fd556ce4d3bf0517ee057 with parent b998821, future comparison URL.

rust-timer · 2022-08-12T06:10:51Z

Finished benchmarking commit (b4e3257899c45300440fd556ce4d3bf0517ee057): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

Primary benchmarks: mixed results
Secondary benchmarks: ✅ relevant improvement found

	mean¹	max	count²
Regressions ❌ (primary)	2.5%	2.5%	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.1%	-4.1%	1
Improvements ✅ (secondary)	-3.6%	-3.6%	1
All ❌✅ (primary)	-0.8%	-4.1%	2

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

the arithmetic mean of the percent change ↩
number of relevant changes ↩

bjorn3 · 2022-08-12T10:34:09Z

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -143,7 +143,7 @@ impl DepKind {
 }

 macro_rules! define_dep_nodes {
-    (<$tcx:tt>
+    (//<$tcx:tt>


Comment should be removed

bjorn3 · 2022-08-12T10:34:20Z

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -179,7 +179,7 @@ macro_rules! define_dep_nodes {
    );
 }

-rustc_dep_node_append!([define_dep_nodes!][ <'tcx>
+rustc_dep_node_append!([define_dep_nodes!][ //<'tcx>


bjorn3 · 2022-08-12T10:35:42Z

compiler/rustc_query_impl/src/profiling_support.rs

@@ -306,7 +306,7 @@ pub fn alloc_self_profile_query_strings(tcx: TyCtxt<'_>) {
    let mut string_cache = QueryKeyStringCache::new();

    macro_rules! alloc_once {
-        (<$tcx:tt>
+        (//<$tcx:tt>


cjgillot

r=me with one nit and commented out code removed.
Marking as part of #96524 because of the change on make_query module.

cjgillot · 2022-08-14T10:13:39Z

compiler/rustc_query_impl/src/plumbing.rs

-                opt_remap_env_constness!([$($modifiers)*][key]);
-                let kind = dep_graph::DepKind::$name;
-                let name = stringify!($name);
+            fn create_query_frame<'tcx, K: Copy + Key + for<'a> HashStable<StableHashingContext<'a>>>(tcx: QueryCtxt<'tcx>, do_describe: fn(QueryCtxt<'tcx>, K) -> String, key: K, kind: DepKind, name: &'static str) -> QueryStackFrame {


Can you move this outside the macro?

good idea, done :) had to rearrange a bunch of imports because define_queries is expanded in the crate root; always confuses me that those are different. I'll see if I can make that the same in a follow-up PR actually.

lcnr · 2022-08-14T20:38:08Z

r? @cjgillot

It's not actually necessary and it makes the code harder to read.

Rustdoc documents these with the name of the type alias instead of normalizing them to the underlying type. Use associated types instead so that the generated docs for nightly-rustc are easier to read.

jyn514 · 2022-08-24T02:41:34Z

@bors r=cjgillot rollup=never (very perf-sensitive)

bors · 2022-08-24T02:41:36Z

📌 Commit c6c5bf7cf262d7e86f75ba312ba0e5efe01eeb77 has been approved by cjgillot

It is now in the queue for this repository.

jyn514 · 2022-08-24T02:51:40Z

@bors r-

This should both make the code easier to read and also greatly reduce the amount of codegen the compiler has to do, since it only needs to monomorphize `create_query_frame` for each new key and not for each query.

jyn514 · 2022-08-24T02:53:33Z

@bors r=cjgillot rollup=never (very perf-sensitive)

bors · 2022-08-24T02:53:35Z

📌 Commit e188868 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2022-08-25T05:35:30Z

⌛ Testing commit e188868 with merge 76531be...

bors · 2022-08-25T08:16:35Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 76531be to master...

rust-timer · 2022-08-25T10:08:01Z

Finished benchmarking commit (76531be): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.3%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	2.5%	[2.5%, 2.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.1%	[-5.1%, -5.1%]	1
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-1.3%	[-5.1%, 2.5%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-4.0%, -2.0%]	2
All ❌✅ (primary)	-	-	0

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

Simplify the arguments to macros generated by the `rustc_queries` proc macro Very small cleanup. Based on rust-lang#100436 which modifies some of the same code. r? `@cjgillot`

rust-highfive assigned lcnr Aug 12, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2022

jyn514 force-pushed the macro-query-system branch from fa3df6c to 09a56fd Compare August 12, 2022 03:30

jyn514 removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 12, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 12, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 12, 2022

bjorn3 reviewed Aug 12, 2022

View reviewed changes

cjgillot self-assigned this Aug 12, 2022

cjgillot reviewed Aug 14, 2022

View reviewed changes

rust-highfive unassigned lcnr Aug 14, 2022

cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 18, 2022

jyn514 force-pushed the macro-query-system branch from 09a56fd to bf77d94 Compare August 24, 2022 02:30

jyn514 added 2 commits August 23, 2022 21:33

Remove $tcx metavariable from rustc_query_append

b537619

It's not actually necessary and it makes the code harder to read.

Get rid of some usages of query_keys

1de08b1

Rustdoc documents these with the name of the type alias instead of normalizing them to the underlying type. Use associated types instead so that the generated docs for nightly-rustc are easier to read.

jyn514 force-pushed the macro-query-system branch from bf77d94 to c6c5bf7 Compare August 24, 2022 02:35

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2022

jyn514 added 2 commits August 23, 2022 21:52

Move most of make_query into a generic function, away from the macro

0bedd35

This should both make the code easier to read and also greatly reduce the amount of codegen the compiler has to do, since it only needs to monomorphize `create_query_frame` for each new key and not for each query.

get rid of another unnecessary lifetime macro argument

e188868

jyn514 force-pushed the macro-query-system branch from c6c5bf7 to e188868 Compare August 24, 2022 02:52

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2022

jyn514 mentioned this pull request Aug 24, 2022

Simplify the define_query macro #100943

Merged

jyn514 mentioned this pull request Aug 24, 2022

Simplify the arguments to macros generated by the rustc_queries proc macro #100946

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 25, 2022

bors merged commit 76531be into rust-lang:master Aug 25, 2022

rustbot added this to the 1.65.0 milestone Aug 25, 2022

jyn514 deleted the macro-query-system branch August 25, 2022 14:27

matthiaskrgr mentioned this pull request Aug 4, 2023

ICE: broken mir with -Zprint-mono-items=eager between passes RemoveStorageMarkers and Inline #114458

Closed

matthiaskrgr mentioned this pull request Dec 18, 2023

MIR validation ICE: Use of local which has no storage #118990

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

try and simplify some things in the query system #100436

try and simplify some things in the query system #100436

jyn514 commented Aug 12, 2022

rust-highfive commented Aug 12, 2022

jyn514 commented Aug 12, 2022

rust-timer commented Aug 12, 2022

bors commented Aug 12, 2022

bors commented Aug 12, 2022

rust-timer commented Aug 12, 2022

rust-timer commented Aug 12, 2022

bjorn3 Aug 12, 2022 •

edited

Loading

bjorn3 Aug 12, 2022

bjorn3 Aug 12, 2022

cjgillot left a comment

cjgillot Aug 14, 2022

jyn514 Aug 24, 2022

lcnr commented Aug 14, 2022

jyn514 commented Aug 24, 2022

bors commented Aug 24, 2022

jyn514 commented Aug 24, 2022

jyn514 commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 25, 2022

bors commented Aug 25, 2022

rust-timer commented Aug 25, 2022

try and simplify some things in the query system #100436

try and simplify some things in the query system #100436

Conversation

jyn514 commented Aug 12, 2022

rust-highfive commented Aug 12, 2022

jyn514 commented Aug 12, 2022

rust-timer commented Aug 12, 2022

bors commented Aug 12, 2022

bors commented Aug 12, 2022

rust-timer commented Aug 12, 2022

rust-timer commented Aug 12, 2022

Footnotes

bjorn3 Aug 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjgillot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Aug 14, 2022

jyn514 commented Aug 24, 2022

bors commented Aug 24, 2022

jyn514 commented Aug 24, 2022

jyn514 commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 25, 2022

bors commented Aug 25, 2022

rust-timer commented Aug 25, 2022

Overall result: ✅ improvements - no action needed

Footnotes

bjorn3 Aug 12, 2022 •

edited

Loading