Skip to content

Commit 326b106

Browse files
authored
Use thread_local instead of thread_specific_storage for internals (#5834)
* Use thread_local instead of thread_specific_storage for internals mangement thread_local is faster. * Make the pp manager a singleton. Strictly speaking, since the members are static, the instances must also be singletons or this wouldn't work. They already are, but we can make the class enforce it to be more 'self-documenting'.
1 parent d4d555d commit 326b106

File tree

1 file changed

+30
-20
lines changed

1 file changed

+30
-20
lines changed

include/pybind11/detail/internals.h

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,11 @@ template <typename InternalsType>
502502
class internals_pp_manager {
503503
public:
504504
using on_fetch_function = void(InternalsType *);
505-
internals_pp_manager(char const *id, on_fetch_function *on_fetch)
506-
: holder_id_(id), on_fetch_(on_fetch) {}
505+
506+
inline static internals_pp_manager &get_instance(char const *id, on_fetch_function *on_fetch) {
507+
static internals_pp_manager instance(id, on_fetch);
508+
return instance;
509+
}
507510

508511
/// Get the current pointer-to-pointer, allocating it if it does not already exist. May
509512
/// acquire the GIL. Will never return nullptr.
@@ -514,15 +517,15 @@ class internals_pp_manager {
514517
// internals_pp so that it can be pulled from the interpreter's state dict. That is
515518
// slow, so we use the current PyThreadState to check if it is necessary.
516519
auto *tstate = get_thread_state_unchecked();
517-
if (!tstate || tstate->interp != last_istate_.get()) {
520+
if (!tstate || tstate->interp != last_istate_tls()) {
518521
gil_scoped_acquire_simple gil;
519522
if (!tstate) {
520523
tstate = get_thread_state_unchecked();
521524
}
522-
last_istate_ = tstate->interp;
523-
internals_tls_p_ = get_or_create_pp_in_state_dict();
525+
last_istate_tls() = tstate->interp;
526+
internals_p_tls() = get_or_create_pp_in_state_dict();
524527
}
525-
return internals_tls_p_.get();
528+
return internals_p_tls();
526529
}
527530
#endif
528531
if (!internals_singleton_pp_) {
@@ -536,8 +539,8 @@ class internals_pp_manager {
536539
void unref() {
537540
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
538541
if (get_num_interpreters_seen() > 1) {
539-
last_istate_.reset();
540-
internals_tls_p_.reset();
542+
last_istate_tls() = nullptr;
543+
internals_p_tls() = nullptr;
541544
return;
542545
}
543546
#endif
@@ -549,8 +552,8 @@ class internals_pp_manager {
549552
if (get_num_interpreters_seen() > 1) {
550553
auto *tstate = get_thread_state_unchecked();
551554
// this could be called without an active interpreter, just use what was cached
552-
if (!tstate || tstate->interp == last_istate_.get()) {
553-
auto tpp = internals_tls_p_.get();
555+
if (!tstate || tstate->interp == last_istate_tls()) {
556+
auto tpp = internals_p_tls();
554557
if (tpp) {
555558
delete tpp;
556559
}
@@ -564,6 +567,9 @@ class internals_pp_manager {
564567
}
565568

566569
private:
570+
internals_pp_manager(char const *id, on_fetch_function *on_fetch)
571+
: holder_id_(id), on_fetch_(on_fetch) {}
572+
567573
std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
568574
error_scope err_scope;
569575
dict state_dict = get_python_state_dict();
@@ -589,12 +595,20 @@ class internals_pp_manager {
589595
return pp;
590596
}
591597

592-
char const *holder_id_ = nullptr;
593-
on_fetch_function *on_fetch_ = nullptr;
594598
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
595-
thread_specific_storage<PyInterpreterState> last_istate_;
596-
thread_specific_storage<std::unique_ptr<InternalsType>> internals_tls_p_;
599+
static PyInterpreterState *&last_istate_tls() {
600+
static thread_local PyInterpreterState *last_istate = nullptr;
601+
return last_istate;
602+
}
603+
604+
static std::unique_ptr<InternalsType> *&internals_p_tls() {
605+
static thread_local std::unique_ptr<InternalsType> *internals_p = nullptr;
606+
return internals_p;
607+
}
597608
#endif
609+
610+
char const *holder_id_ = nullptr;
611+
on_fetch_function *on_fetch_ = nullptr;
598612
std::unique_ptr<InternalsType> *internals_singleton_pp_;
599613
};
600614

@@ -624,10 +638,8 @@ inline internals_pp_manager<internals> &get_internals_pp_manager() {
624638
#else
625639
# define ON_FETCH_FN &check_internals_local_exception_translator
626640
#endif
627-
static internals_pp_manager<internals> internals_pp_manager(PYBIND11_INTERNALS_ID,
628-
ON_FETCH_FN);
641+
return internals_pp_manager<internals>::get_instance(PYBIND11_INTERNALS_ID, ON_FETCH_FN);
629642
#undef ON_FETCH_FN
630-
return internals_pp_manager;
631643
}
632644

633645
/// Return a reference to the current `internals` data
@@ -655,9 +667,7 @@ inline internals_pp_manager<local_internals> &get_local_internals_pp_manager() {
655667
static const std::string this_module_idstr
656668
= PYBIND11_MODULE_LOCAL_ID
657669
+ std::to_string(reinterpret_cast<uintptr_t>(&this_module_idstr));
658-
static internals_pp_manager<local_internals> local_internals_pp_manager(
659-
this_module_idstr.c_str(), nullptr);
660-
return local_internals_pp_manager;
670+
return internals_pp_manager<local_internals>::get_instance(this_module_idstr.c_str(), nullptr);
661671
}
662672

663673
/// Works like `get_internals`, but for things which are locally registered.

0 commit comments

Comments
 (0)