Rejig rustc_with_all_queries!

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.

[nnethercote]

View all comments

There are three things relating to rustc_with_all_queries! that have been bugging me.

• rustc_with_all_queries!'s ability to receive extra_fake_queries lines like [] fn Null(()) -> (), where the only real thing is the Null, and everything is just pretending to be a normal query, ugh.
• make_dep_kind_array!: a macro produced by one macro (define_dep_nodes!) and used by another macro (define_queries!) in another crate, ugh.
• The _dep_kind_vtable_ctors module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by make_dep_kind_array!, ugh.

By making some adjustments to how rustc_with_all_queries! works, all three of these things are eliminated.

r? @Zalathar

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[nnethercote]

@Zalathar: I find it hard to predict where our opinions will align on cleanups, but I figure I'll just keep throwing stuff against the wall to see what sticks.

A couple of things I'm uncertain about this one:

• I've used the term "non-queries" for Null, Red, etc. Not sure that's the best. "Fake" queries, maybe?
• As per the "njn:" comment, I'm not sure if the proc macro is the best place to embed the list of non-queries.

I'm interested to hear what you think, thanks.

[Zalathar]

At first this didn't jump out to me as a big improvement, but after pondering it some more I think it's probably a good direction.

The part I really like is being able to get rid of $extra_fake_queries from rustc_with_all_queries!.

Having to explicitly ignore non_queries in all of the other callback macros is a bit unfortunate, but I think it's a good tradeoff, because it's just a tiny bit of boilerplate that is easily ignored.

I don't have great suggestions for what to call the non-query dep kinds. Some possibilities that come to mind are non_query_dep_kinds or special_dep_kinds, which are perhaps a little bit more self-describing when seen in contexts that don't care about things that aren't queries.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nnethercote]

Comments addressed, should be ready for re-review.

[Zalathar]

Thanks, r=me after resolving and/or rejecting each of the nits.

[nnethercote]

I addressed all the nits. Let's do a perf run to be safe.

@bors try @rust-timer queue

[rust-timer]

Finished benchmarking commit (dc13f8f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.4%, 4.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.5%	[-7.5%, -7.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-7.5%, 4.8%]	3

Cycles

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

Binary size

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

Bootstrap: 480.149s -> 477.903s (-0.47%)
Artifact size: 396.96 MiB -> 396.96 MiB (-0.00%)

[nnethercote]

No perf effects.

@bors r=Zalathar rollup=maybe

[rust-bors]

📌 Commit 40b2274 has been approved by Zalathar

It is now in the queue for this repository.

[JonathanBrouwer]

@rust-timer build 2168ca5

[rust-timer]

Finished benchmarking commit (2168ca5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.0%, secondary -2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.1%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.6%	[-9.6%, -9.6%]	1
Improvements ✅ (secondary)	-2.7%	[-4.3%, -1.1%]	2
All ❌✅ (primary)	-1.0%	[-9.6%, 3.6%]	3

Cycles

Results (secondary 5.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.6%	[4.5%, 6.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 480.113s -> 477.631s (-0.52%)
Artifact size: 396.96 MiB -> 394.94 MiB (-0.51%)

[Zoxc]

non_query seems quite odd to me. I'd probably prefer just dep_kind.