Fix visible_parent_map fallback map merging perf regression - #160811
Fix visible_parent_map fallback map merging perf regression#160811zalanlevai wants to merge 1 commit into
Conversation
|
@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.
Fix visible_parent_map fallback map merging perf regression
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9cbccc7): 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 rustc-perf 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 -9.2%, secondary -0.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 0.2%)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: 458.036s -> 460.145s (0.46%) |
|
@JonathanBrouwer Thanks! I had a look through the perf run results and compared each benchmark that regressed in #160464 with the corresponding benchmark in this perf run. This change resolves all 7 perf regressions introduced in #160464, with the percentage deltas in this perf run cancelling out the ones in the perf run of the regressed PR. This change's perf run also shows 7 other perf regressions of similar magnitude: 5 are the |
|
I believe this is the correct label given the findings in my previous comment. @rustbot label +perf-regression-triaged |
This PR attempts to fix the
visible_parent_mapperf regression introduced in #160464.Because the original PR fixes unnecessary iterations in the breadth-first search (BFS), meaning that it only reduces the amount of work during the BFS, the likely cause of the perf regression is the changed merging of the fallback map into the final visible parent map.
The goal of this draft PR is to determine whether this is the case through a perf try run. The change itself works around the Unord* APIs, but might be worth it to work around the perf regression.