Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a single value cache for the () query key #107643

Merged
merged 1 commit into from
Feb 12, 2023

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Feb 3, 2023

Since queries using () as the key can only store a single value, specialize for that case.

This looks like a minor performance improvement:

BenchmarkBeforeAfter
TimeTime%
🟣 clap:check1.8477s1.8415s -0.33%
🟣 hyper:check0.2666s0.2655s -0.40%
🟣 syntex_syntax:check6.3943s6.3686s -0.40%
🟣 syn:check1.6413s1.6345s -0.42%
🟣 regex:check1.0337s1.0313s -0.24%
Total11.1836s11.1414s -0.38%
Summary1.0000s0.9964s -0.36%

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2023

r? @michaelwoerister

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

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 3, 2023
@Kobzol
Copy link
Contributor

Kobzol commented Feb 3, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 3, 2023
@bors
Copy link
Contributor

bors commented Feb 3, 2023

⌛ Trying commit 0b5596289641e0a47ca3d884f307a7e96bdfc151 with merge 9b74d0d9d6c9601238b6a07f19da4ca467e78dcd...

@cjgillot cjgillot self-assigned this Feb 3, 2023
@cjgillot
Copy link
Contributor

cjgillot commented Feb 4, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 4, 2023

⌛ Trying commit ba143bc3067c723344af9cdd305732d909f12597 with merge b3fa81ef1f2c856493537937ac8080386c81739a...

@bors
Copy link
Contributor

bors commented Feb 4, 2023

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

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b3fa81ef1f2c856493537937ac8080386c81739a): 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
Regressions ❌
(secondary)
0.6% [0.5%, 0.7%] 3
Improvements ✅
(primary)
-0.3% [-0.3%, -0.2%] 8
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.2%] 3
All ❌✅ (primary) -0.3% [-0.3%, -0.2%] 8

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)
3.5% [2.7%, 4.6%] 4
Regressions ❌
(secondary)
2.8% [0.6%, 4.7%] 24
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.5% [2.7%, 4.6%] 4

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)
1.2% [1.2%, 1.2%] 1
Improvements ✅
(primary)
-1.1% [-1.1%, -1.1%] 2
Improvements ✅
(secondary)
-1.7% [-2.1%, -1.5%] 6
All ❌✅ (primary) -1.1% [-1.1%, -1.1%] 2

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 4, 2023
@cjgillot
Copy link
Contributor

cjgillot commented Feb 7, 2023

My pending review was asking for a version with arena, but considering this discussion https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60arena_cache.60.20query.20option

Perf is neutral-green, within <1% range.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 7, 2023

📌 Commit ba143bc3067c723344af9cdd305732d909f12597 has been approved by cjgillot

It is now in the queue for this repository.

@bors 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 7, 2023
@klensy
Copy link
Contributor

klensy commented Feb 7, 2023

Perf is neutral-green, within <1% range.

While instructions ok, max-rss not so neutral.

@bors
Copy link
Contributor

bors commented Feb 8, 2023

⌛ Testing commit ba143bc3067c723344af9cdd305732d909f12597 with merge 788404bfb6f4f482a386e1306606a5fe59ed273f...

@bors
Copy link
Contributor

bors commented Feb 8, 2023

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 8, 2023
@cjgillot
Copy link
Contributor

cjgillot commented Feb 9, 2023

@bors retry curl: (6) Could not resolve host: ci-mirrors.rust-lang.org

@bors 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 9, 2023
@bors
Copy link
Contributor

bors commented Feb 9, 2023

⌛ Testing commit ba143bc3067c723344af9cdd305732d909f12597 with merge 9550d95507c1c9af656f32551702ab3e9f1b9d49...

@bors
Copy link
Contributor

bors commented Feb 10, 2023

💔 Test failed - checks-actions

@bors 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 Feb 10, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@michaelwoerister michaelwoerister removed their assignment Feb 10, 2023
@cjgillot 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 Feb 10, 2023
@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2023

📌 Commit 80d2652 has been approved by cjgillot

It is now in the queue for this repository.

@bors 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 Feb 12, 2023
@bors
Copy link
Contributor

bors commented Feb 12, 2023

⌛ Testing commit 80d2652 with merge 5b8f284...

@bors
Copy link
Contributor

bors commented Feb 12, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 12, 2023
@bors bors merged commit 5b8f284 into rust-lang:master Feb 12, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 12, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5b8f284): comparison URL.

Overall result: ❌✅ regressions and 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)
1.2% [1.2%, 1.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.2% [-4.2%, -4.2%] 1
All ❌✅ (primary) 1.2% [1.2%, 1.2%] 1

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

Cycles

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

@rustbot rustbot removed the perf-regression Performance regression. label Feb 12, 2023
@Zoxc Zoxc deleted the single-cache branch February 14, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants