RFC proposal for thread affinity

[jamperry]

No description provided.

[durka]

this comment seems wrong

[durka]

if lock_on is analogous to lock, this line and the one after it shouldn't be here

[retep998]

JoinHandle is what actually contains the OS-specific thread handle. On Windows JoinHandle implements AsRawHandle and IntoRawHandle and on non-windows it implements JoinHandleExt. Also getting the handle to the current thread is pretty simple (GetCurrentThread or pthread_self depending on your platform).

rust-lang/rust#30177

[retep998]

There's a lot of room for detail on how exactly it locks to an arbitrary free CPU core. Does it determine what CPU core the thread is currently executing on and then lock to that? Does it pick a random CPU core?

[retep998]

Relevant discussion on Rust internals https://internals.rust-lang.org/t/pre-rfc-thread-affinity/3117/9

[retep998]

I personally think this should start as an external library.

[peterhj]

Thread affinity is something I'm sorely needing in Rust today as well. Some immediate comments on the RFC:

1. Rather than have just a lock, probably want a set, which maps better to the underlying thread affinity implementations (e.g., cpu_set_t), and which better reflects some real cases of "don't care" among threads on the same physical processor/socket.
2. At least for the pthread implementation of threads, there is a builder that initializes its pthread_attr_t before the thread is even spawned. For example, that is how the thread stack size is set (see libstd/sys/unix/thread.rs). Logically, it would make sense to allow setting the affinity through the thread builder as well, using pthread_attr_setaffinity_np under the hood. This could be in addition to a guard-based mechanism as you described.

There are also general comments which I think concern the affinity implementations for any language's standard library. Dumping some here off the top of my head:

• How are processors/sockets, cores, and threads numbered by the "system"? Typically the "system" numbering convention can be arbitrary or insane.
• How to query for all threads belonging to a particular physical core or processor? In practice, this is often the key feature one really wants. I'm aware of the C libraries libnuma and hwloc which address this, neither of which are particularly standard.
• What numbering convention to accept through your affinity interface? My feeling is that one should just interpret thread indices by the "system" numbering scheme (which is what you do already), and leave the finessing of CPU/core topology to another library.

It could be interesting if libstd received a standard way of querying CPU topology. But more likely it'll start off as just another library, which is just fine if it just works.

[alexcrichton]

Thanks for the RFC @jamperry! The discussion here and on the internals forum leads me to think that this may best be served by starting as an external library? It sounds like there's a lot of competing possible strategies and there's a lot of concerns to take into account. Taking some time to flesh out the library externally, get some experience, and toy with an API may be the best step forward here?

[alexcrichton]

🔔 This RFC is now entering its week-long final comment period 🔔

[aturon]

I agree with @retep998, @alexcrichton and others that this work should start in an external library and be considered for addition to std at a later time, after we've gained practical experience with it.

[diwic]

If implemented, how about using the thread Builder like this: thread::Builder::new().affinity(0x3).spawn(move || { /* ... */ }) ...where 0x3 is a bitmask of allowed cores.

[Florob]

Like others I think this should (at least initially) live in an external library.
Apart from that I think the term "lock(ing)" is unfortunate in this context, as it is already used with different meaning(s) (cf. mutexes, and x86's lock prefix). I would prefer pinning, binding, or affinity.

[alexcrichton]

The libs team discussed this RFC at triage today and the decision was to close for now. There doesn't appear to be a fundamental reason why this needs to be in the standard library right now, and otherwise it seems like this can be developed externally on crates.io

Thanks again though for the RFC @jamperry!