-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Remove all json handling from rustc_serialize #85993
Conversation
r? @varkor (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit a5c45c2d6b6cb76b849ffc51be5de6c195c949c1 with merge 8b3e2e37129e9123c00dc4170c440c0c0b5dc51b... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit b037d152bc6fc039a192cee277f094e968143ce4 with merge a0d810daa8d1883f9c5ce417226f96be2b28e2d7... |
This comment has been minimized.
This comment has been minimized.
@bors try- |
@bors r- retry |
This comment has been minimized.
This comment has been minimized.
@rust-timer build a0d810daa8d1883f9c5ce417226f96be2b28e2d7 |
Queued a0d810daa8d1883f9c5ce417226f96be2b28e2d7 with parent 595088d, future comparison URL. |
Finished benchmarking try commit (a0d810daa8d1883f9c5ce417226f96be2b28e2d7): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
They aren't used anymore now that the json format has been removed
They aren't overridden anyway
It doesn't do anything for all encoders
Rebased @bors r=wesleywiser |
📌 Commit 5cc3593 has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (7e9b92c): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Footnotes |
I was unable to reproduce the perf regressions in a local build. Some of the benchmarks in question (e.g. cranelift-codegen-0.82.1, keccak) are sensitive to small changes in codegen in and around |
These were removed in 2021. rust-lang/rust#85993
These were removed in 2021. rust-lang/rust#85993
Json is now handled using serde_json. Where appropriate I have replaced json usage with binary serialization (rmeta files) or manual string formatting (emcc linker arg generation).
This allowed for removing and simplifying a lot of code, which hopefully results in faster serialization/deserialization and faster compiles of rustc itself.
Where sensible we now use serde. Metadata and incr cache serialization keeps using a heavily modified (compared to crates.io) rustc-serialize version that in the future could probably be extended with zero-copy deserialization or other perf tricks that serde can't support due to supporting more than one serialization format.
Note that I had to remove
-Zast-json
and-Zast-json-noexpand
as the relevant AST types don't implementserde::Serialize
.Fixes #40177
See also rust-lang/compiler-team#418