Skip to content

Commit f40c22b

Browse files
committed
Simplify loader_life_support TLS storage
Replace the `fake_thread_specific_storage` struct with a direct thread-local pointer managed via a function-local static: static loader_life_support *& tls_current_frame() This retains the "stack of frames" behavior via the `parent` link. It also reduces indirection and clarifies intent. Note: this form is C++11-compatible; once pybind11 requires C++17, the helper can be simplified to: inline static thread_local loader_life_support *tls_current_frame = nullptr;
1 parent efc678f commit f40c22b

File tree

1 file changed

+21
-31
lines changed

1 file changed

+21
-31
lines changed

include/pybind11/detail/type_caster_base.h

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -42,48 +42,38 @@ PYBIND11_NAMESPACE_BEGIN(detail)
4242
/// Adding a patient will keep it alive up until the enclosing function returns.
4343
class loader_life_support {
4444
private:
45+
// Thread-local top-of-stack for loader_life_support frames (linked via parent).
46+
// Observation: loader_life_support needs to be thread-local,
47+
// but we don't need to go to extra effort to keep it
48+
// per-interpreter (i.e., by putting it in internals) since
49+
// individual function calls are already isolated to a single
50+
// interpreter, even though they could potentially call into a
51+
// different interpreter later in the same call chain. This
52+
// saves a significant cost per function call spent in
53+
// loader_life_support destruction.
54+
// Note for future C++17 simplification:
55+
// inline static thread_local loader_life_support *tls_current_frame = nullptr;
56+
static loader_life_support *&tls_current_frame() {
57+
static thread_local loader_life_support *frame_ptr = nullptr;
58+
return frame_ptr;
59+
}
60+
4561
loader_life_support *parent = nullptr;
4662
std::unordered_set<PyObject *> keep_alive;
4763

48-
struct fake_thread_specific_storage {
49-
loader_life_support *instance = nullptr;
50-
loader_life_support *get() const { return instance; }
51-
52-
fake_thread_specific_storage &operator=(loader_life_support *pval) {
53-
instance = pval;
54-
return *this;
55-
}
56-
};
57-
using loader_storage = fake_thread_specific_storage;
58-
59-
static loader_storage &get_stack_top() {
60-
// Observation: loader_life_support needs to be thread-local,
61-
// but we don't need to go to extra effort to keep it
62-
// per-interpreter (i.e., by putting it in internals) since
63-
// individual function calls are already isolated to a single
64-
// interpreter, even though they could potentially call into a
65-
// different interpreter later in the same call chain. This
66-
// saves a significant cost per function call spent in
67-
// loader_life_support destruction.
68-
static thread_local fake_thread_specific_storage storage;
69-
return storage;
70-
}
71-
7264
public:
7365
/// A new patient frame is created when a function is entered
7466
loader_life_support() {
75-
auto &stack_top = get_stack_top();
76-
parent = stack_top.get();
77-
stack_top = this;
67+
parent = tls_current_frame();
68+
tls_current_frame() = this;
7869
}
7970

8071
/// ... and destroyed after it returns
8172
~loader_life_support() {
82-
auto &stack_top = get_stack_top();
83-
if (stack_top.get() != this) {
73+
if (tls_current_frame() != this) {
8474
pybind11_fail("loader_life_support: internal error");
8575
}
86-
stack_top = parent;
76+
tls_current_frame() = parent;
8777
for (auto *item : keep_alive) {
8878
Py_DECREF(item);
8979
}
@@ -92,7 +82,7 @@ class loader_life_support {
9282
/// This can only be used inside a pybind11-bound function, either by `argument_loader`
9383
/// at argument preparation time or by `py::cast()` at execution time.
9484
PYBIND11_NOINLINE static void add_patient(handle h) {
95-
loader_life_support *frame = get_stack_top().get();
85+
loader_life_support *frame = tls_current_frame();
9686
if (!frame) {
9787
// NOTE: It would be nice to include the stack frames here, as this indicates
9888
// use of pybind11::cast<> outside the normal call framework, finding such

0 commit comments

Comments
 (0)