-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 lock wrappers in sys_common
#103150
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
5186a11
to
e9ceb57
Compare
Nice! This is great. I haven't finished reviewing the diff yet, but I'm really happy to finally get the lock wrappers in sys_common removed. |
Thank you! @bors r+ rollup=never |
📌 Commit d57bb300e78058bc02965f582a4cb3acd7a48d31 has been approved by It is now in the queue for this repository. |
@bors r- |
4362c6d
to
168f1f1
Compare
@bors r+ |
This comment has been minimized.
This comment has been minimized.
7bc8272
to
3e35cfb
Compare
@m-ou-se Could you try again? |
@bors r+ |
📌 Commit 3e35cfb4b9f08090ed9959f3337cae53b39de32c has been approved by It is now in the queue for this repository. |
⌛ Testing commit 3e35cfb4b9f08090ed9959f3337cae53b39de32c with merge dd7fc2725173977b5a742999700146c50530b659... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
3e35cfb
to
f30614a
Compare
Sorry for taking up so much time; I didn't read the error messages carefully enough (twice 🤦). |
No worries. :) @bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (b0c6527): 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.
|
Remove lock wrappers in `sys_common` This moves the lazy allocation to `sys` (SGX and UNIX). While this leads to a bit more verbosity, it will simplify future improvements by making room in `sys_common` for platform-independent implementations. This also removes the condvar check on SGX as it is not necessary for soundness and will be removed anyway once mutex has been made movable. For simplicity's sake, `libunwind` also uses lazy allocation now on SGX. This will require an update to the C definitions before merging this (CC `@raoulstrackx).` r? `@m-ou-se`
This moves the lazy allocation to
sys
(SGX and UNIX). While this leads to a bit more verbosity, it will simplify future improvements by making room insys_common
for platform-independent implementations.This also removes the condvar check on SGX as it is not necessary for soundness and will be removed anyway once mutex has been made movable.
For simplicity's sake,
libunwind
also uses lazy allocation now on SGX. This will require an update to the C definitions before merging this (CC @raoulstrackx).r? @m-ou-se