-
Notifications
You must be signed in to change notification settings - Fork 14k
rustdoc: split build_impl into build_{local,external}_impl #145907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
rustbot has assigned @GuillaumeGomez. Use |
|
@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.
rustdoc: split build_impl into build_{local,external}_impl
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (c00bc1f): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient 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 -1.4%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.2%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 469.036s -> 466.669s (-0.50%) |
|
because this is built off the previous performance PR, here's the comparison URL that should actually be used: https://perf.rust-lang.org/compare.html?start=05c0f818fe424341111ec6f6ccc79df099d0a142&end=c00bc1f8148ea75996f48ef92478ecb1e7e7c9ed&stat=instructions%3Au&opt=false&debug=false&check=false quite discouraging actually, and i'm not sure how to intuit this. you would think removing branches would speed things up, at least a little bit. |
|
Hard to compare times. The main comparison is how many instructions were run, so I guess improving branch prediction doesn't allow to do that. |
|
I mean, removing a branching instruction should save an instruction, in theory. but looking at i think what's happening here is the tiny bit of duplicated code at the start is getting inlined into actually a fairly significant amount of instructions, including a branch, and that's canceling out any improvements (i believe those old removed there might be something i can do to make this worth it, so i'll keep this open for now, but i'm not confident. |
|
☔ The latest upstream changes (presumably #138907) made this pull request unmergeable. Please resolve the merge conflicts. |
splitting these out should let us avoid quite a few more branches, and hopefully also improve cache locality of the code.
followup to #145851