Move `DepKind` to `rustc_query_system` and define it as `u16` #115920

Zoxc · 2023-09-17T22:31:34Z

This moves the DepKind type to rustc_query_system where it's defined with an inner u16 field. This decouples it from rustc_middle and is a step towards letting other crates define dep kinds. It also allows some type parameters to be removed. The DepKind trait is replaced with a Deps trait. That's used when some operations or information about dep kinds which is unavailable in rustc_query_system are still needed.

r? @cjgillot

rustbot · 2023-09-17T22:31:44Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

cjgillot · 2023-09-18T10:55:11Z

@bors try @rust-timer queue

bors · 2023-09-18T10:55:21Z

⌛ Trying commit aa95dcf with merge 30aebb4...

Move `DepKind` to `rustc_query_system` and define it as `u16` This moves the `DepKind` type to `rustc_query_system` where it's defined with an inner `u16` field. This decouples it from `rustc_middle` and is a step towards letting other crates define dep kinds. It also allows some type parameters to be removed. The `DepKind` trait is replaced with a `Deps` trait. That's used when some operations or information about dep kinds which is unavailable in `rustc_query_system` are still needed. r? `@cjgillot`

bors · 2023-09-18T12:06:52Z

☀️ Try build successful - checks-actions
Build commit: 30aebb4 (30aebb459167c051915f5c9ad08126ef72870ce4)

rust-timer · 2023-09-18T14:47:08Z

Finished benchmarking commit (30aebb4): comparison URL.

Overall result: ❌ regressions - 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-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.7%	[0.6%, 0.7%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
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)	-	-	0
Regressions ❌ (secondary)	4.9%	[1.1%, 11.4%]	12
Improvements ✅ (primary)	-2.0%	[-2.0%, -1.9%]	2
Improvements ✅ (secondary)	-2.6%	[-6.6%, -0.8%]	7
All ❌✅ (primary)	-2.0%	[-2.0%, -1.9%]	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)	-1.4%	[-1.6%, -1.1%]	5
Improvements ✅ (secondary)	-2.6%	[-2.8%, -2.4%]	3
All ❌✅ (primary)	-1.4%	[-1.6%, -1.1%]	5

Binary size

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

Bootstrap: 633.305s -> 632.869s (-0.07%)
Artifact size: 317.99 MiB -> 317.96 MiB (-0.01%)

cjgillot

Does this refactor open the way to move ty::tls to rustc_query_system?
That would almost completely remove the need for trait Deps.

compiler/rustc_middle/src/dep_graph/mod.rs

compiler/rustc_middle/src/dep_graph/dep_node.rs

cjgillot · 2023-09-18T17:02:18Z

compiler/rustc_query_system/src/dep_graph/mod.rs

+    const DEP_KIND_NULL: DepKind;
+    const DEP_KIND_RED: DepKind;
+    const DEP_KIND_MAX: u16;


Could you doc-comment those?

cjgillot · 2023-09-18T17:06:41Z

compiler/rustc_query_system/src/dep_graph/mod.rs

+    const DEP_KIND_NULL: DepKind;
+    const DEP_KIND_RED: DepKind;


Could we make those associated constants on DepKind with values 0 and 1, and have rustc_middle check the consistency?

I think it makes sense to delay dealing with these until multiple crates can define dep kinds.

Zoxc · 2023-09-18T19:15:36Z

Does this refactor open the way to move ty::tls to rustc_query_system?

Not very cleanly as you'd have to swap TyCtxt with a pointer.

bors · 2023-09-21T00:31:00Z

☔ The latest upstream changes (presumably #115542) made this pull request unmergeable. Please resolve the merge conflicts.

cjgillot · 2023-09-21T06:32:20Z

r=me after rebase

Zoxc · 2023-09-21T15:51:26Z

Rebased.

cjgillot · 2023-09-21T16:22:49Z

@bors r+

bors · 2023-09-21T16:22:51Z

📌 Commit 1806efe has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-09-21T16:30:11Z

⌛ Testing commit 1806efe with merge fcb7961...

Move `DepKind` to `rustc_query_system` and define it as `u16` This moves the `DepKind` type to `rustc_query_system` where it's defined with an inner `u16` field. This decouples it from `rustc_middle` and is a step towards letting other crates define dep kinds. It also allows some type parameters to be removed. The `DepKind` trait is replaced with a `Deps` trait. That's used when some operations or information about dep kinds which is unavailable in `rustc_query_system` are still needed. r? `@cjgillot`

rust-log-analyzer · 2023-09-21T17:28:47Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2023-09-21T17:29:24Z

💔 Test failed - checks-actions

cjgillot · 2023-09-21T23:27:07Z

@bors retry

bors · 2023-09-22T00:46:17Z

⌛ Testing commit 1806efe with merge b757318...

bors · 2023-09-22T02:36:32Z

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

rust-timer · 2023-09-22T04:11:26Z

Finished benchmarking commit (b757318): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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.3%	[1.8%, 6.1%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-6.8%, -1.2%]	11
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)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-2.1%	[-2.2%, -2.1%]	2
All ❌✅ (primary)	-1.6%	[-1.6%, -1.6%]	1

Binary size

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

Bootstrap: 633.396s -> 631.519s (-0.30%)
Artifact size: 317.61 MiB -> 317.47 MiB (-0.04%)

rustbot assigned cjgillot Sep 17, 2023

This comment has been minimized.

Sign in to view

Zoxc force-pushed the depkind-u16 branch from 368b891 to b2c98c4 Compare September 17, 2023 22:41

This comment has been minimized.

Sign in to view

Zoxc force-pushed the depkind-u16 branch from b2c98c4 to aa95dcf Compare September 17, 2023 23:42

This comment has been minimized.

Sign in to view

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

This comment has been minimized.

Sign in to view

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

cjgillot reviewed Sep 18, 2023

View reviewed changes

Move DepKind to rustc_query_system and define it as u16

1806efe

Zoxc force-pushed the depkind-u16 branch from b93eefa to 1806efe Compare September 21, 2023 15:10

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 21, 2023

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 22, 2023

bors merged commit b757318 into rust-lang:master Sep 22, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Sep 22, 2023

This was referenced Sep 22, 2023

Extract suitable code from rustc_query_impl into a new crate rustc_query_misc #115118

Closed

Make create_def a side effect instead of marking the entire query as always red #115613

Open

Zoxc deleted the depkind-u16 branch September 22, 2023 03:52

lcnr removed the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Sep 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `DepKind` to `rustc_query_system` and define it as `u16` #115920

Move `DepKind` to `rustc_query_system` and define it as `u16` #115920

Zoxc commented Sep 17, 2023

rustbot commented Sep 17, 2023

This comment has been minimized.

This comment has been minimized.

cjgillot commented Sep 18, 2023

This comment has been minimized.

bors commented Sep 18, 2023

bors commented Sep 18, 2023

This comment has been minimized.

rust-timer commented Sep 18, 2023

cjgillot left a comment

cjgillot Sep 18, 2023

cjgillot Sep 18, 2023

Zoxc Sep 18, 2023

Zoxc commented Sep 18, 2023

bors commented Sep 21, 2023

cjgillot commented Sep 21, 2023

Zoxc commented Sep 21, 2023

cjgillot commented Sep 21, 2023

bors commented Sep 21, 2023

bors commented Sep 21, 2023

rust-log-analyzer commented Sep 21, 2023

bors commented Sep 21, 2023

cjgillot commented Sep 21, 2023

bors commented Sep 22, 2023

bors commented Sep 22, 2023

rust-timer commented Sep 22, 2023

Move DepKind to rustc_query_system and define it as u16 #115920

Move DepKind to rustc_query_system and define it as u16 #115920

Conversation

Zoxc commented Sep 17, 2023

rustbot commented Sep 17, 2023

This comment has been minimized.

This comment has been minimized.

cjgillot commented Sep 18, 2023

This comment has been minimized.

bors commented Sep 18, 2023

bors commented Sep 18, 2023

This comment has been minimized.

rust-timer commented Sep 18, 2023

Overall result: ❌ regressions - no action needed

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

Choose a reason for hiding this comment

Zoxc commented Sep 18, 2023

bors commented Sep 21, 2023

cjgillot commented Sep 21, 2023

Zoxc commented Sep 21, 2023

cjgillot commented Sep 21, 2023

bors commented Sep 21, 2023

bors commented Sep 21, 2023

rust-log-analyzer commented Sep 21, 2023

bors commented Sep 21, 2023

cjgillot commented Sep 21, 2023

bors commented Sep 22, 2023

bors commented Sep 22, 2023

rust-timer commented Sep 22, 2023

Overall result: no relevant changes - no action needed

Move `DepKind` to `rustc_query_system` and define it as `u16` #115920

Move `DepKind` to `rustc_query_system` and define it as `u16` #115920