Skip to content

Build the dep-graph reverse index lazily, per DepKind#157319

Open
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:perf/lazy-depkind-reverse-index
Open

Build the dep-graph reverse index lazily, per DepKind#157319
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:perf/lazy-depkind-reverse-index

Conversation

@xmakro

@xmakro xmakro commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

View all comments

Replace the eager per-DepKind fingerprint-to-index map built at decode with a counting sort into per-kind ranges plus a lazily-built map per kind.

@rustbot rustbot added 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 Jun 2, 2026
@rust-log-analyzer

This comment has been minimized.

@cjgillot cjgillot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea. Could you fix tidy so we can test and perf it?

View changes since this review

Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
@cjgillot cjgillot self-assigned this Jun 3, 2026
@xmakro xmakro force-pushed the perf/lazy-depkind-reverse-index branch from c801cd6 to 20be77e Compare June 3, 2026 21:52
@xmakro

xmakro commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I addressed the comments

@xmakro xmakro marked this pull request as ready for review June 3, 2026 23:20
@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 Jun 3, 2026
@cjgillot

cjgillot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@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 Jun 4, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 4, 2026
Build the dep-graph reverse index lazily, per DepKind
@rust-bors

rust-bors Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 4754d42 (4754d42b1ab5ad44b221d4c4a7b09bee66237a16, parent: 49b19d32b9f01a5aa606f3bf2e90e6e0aa462c03)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (4754d42): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@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.1% [0.0%, 0.3%] 7
Improvements ✅
(primary)
-0.4% [-0.7%, -0.2%] 50
Improvements ✅
(secondary)
-0.7% [-2.7%, -0.0%] 40
All ❌✅ (primary) -0.4% [-0.7%, -0.2%] 50

Max RSS (memory usage)

Results (primary -2.3%, secondary -1.9%)

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

mean range count
Regressions ❌
(primary)
1.1% [0.7%, 1.8%] 3
Regressions ❌
(secondary)
4.0% [2.1%, 7.7%] 4
Improvements ✅
(primary)
-2.6% [-5.1%, -0.8%] 37
Improvements ✅
(secondary)
-3.1% [-4.9%, -1.8%] 20
All ❌✅ (primary) -2.3% [-5.1%, 1.8%] 40

Cycles

Results (primary -2.5%, secondary -1.8%)

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.0% [3.9%, 5.6%] 3
Improvements ✅
(primary)
-2.5% [-3.1%, -2.2%] 3
Improvements ✅
(secondary)
-4.3% [-6.7%, -3.3%] 8
All ❌✅ (primary) -2.5% [-3.1%, -2.2%] 3

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 511.14s -> 512.503s (0.27%)
Artifact size: 398.59 MiB -> 400.66 MiB (0.52%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 4, 2026

@cjgillot cjgillot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perf is great, the implementation looks good. A few nits on comments.
r=me afterwards

View changes since this review

Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs Outdated
Comment thread compiler/rustc_middle/src/dep_graph/serialized.rs
nodes: &IndexSlice<SerializedDepNodeIndex, DepNode>,
nodes_by_kind: &[Option<SerializedDepNodeIndex>],
) -> &UnhashMap<PackedFingerprint, SerializedDepNodeIndex> {
self.map.get_or_init(|| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.map.get_or_init(|| {
self.map.get_or_init(|| {
let _prof_timer = self.profiler.generic_activity("incr_comp_load_dep_graph_reverse_index");

Let's add a timer to for detailed perf output. This will make easier to understand why regression happen in some queries.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added the timer in fingerprint_map but I put the profiler on SerializedDepGraph rather than on LazyKindIndex, since there's one of those per DepKind and it would be a lot of clones. Annoying is that this needs a manual Debug impl. Please let me know if you see a better way

@xmakro

xmakro commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! I made a separate commit so it is easier to review. If it looks good, I would squash the commits into one before the merge

@cjgillot

cjgillot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Thanks!
@bors r+

@rust-bors

rust-bors Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 62b140a has been approved by cjgillot

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Jun 5, 2026
@xmakro xmakro force-pushed the perf/lazy-depkind-reverse-index branch from 62b140a to b62dfa6 Compare June 5, 2026 11:24
@rust-bors rust-bors Bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 5, 2026
@rust-bors

rust-bors Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ A new commit b62dfa6c25ac1ba30a086f36c464959c5bfe448b was pushed.

This pull request was unapproved.

@rust-bors rust-bors Bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 5, 2026
@JonathanBrouwer

Copy link
Copy Markdown
Contributor

@bors retry
@bors treeclosed=5
GitHub is still super unstable, some builds are succeeding but I'd rather just keep the tree closed while things are like this

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 10, 2026
…illot

Build the dep-graph reverse index lazily, per DepKind



Replace the eager per-DepKind fingerprint-to-index map built at decode with a counting sort into per-kind ranges plus a lazily-built map per kind.
@jhpratt

jhpratt commented Jun 10, 2026

Copy link
Copy Markdown
Member

stalled

@bors yield

@bors try jobs=dist-i686-msvc

@rust-bors

rust-bors Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #157319.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 10, 2026
Build the dep-graph reverse index lazily, per DepKind


try-job: dist-i686-msvc
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 10, 2026
…illot

Build the dep-graph reverse index lazily, per DepKind



Replace the eager per-DepKind fingerprint-to-index map built at decode with a counting sort into per-kind ranges plus a lazily-built map per kind.
@jhpratt

jhpratt commented Jun 10, 2026

Copy link
Copy Markdown
Member

@bors p=4

@bors yield

@rust-bors

rust-bors Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #157726.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 10, 2026
…illot

Build the dep-graph reverse index lazily, per DepKind



Replace the eager per-DepKind fingerprint-to-index map built at decode with a counting sort into per-kind ranges plus a lazily-built map per kind.
@jhpratt

jhpratt commented Jun 11, 2026

Copy link
Copy Markdown
Member

stalled yet again, including the try job

@bors yield

@bors try cancel

@bors try jobs=dist-i686-msvc

@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #157726.

@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Try build cancelled. Cancelled workflows:

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 11, 2026
Build the dep-graph reverse index lazily, per DepKind


try-job: dist-i686-msvc
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 11, 2026
…illot

Build the dep-graph reverse index lazily, per DepKind



Replace the eager per-DepKind fingerprint-to-index map built at decode with a counting sort into per-kind ranges plus a lazily-built map per kind.
@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

💔 Test for a4dd565 failed: CI. Failed jobs:

@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

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

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

@jhpratt

jhpratt commented Jun 11, 2026

Copy link
Copy Markdown
Member

@bors retry

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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 Jun 11, 2026
@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

💔 Test for 729af82 failed: CI. Failed job:

@jieyouxu

Copy link
Copy Markdown
Member

@bors try jobs=dist-i686-msvc

@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

⌛ Trying commit 0f7c98f with merge 853d73e

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/27326398758

rust-bors Bot pushed a commit that referenced this pull request Jun 11, 2026
Build the dep-graph reverse index lazily, per DepKind


try-job: dist-i686-msvc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

8 participants