-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Change Backtrace::enabled atomic from SeqCst to Relaxed #92139
Conversation
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
@bors r+ |
📌 Commit 984b10d has been approved by |
⌛ Testing commit 984b10d with merge b0dcf2c76b3159084138c63b18bd31373c9e679b... |
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@@ -268,7 +268,7 @@ impl Backtrace { | |||
Err(_) => false, | |||
}, | |||
}; | |||
ENABLED.store(enabled as usize + 1, SeqCst); | |||
ENABLED.store(enabled as usize + 1, Relaxed); |
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.
How about storing boolean as is in atomic?
I managed to remove 1 branch and make code shorter by this:
godbolt
The CI error was due to some infrastructure problem. I would retry, but do you want to respond to the comment at #92139 (comment) first? |
That comment seems orthogonal to the change in this PR. @bors retry |
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#88858 (Allow reverse iteration of lowercase'd/uppercase'd chars) - rust-lang#91544 (Fix duplicate derive clone suggestion) - rust-lang#92026 (Add some JSDoc comments to rustdoc JS) - rust-lang#92117 (kmc-solid: Add `std::sys::solid::fs::File::read_buf`) - rust-lang#92139 (Change Backtrace::enabled atomic from SeqCst to Relaxed) - rust-lang#92146 (Don't emit shared files when scraping examples from dependencies in Rustdoc) - rust-lang#92208 (Quote bat script command line) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This atomic is not synchronizing anything outside of its own value, so we don't need the
Acquire
/Release
guarantee that all memory operations prior to the store are visible after the subsequent load, nor theSeqCst
guarantee of all threads seeing all of the sequentially consistent operations in the same order.Using
Relaxed
reduces the overhead ofBacktrace::capture()
in the case that backtraces are not enabled.Benchmark
Before: 6.73 seconds
After: 5.18 seconds