Add inlining attributes for query system functions #108375

Zoxc · 2023-02-23T00:15:57Z

These only have a single caller, but don't always get inlined.

rustbot · 2023-02-23T00:16:04Z

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

Kobzol · 2023-02-23T09:32:36Z

@bors try @rust-timer queue

bors · 2023-02-23T09:32:46Z

⌛ Trying commit b27826e76f00e789ffafd409fcd9ee25abbb67e5 with merge b621e618f35e198e1804feafe1b868837650200f...

bors · 2023-02-23T12:13:06Z

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

rust-timer · 2023-02-23T18:03:12Z

Finished benchmarking commit (b621e618f35e198e1804feafe1b868837650200f): comparison URL.

Overall result: ❌✅ regressions and improvements - 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-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.3%	[0.2%, 0.4%]	9
Regressions ❌ (secondary)	0.4%	[0.1%, 2.6%]	22
Improvements ✅ (primary)	-0.8%	[-0.9%, -0.7%]	2
Improvements ✅ (secondary)	-1.5%	[-2.4%, -0.3%]	9
All ❌✅ (primary)	0.1%	[-0.9%, 0.4%]	11

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

Zoxc · 2023-02-24T22:43:53Z

I've added more inlining attribute after inspecting LLVM IR output. This is now also based on #108134.

The cumulative effect is now a performance win locally:

Benchmark	Before	After
Benchmark	Time	Time	%
🟣 clap:check:unchanged	0.4699s	0.4687s	-0.26%
🟣 hyper:check:unchanged	0.1359s	0.1348s	-0.84%
🟣 regex:check:unchanged	0.3395s	0.3382s	-0.36%
🟣 syn:check:unchanged	0.6333s	0.6292s	-0.65%
🟣 syntex_syntax:check:unchanged	1.8633s	1.8458s	-0.94%
Total	3.4419s	3.4167s	-0.73%
Summary	1.0000s	0.9939s	-0.61%

Benchmark	Before	After
Benchmark	Time	Time	%
🟣 clap:check	1.6962s	1.6815s	-0.86%
🟣 hyper:check	0.2551s	0.2550s	-0.05%
🟣 regex:check	0.9684s	0.9578s	💚 -1.10%
🟣 syn:check	1.5518s	1.5419s	-0.64%
🟣 syntex_syntax:check	5.9008s	5.8854s	-0.26%
Total	10.3724s	10.3217s	-0.49%
Summary	1.0000s	0.9942s	-0.58%

This could use another perf run now.

lqd · 2023-02-25T06:50:36Z

@bors try @rust-timer queue

bors · 2023-02-25T06:50:45Z

⌛ Trying commit dd73080 with merge 0b48b07587a417881f1bff44bece1fbbe93f6b90...

bors · 2023-02-25T09:37:09Z

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

rust-timer · 2023-02-25T11:53:11Z

Finished benchmarking commit (0b48b07587a417881f1bff44bece1fbbe93f6b90): comparison URL.

Overall result: ❌✅ regressions and improvements - 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-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.4%	[0.2%, 0.5%]	2
Regressions ❌ (secondary)	0.4%	[0.1%, 0.7%]	10
Improvements ✅ (primary)	-0.4%	[-1.0%, -0.2%]	59
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	20
All ❌✅ (primary)	-0.4%	[-1.0%, 0.5%]	61

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.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	4.3%	[4.2%, 4.5%]	2
Improvements ✅ (primary)	-5.6%	[-5.6%, -5.6%]	1
Improvements ✅ (secondary)	-3.0%	[-3.7%, -2.3%]	2
All ❌✅ (primary)	-1.7%	[-5.6%, 2.3%]	2

Cycles

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

cjgillot · 2023-02-25T13:20:57Z

Perf is mostly green, and green on average.
@bors r+

bors · 2023-02-25T13:20:58Z

📌 Commit dd73080 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-02-26T09:44:57Z

⌛ Testing commit dd73080 with merge 43ee4d1...

bors · 2023-02-26T12:39:58Z

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

rust-timer · 2023-02-26T15:12:49Z

Finished benchmarking commit (43ee4d1): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.4%	[0.2%, 0.7%]	5
Improvements ✅ (primary)	-0.4%	[-1.1%, -0.2%]	38
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	12
All ❌✅ (primary)	-0.4%	[-1.1%, -0.2%]	38

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)	-4.3%	[-4.6%, -3.9%]	3
All ❌✅ (primary)	-	-	0

Cycles

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

pnkfelix · 2023-03-01T03:31:08Z

wins clearly outweigh losses here.
marking as triaged.

@rustbot label: perf-regression-triaged

rustbot assigned compiler-errors Feb 23, 2023

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 Feb 23, 2023

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 23, 2023

Zoxc force-pushed the query-inline branch from b27826e to a098a96 Compare February 24, 2023 22:40

Zoxc changed the title ~~Always inline execute_job and get_query~~ Add inlining attributes for query system functions Feb 24, 2023

Zoxc force-pushed the query-inline branch from a098a96 to 6fbb9e9 Compare February 25, 2023 00:22

Zoxc added 4 commits February 25, 2023 06:11

Reduce calls to current_query_job

5fa60a5

Move ensure_sufficient_stack to try_execute_query callers

a049550

Add inlining attributes for query system functions

ab5d3fb

Don't inline try_execute_query

dd73080

Zoxc force-pushed the query-inline branch from 6fbb9e9 to dd73080 Compare February 25, 2023 05:11

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 Feb 25, 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 Feb 25, 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 Feb 25, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 26, 2023

bors merged commit 43ee4d1 into rust-lang:master Feb 26, 2023

rustbot added this to the 1.69.0 milestone Feb 26, 2023

bors mentioned this pull request Feb 26, 2023

Make rustc_query_system take QueryConfig by instance. #108167

Merged

Zoxc deleted the query-inline branch February 26, 2023 22:32

rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 1, 2023

Zoxc mentioned this pull request Mar 2, 2023

Don't monomorphize ty::tls closures in rustc_query_impl #106311

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inlining attributes for query system functions #108375

Add inlining attributes for query system functions #108375

Zoxc commented Feb 23, 2023

rustbot commented Feb 23, 2023

Kobzol commented Feb 23, 2023

This comment has been minimized.

bors commented Feb 23, 2023

bors commented Feb 23, 2023

This comment has been minimized.

rust-timer commented Feb 23, 2023

Zoxc commented Feb 24, 2023

lqd commented Feb 25, 2023

This comment has been minimized.

bors commented Feb 25, 2023

bors commented Feb 25, 2023

This comment has been minimized.

rust-timer commented Feb 25, 2023

cjgillot commented Feb 25, 2023

bors commented Feb 25, 2023

bors commented Feb 26, 2023

bors commented Feb 26, 2023

rust-timer commented Feb 26, 2023

pnkfelix commented Mar 1, 2023

Add inlining attributes for query system functions #108375

Add inlining attributes for query system functions #108375

Conversation

Zoxc commented Feb 23, 2023

rustbot commented Feb 23, 2023

Kobzol commented Feb 23, 2023

This comment has been minimized.

bors commented Feb 23, 2023

bors commented Feb 23, 2023

This comment has been minimized.

rust-timer commented Feb 23, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Zoxc commented Feb 24, 2023

lqd commented Feb 25, 2023

This comment has been minimized.

bors commented Feb 25, 2023

bors commented Feb 25, 2023

This comment has been minimized.

rust-timer commented Feb 25, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

cjgillot commented Feb 25, 2023

bors commented Feb 25, 2023

bors commented Feb 26, 2023

bors commented Feb 26, 2023

rust-timer commented Feb 26, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

pnkfelix commented Mar 1, 2023