std: use sync::RwLock for internal statics#100581
Conversation
|
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
I'm not a fan of making this change (or the one in #100579). Can we instead change StaticMutex/StaticRwlock to use the locks internally on platforms where that impl isn't worse? |
|
It looks like we use this for the environment lock, and for the panic hook. I don't really feel like either of these are likely to be hot enough for the extra allocation to matter, so I'm not sure how much I actually care here now that I look. r? @thomcc |
|
This looks fine. This does actually introduce a branch even on OSes with the good locks (due to the poison error handling). I doubt it matters but is probably worth preventing a rollup as a result. @bors r+ rollup=never |
|
📌 Commit 25cc0ce3c3eed6278bdeed9eddca8dc43ea55002 has been approved by It is now in the queue for this repository. |
|
⌛ Testing commit 25cc0ce3c3eed6278bdeed9eddca8dc43ea55002 with merge 0f66914ea522ad0920154b14127447512a361cf0... |
|
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
@rustbot author |
25cc0ce to
be09a4a
Compare
|
@rustbot ready |
|
@bors r+ |
|
And for good measure, @bors delegate+ |
|
✌️ @joboet can now approve this pull request |
|
⌛ Testing commit be09a4a with merge 4456640f3e841eacc29fc3ecd88718c03bd0fcce... |
|
💔 Test failed - checks-actions |
|
That looks like a spurious failure... |
|
💡 This pull request was already approved, no need to approve it again.
|
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (7743aa8): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
Since
sync::RwLockis nowconst-constructible, it can be used for internal statics, removing the need forsys_common::StaticRwLock. This adds some extra allocations on platforms which need to box their locks (currently SGX and some UNIX), but these will become unnecessary with the lock improvements tracked in #93740.