-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[rustdoc] Don't serialize & deserialize data that doesn't go OTW #147402
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
[rustdoc] Don't serialize & deserialize data that doesn't go OTW #147402
Conversation
entry_data: Option<EntryData>, | ||
desc: String, | ||
function_data: Option<FunctionData>, | ||
function_data: Option<IndexItemFunctionType>, |
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.
this is the (hopefully) impactful part: FunctionData
is an OTW representation of IndexItemFunctionType
, and there's no need to do the writing and parsing part until we leave the boundaries of the current process invocation
first commit is just a cleanup (IMHO, probably a matter of taste): 15c5e34 |
cc @notriddle as well. Starting perf check. :) @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.
…ay, r=<try> [PERF] [rustdoc] Try to speed up search index building
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (53439ce): 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 -0.5%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.6%)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.924s -> 473.064s (0.45%) |
5ea7e13
to
3407d8b
Compare
Results are nice, but let's see what they're like without 15c5e34 |
@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.
…ay, r=<try> [PERF] [rustdoc] Try to speed up search index building
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (de083db): 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 1.5%, secondary 1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.2%)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: 472.01s -> 472.099s (0.02%) |
Not as much as I would have hoped but still visible positive impact, so looks good to me. :) |
3407d8b
to
c5ab530
Compare
@GuillaumeGomez thanks! Added two commits. First one (978ab7c) is to make the manual serde impls for
Second one (c5ab530) moves those serde impls out into a separate module. Let me know what you think and I'll squash and rewrite the commit message with whatever changes we end up keeping. |
rustbot has assigned @GuillaumeGomez. Use |
✌️ @yotamofek, you can now approve this pull request! If @GuillaumeGomez told you to " |
Hum, my rollup instruction was ignored apparently. Let's try again. @bors rollup=maybe |
Rollup of 7 pull requests Successful merges: - #145943 (stdlib docs: document lifetime extension for `format_args!`'s arguments) - #147243 (cmse: disallow `impl Trait` in `cmse-nonsecure-entry` return types) - #147402 ([rustdoc] Don't serialize & deserialize data that doesn't go OTW) - #147418 (Fix target list of `link_section`) - #147429 (Print tip for human error format in runtest) - #147441 (Fix comments error for Provenance impls) - #147442 (c-variadic: fix thir-print for `...` without a pattern) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #147402 - yotamofek:wip/rustdoc/search_index/impl-display, r=GuillaumeGomez [rustdoc] Don't serialize & deserialize data that doesn't go OTW `@GuillaumeGomez`
@GuillaumeGomez