-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Replace rustc_span::Span
with a stripped down version for librustdoc's highlighter
#147189
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: master
Are you sure you want to change the base?
Replace rustc_span::Span
with a stripped down version for librustdoc's highlighter
#147189
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.
…2, r=<try> [PERF] Use small `Span`s in rustdoc's highlighter
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (7b69a13): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @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.8%, secondary -7.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.9%, secondary -11.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 470.437s -> 471.961s (0.32%) |
a2250d0
to
2d03ab1
Compare
r? @notriddle rustbot has assigned @notriddle. Use |
Span
s in rustdoc's highlighterrustc_span::Span
with a stripped down version for librustdoc's highlighter
/// never actually use the context and parent that are stored in a normal `Span`, we can replace its usages with this | ||
/// one, which is much cheaper to construct. | ||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
pub(crate) struct Span { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't really sure where to put this..
The job Click to see the possible cause of the failure (guessed by this bot)
|
This error make it seems like a spurious network error:
|
✌️ @yotamofek, you can now approve this pull request! If @GuillaumeGomez told you to " |
…imizations-2, r=GuillaumeGomez Replace `rustc_span::Span` with a stripped down version for librustdoc's highlighter While profiling rustdoc's syntax highlighter, I noticed a lot of time being spent in the `Span` interner, due to the highlighter creating a lot of (new) spans. Since the only data from the `Span` that we use is the `hi` and `lo` byte positions - I replaced the regular `Span` with a simple one with two fields, and in my benchmarks it seemed to make a big dent in the highlighter's perf, so thought I would see what the perf runner says.
Rollup of 11 pull requests Successful merges: - #146918 (add regression test) - #146980 (simplify setup_constraining_predicates, and note it is potentially cubic) - #147170 (compiletest: Pass around `DirectiveLine` instead of bare strings) - #147180 (add tests) - #147188 (Remove usage of `compiletest-use-stage0-libtest` from CI) - #147189 (Replace `rustc_span::Span` with a stripped down version for librustdoc's highlighter) - #147199 (remove outdated comment in (inner) `InferCtxt`) - #147200 (Fix autodiff empty ret regression) - #147209 (Remove `no-remap-src-base` from tests) - #147213 (Fix broken STD build for ESP-IDF) - #147217 (Don't create a top-level `true` directory when running UI tests) r? `@ghost` `@rustbot` modify labels: rollup
While profiling rustdoc's syntax highlighter, I noticed a lot of time being spent in the
Span
interner, due to the highlighter creating a lot of (new) spans.Since the only data from the
Span
that we use is thehi
andlo
byte positions - I replaced the regularSpan
with a simple one with two fields, and in my benchmarks it seemed to make a big dent in the highlighter's perf, so thought I would see what the perf runner says.