No std ThreadId#6056
Conversation
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, one comment on the FFI definition (see below).
Do you expect that the test suite (which does a lot of thread spawning) will need to change?
| use core::num::NonZero; | ||
|
|
||
| extern_libpython! { | ||
| pub fn PyThread_get_thread_ident() -> NonZero<u64>; |
There was a problem hiding this comment.
There is also PyThread_get_thread_native_id. Do you know how these might differ?
I see that PyThread_get_thread_native_id is only available on some platforms, it might be challenging to define that availability with cfgs. (We could probably get it from the interpreter sysconfig when available, and make assumptions for "default" stable ABI builds.)
Also, we should use c_ulong as the return type here, to match the header.
The niche is interesting but we haven't made those part of the FFI definitions previously (e.g. many pointers could realistically be NonNull<T> rather than *mut T).
| pub fn PyThread_get_thread_ident() -> NonZero<u64>; | |
| pub fn PyThread_get_thread_ident() -> c_ulong; |
There was a problem hiding this comment.
There is also
PyThread_get_thread_native_id. Do you know how these might differ?I see that
PyThread_get_thread_native_idis only available on some platforms, it might be challenging to define that availability with cfgs. (We could probably get it from the interpretersysconfigwhen available, and make assumptions for "default" stable ABI builds.)
If the native version exists, it will get the thread id using platform APIs. That would make it match rust's ThreadId. I don't think there's any advantage in doing it that way since we can't compare it anyways because a) it's not stable b) in no_std rust has no ThreadId.
There was a problem hiding this comment.
Also, we should use
c_ulongas the return type here, to match the header.The niche is interesting but we haven't made those part of the FFI definitions previously (e.g. many pointers could realistically be
NonNull<T>rather than*mut T).
How do you feel about NonZero<c_ulong>? Or do you specifically want to ignore niches?
I don't think it will need to change, but it might be worth enabling it for wasm build and pulling in one of the wasm threading crates as a dev dependency. |
3 participants
Re-implement
ThreadIdusingPyThread_get_thread_ident. This makes is usable inno_stdcontext.