-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Allow disabling LLVM assertions in rustc (fixes #15548) #15559
Conversation
Heads up! This PR modifies the following files:
|
7f8b429
to
09d88cb
Compare
I got some numbers on Servo commit 09d88cb with Rust commit rust-lang/rust@025c328, on a fast desktop. My config has incremental compilation and
Observations:
|
This PR doesn’t change the default config, so unless you tweak @bors-servo r+ |
📌 Commit 09d88cb has been approved by |
Allow disabling LLVM assertions in rustc (fixes #15548) <!-- Reviewable:start --> This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15559) <!-- Reviewable:end -->
💔 Test failed - linux-dev |
|
@bors-servo r+ |
📌 Commit 57fd45a has been approved by |
Allow disabling LLVM assertions in rustc (fixes #15548) <!-- Reviewable:start --> This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15559) <!-- Reviewable:end -->
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev |
Do not merge yet: Disable LLVM assertions by default <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15564) <!-- Reviewable:end -->
Do not merge yet: Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15564) <!-- Reviewable:end -->
Do not merge yet: Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15564) <!-- Reviewable:end -->
Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15564) <!-- Reviewable:end -->
Conditional compilation means we tend to compile less code in release mode (due to assertions and logging being omitted), though I'm surprised it makes this much difference.
This probably means that most of the assertions are in code that is only run during release builds, so enabling assertions during debug builds will be of much more limited use for finding LLVM bugs. |
…d platforms (from servo:no-gods-no-masters-no-assertions); r=Ms2ger <!-- Please describe your changes on the following line: --> servo/servo#15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 1afae52c47e754c6573f4a8b72fcc2e6994d253f --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 7cbc302040099b6b6d0b09d6322b618b26ba61ac
…d platforms (from servo:no-gods-no-masters-no-assertions); r=Ms2ger <!-- Please describe your changes on the following line: --> servo/servo#15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 1afae52c47e754c6573f4a8b72fcc2e6994d253f
…d platforms (from servo:no-gods-no-masters-no-assertions); r=Ms2ger <!-- Please describe your changes on the following line: --> servo/servo#15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 1afae52c47e754c6573f4a8b72fcc2e6994d253f
…d platforms (from servo:no-gods-no-masters-no-assertions); r=Ms2ger <!-- Please describe your changes on the following line: --> servo/servo#15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 1afae52c47e754c6573f4a8b72fcc2e6994d253f
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)