Build the dep-graph reverse index lazily, per DepKind#157319
Conversation
This comment has been minimized.
This comment has been minimized.
c801cd6 to
20be77e
Compare
|
Thanks for the review! I addressed the comments |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Build the dep-graph reverse index lazily, per DepKind
This comment has been minimized.
This comment has been minimized.
|
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 @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
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.
CyclesResults (primary -2.5%, secondary -1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 511.14s -> 512.503s (0.27%) |
| nodes: &IndexSlice<SerializedDepNodeIndex, DepNode>, | ||
| nodes_by_kind: &[Option<SerializedDepNodeIndex>], | ||
| ) -> &UnhashMap<PackedFingerprint, SerializedDepNodeIndex> { | ||
| self.map.get_or_init(|| { |
There was a problem hiding this comment.
| 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.
There was a problem hiding this comment.
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
|
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 |
|
Thanks! |
62b140a to
b62dfa6
Compare
|
This pull request was unapproved. |
This comment has been minimized.
This comment has been minimized.
…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.
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #157319. |
This comment has been minimized.
This comment has been minimized.
Build the dep-graph reverse index lazily, per DepKind try-job: dist-i686-msvc
This comment has been minimized.
This comment has been minimized.
…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.
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #157726. |
This comment has been minimized.
This comment has been minimized.
…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.
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #157726. |
|
Try build cancelled. Cancelled workflows: |
This comment has been minimized.
This comment has been minimized.
Build the dep-graph reverse index lazily, per DepKind try-job: dist-i686-msvc
This comment has been minimized.
This comment has been minimized.
…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.
|
💔 Test for a4dd565 failed: CI. Failed jobs:
|
|
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) |
|
@bors retry |
|
💔 Test for 729af82 failed: CI. Failed job:
|
|
@bors try jobs=dist-i686-msvc |
|
⌛ Trying commit 0f7c98f with merge 853d73e… To cancel the try build, run the command Workflow: https://github.com/rust-lang/rust/actions/runs/27326398758 |
Build the dep-graph reverse index lazily, per DepKind try-job: dist-i686-msvc
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.