std: use futex in `Once` #99505

joboet · 2022-07-20T10:07:44Z

Now that we have efficient locks, let's optimize the rest of sync as well. This PR adds a futex-based implementation for Once, which drastically simplifies the implementation compared to the generic version, which is provided as fallback for platforms without futex (Windows only supports them on newer versions, so it uses the fallback for now).

Instead of storing a linked list of waiters, the new implementation adds another state (QUEUED), which is set when there are waiting threads. These now use futex_wait on that state and are woken by the running thread when it finishes and notices the QUEUED state, thereby avoiding unnecessary calls to futex_wake_all.

rustbot · 2022-07-20T10:07:47Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

rust-highfive · 2022-07-20T10:07:47Z

r? @kennytm

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

joboet · 2022-07-20T14:24:32Z

I heard you have little time, @kennytm, please reassign if I'm wrong!

joboet · 2022-07-23T10:52:48Z

r? rust-lang/libs

Mark-Simulacrum · 2022-07-23T22:48:44Z

r? @m-ou-se

thomcc · 2022-09-27T16:07:17Z

r? @thomcc

library/std/src/sync/once.rs

joboet · 2022-09-28T23:05:33Z

library/std/src/sys_common/once/generic.rs

+    // This is a non-generic function to reduce the monomorphization cost of
+    // using `call_once` (this isn't exactly a trivial or small implementation).


I've copied this from the old implementation, but I'm not sure whether it's really necessary?

joboet · 2022-09-28T23:07:43Z

There was a bug in the futex implementation which I've now resolved (changing the state from RUNNING to QUEUED did not update the local state variable). I added a small optimization to the generic implementation as well, eliminating one load. (In hindsight, maybe that should have been a second commit. Oh well...)

thomcc · 2022-09-29T08:10:14Z

Is generic.rs equivalent to the old code, or does it need close scrutiny?

joboet · 2022-09-29T09:49:27Z

It's basically the same, except for some style changes (using convenience functions for masking the pointers, making the loop in wait smaller) and the usage of UnsafeCell<ManuallyDrop<Thread>> instead of Cell<Option<Thread>> in Waiter (since using Cell in non-sync environments is unidiomatic).

thomcc · 2022-09-30T21:00:06Z

Can you put the changes into a separate commit from the file move? I really don't want to review the entire generic.rs, especially given that the current impl has been proven correct (IIRC).

thomcc

Not done reviewing futex.rs, only nits so far, but I have to run for a bit.

A bit of a meta-comment though, does this actually improve perf here? Our current Once code is actually quite good IMO.

library/std/src/sys_common/once/futex.rs

joboet · 2022-10-01T07:29:06Z

Cool, this really looks like a small but noticeable improvement (println! uses OnceLock so it makes sense that that benchmark should improve)!

thomcc

Thank you, and sorry for the delay (didn't want to halfass the review of a synchronization primitive). The new impl is very clean, it sure is nice when we don't have to manage the linked list ourselves...

thomcc · 2022-10-06T23:44:21Z

@bors r+ rollup=never

bors · 2022-10-06T23:44:23Z

📌 Commit 66278fd73072e0c449a245efcf22d48485d689c7 has been approved by thomcc

It is now in the queue for this repository.

joboet · 2022-10-07T00:31:49Z

@bors r-
I want to squash that revert commit first...

joboet · 2022-10-07T10:14:57Z

@rustbot ready
@thomcc done 🎉

thomcc · 2022-10-08T03:14:17Z

@bors r+

bors · 2022-10-08T03:14:19Z

📌 Commit 5d0211d has been approved by thomcc

It is now in the queue for this repository.

bors · 2022-10-08T03:50:10Z

⌛ Testing commit 5d0211d with merge a688a03...

bors · 2022-10-08T06:46:53Z

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing a688a03 to master...

rust-timer · 2022-10-08T08:07:46Z

Finished benchmarking commit (a688a03): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	0.5%	[0.5%, 0.5%]	1
Regressions ❌ (secondary)	1.7%	[1.0%, 3.3%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	9
All ❌✅ (primary)	0.5%	[0.5%, 0.5%]	1

Max RSS (memory usage)

Results

This 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.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.1%	[1.1%, 2.8%]	4
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.6%]	1
Improvements ✅ (secondary)	-4.4%	[-5.6%, -2.6%]	3
All ❌✅ (primary)	-0.6%	[-0.6%, -0.6%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

rylev · 2022-10-11T15:34:46Z

Looks like this ended up being a small regression. I think the regression results are small and neutral enough that we don't need to investigate.

@rustbot labels: perf-regression-triaged

rust-highfive assigned kennytm Jul 20, 2022

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 20, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2022

This comment has been minimized.

Sign in to view

joboet force-pushed the futex_once branch from 11f4c3c to b9007ad Compare July 20, 2022 10:18

This comment has been minimized.

Sign in to view

joboet force-pushed the futex_once branch from b9007ad to c13f8db Compare July 20, 2022 10:36

rust-highfive assigned Mark-Simulacrum and unassigned kennytm Jul 23, 2022

joboet force-pushed the futex_once branch from c13f8db to 5e0df99 Compare July 23, 2022 12:19

rust-highfive assigned m-ou-se and unassigned Mark-Simulacrum Jul 23, 2022

joboet force-pushed the futex_once branch 2 times, most recently from c79c240 to 198e05b Compare September 7, 2022 17:12

rust-highfive assigned thomcc and unassigned m-ou-se Sep 27, 2022

thomcc reviewed Sep 28, 2022

View reviewed changes

library/std/src/sync/once.rs Outdated Show resolved Hide resolved

joboet force-pushed the futex_once branch from 198e05b to 54dd3e2 Compare September 28, 2022 23:03

joboet commented Sep 28, 2022

View reviewed changes

thomcc reviewed Sep 30, 2022

View reviewed changes

library/std/src/sys_common/once/futex.rs Outdated Show resolved Hide resolved

library/std/src/sys_common/once/futex.rs Outdated Show resolved Hide resolved

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 1, 2022

thomcc approved these changes Oct 6, 2022

View reviewed changes

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 Oct 6, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 7, 2022

std: use futex in Once

5d0211d

joboet force-pushed the futex_once branch from 66278fd to 5d0211d Compare October 7, 2022 10:13

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2022

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 Oct 8, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 8, 2022

bors merged commit a688a03 into rust-lang:master Oct 8, 2022

rustbot added this to the 1.66.0 milestone Oct 8, 2022

joboet deleted the futex_once branch October 8, 2022 07:13

rustbot added the perf-regression Performance regression. label Oct 8, 2022

rustbot added the perf-regression-triaged The performance regression has been triaged. label Oct 11, 2022

workingjubilee mentioned this pull request Jun 3, 2024

Windows: Use futex implementation for Once #125942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

std: use futex in `Once` #99505

std: use futex in `Once` #99505

joboet commented Jul 20, 2022 •

edited

Loading

rustbot commented Jul 20, 2022

rust-highfive commented Jul 20, 2022

This comment has been minimized.

This comment has been minimized.

joboet commented Jul 20, 2022 •

edited

Loading

joboet commented Jul 23, 2022

Mark-Simulacrum commented Jul 23, 2022

thomcc commented Sep 27, 2022

joboet Sep 28, 2022 •

edited

Loading

joboet commented Sep 28, 2022 •

edited

Loading

thomcc commented Sep 29, 2022 •

edited

Loading

joboet commented Sep 29, 2022

thomcc commented Sep 30, 2022 •

edited

Loading

thomcc left a comment

joboet commented Oct 1, 2022

thomcc left a comment

thomcc commented Oct 6, 2022

bors commented Oct 6, 2022

joboet commented Oct 7, 2022

joboet commented Oct 7, 2022

thomcc commented Oct 8, 2022

bors commented Oct 8, 2022

bors commented Oct 8, 2022

bors commented Oct 8, 2022

rust-timer commented Oct 8, 2022

rylev commented Oct 11, 2022

		// This is a non-generic function to reduce the monomorphization cost of
		// using `call_once` (this isn't exactly a trivial or small implementation).

std: use futex in Once #99505

std: use futex in Once #99505

Conversation

joboet commented Jul 20, 2022 • edited Loading

rustbot commented Jul 20, 2022

rust-highfive commented Jul 20, 2022

This comment has been minimized.

This comment has been minimized.

joboet commented Jul 20, 2022 • edited Loading

joboet commented Jul 23, 2022

Mark-Simulacrum commented Jul 23, 2022

thomcc commented Sep 27, 2022

joboet Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

joboet commented Sep 28, 2022 • edited Loading

thomcc commented Sep 29, 2022 • edited Loading

joboet commented Sep 29, 2022

thomcc commented Sep 30, 2022 • edited Loading

thomcc left a comment

Choose a reason for hiding this comment

joboet commented Oct 1, 2022

thomcc left a comment

Choose a reason for hiding this comment

thomcc commented Oct 6, 2022

bors commented Oct 6, 2022

joboet commented Oct 7, 2022

joboet commented Oct 7, 2022

thomcc commented Oct 8, 2022

bors commented Oct 8, 2022

bors commented Oct 8, 2022

bors commented Oct 8, 2022

rust-timer commented Oct 8, 2022

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

rylev commented Oct 11, 2022

std: use futex in `Once` #99505

std: use futex in `Once` #99505

joboet commented Jul 20, 2022 •

edited

Loading

joboet commented Jul 20, 2022 •

edited

Loading

joboet Sep 28, 2022 •

edited

Loading

joboet commented Sep 28, 2022 •

edited

Loading

thomcc commented Sep 29, 2022 •

edited

Loading

thomcc commented Sep 30, 2022 •

edited

Loading