Optimize thread parking on NetBSD #101482

joboet · 2022-09-06T12:49:12Z

As the futex syscall is not present in the latest stable release, NetBSD cannot use the efficient thread parker and locks Linux uses. Currently, it therefore relies on a pthread-based parker, consisting of a mutex and semaphore which protect a state variable. NetBSD however has more efficient syscalls available: _lwp_park and _lwp_unpark. These already provide the exact semantics of thread::park and Thread::unpark, but work with thread ids. In std, this ID is here stored in an atomic state variable, which is also used to optimize cases were the parking token is already available at the time thread::park is called.

r? @m-ou-se

rustbot · 2022-09-06T12:49:16Z

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

m-ou-se · 2022-09-06T16:05:57Z

library/std/src/sys/unix/thread_parker/netbsd.rs

+extern "C" {
+    fn _lwp_park(
+        clock_id: clockid_t,
+        flags: c_int,
+        ts: *mut timespec,
+        unpark: lwpid_t,
+        hint: *const c_void,
+        unparkhint: *const c_void,
+    ) -> c_int;
+    fn _lwp_unpark(lwp: lwpid_t, hint: *const c_void) -> c_int;
+}


It might be good to add these to the libc crate.

The syscall behaviour can change slightly with newer versions (e.g. the ts argument is only modified on NetBSD 9.0 and above, while it is still the same symbol), so it might not necessarily be a good idea to advertise these functions in libc.

I don't think that matters much for inclusion in the libc crate. It contains many symbols that aren't fully "stable" or only exist on certain versions of the operating system or underlying libc.

Anyway, this is not a blocker for this PR.

library/std/src/sys/unix/thread_parker/netbsd.rs

m-ou-se · 2022-09-06T16:22:49Z

If I understand the documentation correctly, it seems best to pass the address of state to the park and unpark syscalls as the hint argument?

library/std/src/sys/unix/thread_parker/netbsd.rs

m-ou-se · 2022-09-06T19:17:15Z

@bors r+

bors · 2022-09-06T19:17:17Z

📌 Commit e41555848ed70cc76d4f17387150a108bdea580d has been approved by m-ou-se

It is now in the queue for this repository.

joboet · 2022-09-06T22:43:44Z

@bors r-
The symbol for _lwp_park is actually ___lwp_park60, so I'm not sure this even compiles properly. Anyways, this should definitely be tested; sorry for not clarifying that I have not yet done so.

joboet · 2022-09-07T13:30:01Z

I've now confirmed that this works on NetBSD 9.3 (x86_64 with QEMU). ___lwp_park60 is indeed the right symbol, as the regular _lwp_park uses the old interface from NetBSD 5.0.

@rustbot ready

sanxiyn · 2022-09-10T11:50:18Z

@bors r+

bors · 2022-09-10T11:50:20Z

📌 Commit adfe1b171e178e436a3f48ae9774461b5d929d3d has been approved by sanxiyn

It is now in the queue for this repository.

bors · 2022-09-10T17:29:45Z

⌛ Testing commit adfe1b171e178e436a3f48ae9774461b5d929d3d with merge 9da77f41d2fc05613b5cb869d0ebf63c2b64d653...

bors · 2022-09-10T17:39:11Z

💔 Test failed - checks-actions

sanxiyn · 2022-09-11T02:16:15Z

@bors r+

bors · 2022-09-11T02:16:17Z

📌 Commit 81b11ed has been approved by sanxiyn

It is now in the queue for this repository.

bors · 2022-09-11T04:07:20Z

⌛ Testing commit 81b11ed with merge 56b625b...

bors · 2022-09-11T06:27:23Z

☀️ Test successful - checks-actions
Approved by: sanxiyn
Pushing 56b625b to master...

rust-timer · 2022-09-11T07:43:52Z

Finished benchmarking commit (56b625b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-	-	0

Cycles

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)	7.1%	[7.1%, 7.1%]	1
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.6%	[-1.6%, -1.6%]	1

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

rust-highfive assigned m-ou-se Sep 6, 2022

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

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

joboet force-pushed the netbsd_parker branch from 9657fe0 to b9eb476 Compare September 6, 2022 12:56

m-ou-se reviewed Sep 6, 2022

View reviewed changes

library/std/src/sys/unix/thread_parker/netbsd.rs Outdated Show resolved Hide resolved

m-ou-se reviewed Sep 6, 2022

View reviewed changes

library/std/src/sys/unix/thread_parker/netbsd.rs Outdated Show resolved Hide resolved

joboet force-pushed the netbsd_parker branch from b9eb476 to e415558 Compare September 6, 2022 16:40

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

joboet force-pushed the netbsd_parker branch from 9831f34 to adfe1b1 Compare September 7, 2022 13:26

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 Sep 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 Sep 10, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 10, 2022

This comment has been minimized.

Sign in to view

std: optimize thread parking on NetBSD

81b11ed

joboet force-pushed the netbsd_parker branch from adfe1b1 to 81b11ed Compare September 10, 2022 18:03

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 Sep 11, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 11, 2022

bors merged commit 56b625b into rust-lang:master Sep 11, 2022

rustbot added this to the 1.65.0 milestone Sep 11, 2022

bors mentioned this pull request Sep 11, 2022

Use correct clock in park_timeout on Horizon #100539

Merged

joboet deleted the netbsd_parker branch February 18, 2023 00:28

workingjubilee added the O-netbsd Operating system: NetBSD label Feb 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize thread parking on NetBSD #101482

Optimize thread parking on NetBSD #101482

joboet commented Sep 6, 2022

rustbot commented Sep 6, 2022

m-ou-se Sep 6, 2022

joboet Sep 6, 2022

m-ou-se Sep 6, 2022

m-ou-se commented Sep 6, 2022

m-ou-se commented Sep 6, 2022

bors commented Sep 6, 2022

joboet commented Sep 6, 2022

joboet commented Sep 7, 2022

sanxiyn commented Sep 10, 2022

bors commented Sep 10, 2022

bors commented Sep 10, 2022

bors commented Sep 10, 2022

This comment has been minimized.

sanxiyn commented Sep 11, 2022

bors commented Sep 11, 2022

bors commented Sep 11, 2022

bors commented Sep 11, 2022

rust-timer commented Sep 11, 2022

Optimize thread parking on NetBSD #101482

Optimize thread parking on NetBSD #101482

Conversation

joboet commented Sep 6, 2022

rustbot commented Sep 6, 2022

m-ou-se Sep 6, 2022

Choose a reason for hiding this comment

joboet Sep 6, 2022

Choose a reason for hiding this comment

m-ou-se Sep 6, 2022

Choose a reason for hiding this comment

m-ou-se commented Sep 6, 2022

m-ou-se commented Sep 6, 2022

bors commented Sep 6, 2022

joboet commented Sep 6, 2022

joboet commented Sep 7, 2022

sanxiyn commented Sep 10, 2022

bors commented Sep 10, 2022

bors commented Sep 10, 2022

bors commented Sep 10, 2022

This comment has been minimized.

sanxiyn commented Sep 11, 2022

bors commented Sep 11, 2022

bors commented Sep 11, 2022

bors commented Sep 11, 2022

rust-timer commented Sep 11, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes