Skip to content

Conversation

nnethercote
Copy link
Contributor

r? @ghost

It's gotten on a hot path but only for use within `debug!`.
They both have a single use. (They can't be united, though, because
`self.colors` might change between the two `get` calls.)
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 3, 2025
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Oct 3, 2025
Avoid getting `dep_dep_node` unnecessarily.
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 3, 2025
Copy link
Member

@hkBst hkBst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change seems obviously good, since it avoids a function call on both Some branches of the match, which only need the result for debugging information.

I have no idea of the potential performance impact of this, but I'm curious to see the results of the perf run.

View changes since this review

@rust-bors
Copy link

rust-bors bot commented Oct 3, 2025

☀️ Try build successful (CI)
Build commit: 7b6fbc1 (7b6fbc1b2e10c887bd6f992a6f0525a844b1ac01, parent: 3b8665c5ab3aeced9b01672404c3764583e722ca)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7b6fbc1): comparison URL.

Overall result: ✅ 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.2% [0.2%, 0.2%] 2
Improvements ✅
(primary)
-1.4% [-3.5%, -0.2%] 110
Improvements ✅
(secondary)
-1.1% [-2.6%, -0.2%] 57
All ❌✅ (primary) -1.4% [-3.5%, -0.2%] 110

Max RSS (memory usage)

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

Cycles

Results (primary -2.8%, 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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.8% [-3.6%, -2.2%] 17
Improvements ✅
(secondary)
-2.7% [-3.4%, -2.0%] 2
All ❌✅ (primary) -2.8% [-3.6%, -2.2%] 17

Binary size

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

Bootstrap: 472.69s -> 471.95s (-0.16%)
Artifact size: 387.73 MiB -> 387.81 MiB (0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 3, 2025
@nnethercote nnethercote marked this pull request as ready for review October 3, 2025 20:53
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 3, 2025
@nnethercote
Copy link
Contributor Author

The icount results were almost twice as good as what I saw locally. And there are even some green cycle count results, which is nice, and uncommon for a micro-optimization like this.

There are a few double-digit wall-time regressions, but only among secondary benchmarks, and only among the really short-running ones, which appear to be prone to large swings with some frequency; the change is so simple that I think they can be ignored.

r? @saethlin

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) 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants