Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jun 4, 2021

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 implement serde::Serialize.

Fixes #40177

See also rust-lang/compiler-team#418

@rust-highfive
Copy link
Contributor

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2021
@bjorn3 bjorn3 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 4, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 4, 2021
@bors
Copy link
Collaborator

bors commented Jun 4, 2021

⌛ Trying commit a5c45c2d6b6cb76b849ffc51be5de6c195c949c1 with merge 8b3e2e37129e9123c00dc4170c440c0c0b5dc51b...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@bors
Copy link
Collaborator

bors commented Jun 4, 2021

⌛ Trying commit b037d152bc6fc039a192cee277f094e968143ce4 with merge a0d810daa8d1883f9c5ce417226f96be2b28e2d7...

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2021

@bors try-

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2021

@bors r- retry

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 4, 2021
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2021

@rust-timer build a0d810daa8d1883f9c5ce417226f96be2b28e2d7

@rust-timer
Copy link
Collaborator

Queued a0d810daa8d1883f9c5ce417226f96be2b28e2d7 with parent 595088d, future comparison URL.

@jyn514
Copy link
Member

jyn514 commented Jun 4, 2021

cc rust-lang/compiler-team#418

@rust-timer
Copy link
Collaborator

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 rollup- to bors.

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 4, 2021
bjorn3 added 5 commits June 3, 2022 16:56
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
@bjorn3
Copy link
Member Author

bjorn3 commented Jun 3, 2022

Rebased

@bors r=wesleywiser

@bors
Copy link
Collaborator

bors commented Jun 3, 2022

📌 Commit 5cc3593 has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2022
@bors
Copy link
Collaborator

bors commented Jun 3, 2022

⌛ Testing commit 5cc3593 with merge 7e9b92c...

@bors
Copy link
Collaborator

bors commented Jun 3, 2022

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 7e9b92c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 3, 2022
@bors bors merged commit 7e9b92c into rust-lang:master Jun 3, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 3, 2022
@bjorn3 bjorn3 deleted the serde_json branch June 3, 2022 20:37
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7e9b92c): comparison url.

Instruction count

  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)
0.5% 1.3% 68
Regressions 😿
(secondary)
0.9% 3.5% 40
Improvements 🎉
(primary)
-0.4% -0.6% 3
Improvements 🎉
(secondary)
-0.5% -1.0% 24
All 😿🎉 (primary) 0.5% 1.3% 71

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
3.0% 3.1% 2
Regressions 😿
(secondary)
3.8% 5.0% 2
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 3.0% 3.1% 2

Cycles

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
2.4% 2.8% 2
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 2.4% 2.8% 2

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-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@nnethercote
Copy link
Contributor

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 process_obligations, I wonder if something happened there. Hopefully it's just noise and will shift back at some point?

mattheww added a commit to mattheww/rustc-dev-guide that referenced this pull request Feb 17, 2024
fmease pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Feb 17, 2024
Kobzol pushed a commit to Kobzol/rust that referenced this pull request Dec 30, 2024
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Jan 2, 2025
Kobzol pushed a commit to Kobzol/rustc-dev-guide that referenced this pull request Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate rustc from rustc-serialize to serde