Sort symbol names by demangled name first to avoid different ordering on hash changes - #162003
Sort symbol names by demangled name first to avoid different ordering on hash changes#162003panstromek wants to merge 3 commits 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.
Sort symbol names by demangled name first to avoid different ordering on hash chanes
|
I'll do initial perf run, and then I'll try to push a few no-op changes and do perf on those (always using the same parent) to see how this affects artifact noise. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Interested in the perf. here, as this was quite hot on some benchmarks, IIRC (but it only showed in cycles). |
|
I believe this should mostly preserve the properties of the ordering you introduced in #145358. It should effectively just move the crate hash to the end. (unless I messed this up :D) Not sure about the demangled string and comparison perf, but at the moment I just want to diff this against future runs, so this should be fine for a prototype. |
|
Finished benchmarking commit (9b041cc): comparison URL. Overall result: ❌ regressions - 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 0.7%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.6%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 475.389s -> 477.502s (0.44%) |
|
Interesting, that's quite an impact. Anyway, let's try the no-op change. @bors try parent=9b041ccfacbdc9bb94384021e11a2192221a0718 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Sort symbol names by demangled name first to avoid different ordering on hash changes
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (fc7d1f2): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary -0.4%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.1%, 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: 477.502s -> 476.641s (-0.18%) |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors try parent=fc7d1f2ff50c0f99e7e96f8bdf291d9a4c5fdcd9 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Sort symbol names by demangled name first to avoid different ordering on hash changes
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d6cf21d): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression 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 0.5%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.2%, secondary 0.1%)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: 476.641s -> 482.1s (1.15%) |
View all comments
fixes #152910
For now just experimenting here to see whether this helps
rustc-perfnoise