3177: RFC: Use a const initializer for `GIL_COUNT` if possible r=adamreichold a=lifthrasiir

Normally [`LocalKey::try_with`](https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.try_with) needs to check for the initialization flag to lazily initialize the TLS. This behaves badly with a compiler optimization and it seems that multiple calls to `gil_is_acquired()` cannot be correctly eliminated. Rust 1.59 added a `const { ... }` initializer (a special form only allowed here for now) in `thread_local!` which removes the initialization flag, allowing those kind of optimizations.

I should note that the performance impact is probably minimal, because the check branch is extremely well predicted and the optimization is only possible when every PyO3 code that leads to `gil_is_acquired()` is inlined, a pretty uncommon situation. I couldn't demonstrate any consistent improvement or regression from my machine, which performance seems to be flaky as well. But at least we would have one less branch there. I'll leave this as an RFC so that someone else can prove or disprove that this is indeed beneficial.

Co-authored-by: Kang Seonghoon <public+git@mearie.org>
This commit is contained in:
bors[bot] 2023-05-24 06:24:12 +00:00 committed by GitHub
commit dfc667fd0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 3 deletions

View File

@ -156,6 +156,11 @@ pub fn print_feature_cfgs() {
if rustc_minor_version >= 53 {
println!("cargo:rustc-cfg=option_insert");
}
// Enable use of const initializer for thread_local! on Rust 1.59 and greater
if rustc_minor_version >= 59 {
println!("cargo:rustc-cfg=thread_local_const_init");
}
}
/// Private exports used in PyO3's build.rs

View File

@ -10,17 +10,29 @@ use std::{mem, ptr::NonNull, sync::atomic};
static START: Once = Once::new();
thread_local! {
cfg_if::cfg_if! {
if #[cfg(thread_local_const_init)] {
use std::thread_local as thread_local_const_init;
} else {
macro_rules! thread_local_const_init {
($($(#[$attr:meta])* static $name:ident: $ty:ty = const { $init:expr };)*) => (
thread_local! { $($(#[$attr])* static $name: $ty = $init;)* }
)
}
}
}
thread_local_const_init! {
/// This is an internal counter in pyo3 monitoring whether this thread has the GIL.
///
/// It will be incremented whenever a GILGuard or GILPool is created, and decremented whenever
/// they are dropped.
///
/// As a result, if this thread has the GIL, GIL_COUNT is greater than zero.
static GIL_COUNT: Cell<usize> = Cell::new(0);
static GIL_COUNT: Cell<usize> = const { Cell::new(0) };
/// Temporarily hold objects that will be released when the GILPool drops.
static OWNED_OBJECTS: RefCell<Vec<NonNull<ffi::PyObject>>> = RefCell::new(Vec::with_capacity(256));
static OWNED_OBJECTS: RefCell<Vec<NonNull<ffi::PyObject>>> = const { RefCell::new(Vec::new()) };
}
/// Checks whether the GIL is acquired.