Move DepKindStruct from rustc_middle to rustc_query_system #101710

jyn514 · 2022-09-12T04:34:30Z

Helps with #96524. cc https://rust-lang.zulipchat.com/#narrow/stream/241847-t-compiler.2Fwg-incr-comp/topic/Moving.20.60DepKindStruct.60.20to.20rustc_query_system.20.2396524

r? @cjgillot

jyn514 · 2022-09-12T04:36:08Z

compiler/rustc_middle/src/dep_graph/dep_node.rs

    fn extract_def_id<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<DefId> {
-        if self.kind.fingerprint_style(tcx) == FingerprintStyle::DefPathHash {
+        if tcx.fingerprint_style(self.kind) == FingerprintStyle::DefPathHash {
            Some(tcx.def_path_hash_to_def_id(DefPathHash(self.hash.into()), &mut || {


This can't be moved to rustc_query_system without adding def_path_hash_to_def_id to the DepContext trait; I can do that, but not sure it was worth the churn. Let me know and I'm happy to change it.

compiler/rustc_middle/src/dep_graph/mod.rs

jyn514 · 2022-09-12T04:36:37Z

compiler/rustc_middle/src/dep_graph/mod.rs

+    fn from_label_string(label: &str) -> Result<Self, ()> {
+        dep_node::dep_kind_from_label_string(label)


needed for the functions I removed from the DepNodeExt trait

jyn514 · 2022-09-12T04:37:50Z

compiler/rustc_query_system/src/dep_graph/mod.rs

-    fn try_force_from_dep_node(&self, dep_node: DepNode<Self::DepKind>) -> bool;
+    fn try_force_from_dep_node(self, dep_node: DepNode<Self::DepKind>) -> bool {


It's possible to make these and the others standalone functions instead of part of the trait, but then I have to change the callsites from tcx.is_eval_always(kind) to is_eval_always(tcx, kind) (and deal with privacy, re-exports, etc). Let me know if you'd prefer I change it.

cjgillot · 2022-09-13T17:05:56Z

@bors try @rust-timer queue

rust-timer · 2022-09-13T17:05:59Z

Awaiting bors try build completion.

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

bors · 2022-09-13T17:06:07Z

⌛ Trying commit bc1ed929c227ed5d3036ec95beb685ff7e93ca02 with merge f293fc9a1c28900c94a2e3ec999f92888e22d37d...

bors · 2022-09-13T18:45:26Z

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

rust-timer · 2022-09-13T18:45:28Z

Queued f293fc9a1c28900c94a2e3ec999f92888e22d37d with parent 5338f5f, future comparison URL.

rust-timer · 2022-09-13T20:48:36Z

Finished benchmarking commit (f293fc9a1c28900c94a2e3ec999f92888e22d37d): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

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

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)	1.4%	[1.4%, 1.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-1.7%, -1.7%]	1
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.9%	[2.9%, 2.9%]	1
Regressions ❌ (secondary)	2.7%	[1.8%, 4.0%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-2.8%, -2.8%]	1
All ❌✅ (primary)	2.9%	[2.9%, 2.9%]	1

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)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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

jyn514 · 2022-09-18T22:23:06Z

@bors try @rust-timer queue

rust-timer · 2022-09-18T22:23:07Z

Awaiting bors try build completion.

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

bors · 2022-09-18T22:29:58Z

⌛ Trying commit 6f86352ff611abb0bf96c6a527659b767c2315a8 with merge 7d71171239224e27e782f4734b314e7bdbf131a3...

bors · 2022-09-19T00:05:34Z

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

rust-timer · 2022-09-19T00:05:36Z

Queued 7d71171239224e27e782f4734b314e7bdbf131a3 with parent a37499a, future comparison URL.

rust-timer · 2022-09-19T01:22:35Z

Finished benchmarking commit (7d71171239224e27e782f4734b314e7bdbf131a3): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +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.7%	[0.4%, 1.3%]	82
Regressions ❌ (secondary)	1.1%	[0.3%, 2.0%]	34
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.4%, 1.3%]	82

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)	-	-	0
Regressions ❌ (secondary)	3.4%	[3.4%, 3.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-3.0%, -2.7%]	2
All ❌✅ (primary)	-	-	0

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)	2.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-3.2%, -2.0%]	11
All ❌✅ (primary)	-	-	0

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

…stem

…ery_impl` This allows removing a function from the `DepKind` trait.

jyn514 · 2022-09-25T17:08:44Z

From the code POV, this looks good. Do you have an idea for the regression? A missing inline perhaps?

Not sure off the top of my head - I added inline to dep_kind_info, I'll see if that helps.

jyn514 · 2022-09-25T17:08:55Z

@bors try @rust-timer queue

rust-timer · 2022-09-25T17:08:57Z

Awaiting bors try build completion.

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

bors · 2022-09-25T17:09:03Z

⌛ Trying commit 00cde6d with merge 1480da8b0f4003f052b5450cd4b17b2f71caede5...

bors · 2022-09-25T18:59:24Z

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

rust-timer · 2022-09-25T18:59:26Z

Queued 1480da8b0f4003f052b5450cd4b17b2f71caede5 with parent 17e8752, future comparison URL.

rust-timer · 2022-09-25T20:19:53Z

Finished benchmarking commit (1480da8b0f4003f052b5450cd4b17b2f71caede5): comparison URL.

Overall result: ✅ improvements - no action needed

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

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.5%	[-0.9%, -0.2%]	66
Improvements ✅ (secondary)	-0.6%	[-1.4%, -0.3%]	22
All ❌✅ (primary)	-0.5%	[-0.9%, -0.2%]	66

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)	-	-	0
Regressions ❌ (secondary)	4.4%	[4.4%, 4.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-3.2%, -3.2%]	1
All ❌✅ (primary)	-	-	0

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)	3.2%	[3.2%, 3.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.2%	[3.2%, 3.2%]	1

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

cjgillot · 2022-09-25T21:09:40Z

@bors r+

bors · 2022-09-25T21:09:41Z

📌 Commit 00cde6d has been approved by cjgillot

It is now in the queue for this repository.

bors · 2022-09-25T21:37:13Z

⌛ Testing commit 00cde6d with merge ff40f2e...

bors · 2022-09-26T00:17:52Z

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

rust-timer · 2022-09-26T01:34:46Z

Finished benchmarking commit (ff40f2e): 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.5%	[-0.9%, -0.2%]	66
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	18
All ❌✅ (primary)	-0.5%	[-0.9%, -0.2%]	66

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.3%, -2.0%]	2
All ❌✅ (primary)	-	-	0

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)	-2.2%	[-2.2%, -2.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.2%	[-2.2%, -2.2%]	1

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

rust-highfive assigned cjgillot Sep 12, 2022

rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 12, 2022

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

jyn514 force-pushed the move-dep-kind-node branch from ad6b82c to bc1ed92 Compare September 12, 2022 04:35

jyn514 commented Sep 12, 2022

View reviewed changes

jyn514 mentioned this pull request Sep 12, 2022

Refactor the query system to reduce the amount of macro-expanded functions #96524

Closed

6 tasks

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

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 13, 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 Sep 14, 2022

jyn514 force-pushed the move-dep-kind-node branch from c805fd0 to 6f86352 Compare September 18, 2022 22:07

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

rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Sep 19, 2022

Move functions on DepKindStruct from rustc_middle to rustc_query_sy…

f3f91bb

…stem

jyn514 force-pushed the move-dep-kind-node branch from 5202395 to 7c3bd30 Compare September 25, 2022 17:07

jyn514 added 2 commits September 25, 2022 12:08

Move some more code from rustc_middle to rustc_query_system

ccc8d00

Move the codegen_unit debug assert from rustc_query_system to `qu…

00cde6d

…ery_impl` This allows removing a function from the `DepKind` trait.

jyn514 force-pushed the move-dep-kind-node branch from 7c3bd30 to 00cde6d Compare September 25, 2022 17:08

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

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Sep 25, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 26, 2022

bors merged commit ff40f2e into rust-lang:master Sep 26, 2022

rustbot added this to the 1.66.0 milestone Sep 26, 2022

bors mentioned this pull request Sep 26, 2022

Compute lint levels by definition #102236

Merged

jyn514 deleted the move-dep-kind-node branch February 25, 2023 18:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move DepKindStruct from rustc_middle to rustc_query_system #101710

Move DepKindStruct from rustc_middle to rustc_query_system #101710

jyn514 commented Sep 12, 2022

jyn514 Sep 12, 2022

jyn514 Sep 12, 2022

jyn514 Sep 12, 2022

cjgillot commented Sep 13, 2022

rust-timer commented Sep 13, 2022

bors commented Sep 13, 2022

bors commented Sep 13, 2022

rust-timer commented Sep 13, 2022

rust-timer commented Sep 13, 2022

jyn514 commented Sep 18, 2022

rust-timer commented Sep 18, 2022

bors commented Sep 18, 2022

bors commented Sep 19, 2022

rust-timer commented Sep 19, 2022

rust-timer commented Sep 19, 2022

jyn514 commented Sep 25, 2022

jyn514 commented Sep 25, 2022

rust-timer commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 25, 2022

rust-timer commented Sep 25, 2022

rust-timer commented Sep 25, 2022

cjgillot commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 26, 2022

rust-timer commented Sep 26, 2022

		fn from_label_string(label: &str) -> Result<Self, ()> {
		dep_node::dep_kind_from_label_string(label)

		fn try_force_from_dep_node(&self, dep_node: DepNode<Self::DepKind>) -> bool;
		fn try_force_from_dep_node(self, dep_node: DepNode<Self::DepKind>) -> bool {

Move DepKindStruct from rustc_middle to rustc_query_system #101710

Move DepKindStruct from rustc_middle to rustc_query_system #101710

Conversation

jyn514 commented Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjgillot commented Sep 13, 2022

rust-timer commented Sep 13, 2022

bors commented Sep 13, 2022

bors commented Sep 13, 2022

rust-timer commented Sep 13, 2022

rust-timer commented Sep 13, 2022

Overall result: ❌✅ regressions and improvements - no action needed

Footnotes

jyn514 commented Sep 18, 2022

rust-timer commented Sep 18, 2022

bors commented Sep 18, 2022

bors commented Sep 19, 2022

rust-timer commented Sep 19, 2022

rust-timer commented Sep 19, 2022

Overall result: ❌ regressions - ACTION NEEDED

Footnotes

jyn514 commented Sep 25, 2022

jyn514 commented Sep 25, 2022

rust-timer commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 25, 2022

rust-timer commented Sep 25, 2022

rust-timer commented Sep 25, 2022

Overall result: ✅ improvements - no action needed

Footnotes

cjgillot commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 25, 2022

bors commented Sep 26, 2022

rust-timer commented Sep 26, 2022

Overall result: ✅ improvements - no action needed

Footnotes