Skip to content

Commit 89565a7

Browse files
committed
Cache return types of called functions
Each time a function is called, we currently do a type lookup in registered_types_cpp to figure out the python type info associated with the return value. This lookup isn't free, particularly for often-called functions, but will usually yield an identical result each time the function is called. This commit adds a per-return-type cache into the return type lookup for generic types (i.e. those inheriting from type_caster_generic) to avoid the hash lookup. The cache is invalidated whenever the runtime type changes (to avoid the problem PR #409 tests for).
1 parent 591a999 commit 89565a7

File tree

2 files changed

+64
-26
lines changed

2 files changed

+64
-26
lines changed

include/pybind11/attr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ struct function_record {
106106
/// Number of arguments
107107
uint16_t nargs;
108108

109+
/// Cached detail::type_info pointer for return value
110+
const detail::type_info *ret_type_cache = nullptr;
111+
109112
/// Python method object
110113
PyMethodDef *def = nullptr;
111114

include/pybind11/cast.h

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,7 @@ class type_caster_generic {
172172
}
173173

174174
PYBIND11_NOINLINE static handle cast(const void *_src, return_value_policy policy, handle parent,
175-
const std::type_info *type_info,
176-
const std::type_info *type_info_backup,
175+
const detail::type_info *tinfo,
177176
void *(*copy_constructor)(const void *),
178177
void *(*move_constructor)(const void *),
179178
const void *existing_holder = nullptr) {
@@ -182,23 +181,6 @@ class type_caster_generic {
182181
return none().inc_ref();
183182

184183
auto &internals = get_internals();
185-
186-
auto it = internals.registered_types_cpp.find(std::type_index(*type_info));
187-
if (it == internals.registered_types_cpp.end()) {
188-
type_info = type_info_backup;
189-
it = internals.registered_types_cpp.find(std::type_index(*type_info));
190-
}
191-
192-
if (it == internals.registered_types_cpp.end()) {
193-
std::string tname = type_info->name();
194-
detail::clean_type_id(tname);
195-
std::string msg = "Unregistered type : " + tname;
196-
PyErr_SetString(PyExc_TypeError, msg.c_str());
197-
return handle();
198-
}
199-
200-
auto tinfo = (const detail::type_info *) it->second;
201-
202184
auto it_instances = internals.registered_instances.equal_range(src);
203185
for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) {
204186
auto instance_type = detail::get_type_info(Py_TYPE(it_i->second), false);
@@ -248,6 +230,54 @@ class type_caster_generic {
248230
const type_info *typeinfo = nullptr;
249231
void *value = nullptr;
250232
object temp;
233+
234+
template <typename T> static const detail::type_info *get_cached_ret_type(const std::type_info *typeinfo) {
235+
// Create some static locals, then forward everything to the non-templated version
236+
static const std::type_info *cache_runtime_type;
237+
static const detail::type_info *cache;
238+
return get_cached_ret_type(typeinfo, &typeid(T), cache, cache_runtime_type);
239+
}
240+
241+
PYBIND11_NOINLINE static const detail::type_info *get_cached_ret_type(
242+
const std::type_info *runtime_type,
243+
const std::type_info *type,
244+
const detail::type_info *&cache,
245+
const std::type_info *&cache_runtime_type) {
246+
247+
// Cache validation is dependent on both the C++ return type (T) *and* the actual run-time
248+
// type (which will differ if returning a subclass of an instance of T; it could also be
249+
// nullptr, and as long as both are nullptr, that's okay too).
250+
if (cache && cache_runtime_type == runtime_type) {
251+
return cache;
252+
}
253+
254+
auto &internals = get_internals();
255+
const detail::type_info *tinfo = nullptr;
256+
257+
// We need to look up the runtime type (if given), then fallback to the base type
258+
for (const std::type_info *t : {runtime_type, type}) {
259+
if (t) {
260+
auto it = internals.registered_types_cpp.find(*t);
261+
if (it != internals.registered_types_cpp.end()) {
262+
tinfo = static_cast<const detail::type_info *>(it->second);
263+
break;
264+
}
265+
}
266+
}
267+
268+
if (tinfo) {
269+
cache_runtime_type = runtime_type;
270+
return (cache = tinfo);
271+
}
272+
273+
std::string tname = runtime_type ? runtime_type->name() : type->name();
274+
detail::clean_type_id(tname);
275+
std::string msg = "Unregistered type : " + tname;
276+
PyErr_SetString(PyExc_TypeError, msg.c_str());
277+
return nullptr;
278+
}
279+
280+
251281
};
252282

253283
/* Determine suitable casting operator */
@@ -277,9 +307,12 @@ template <typename type> class type_caster_base : public type_caster_generic {
277307
}
278308

279309
static handle cast(const itype *src, return_value_policy policy, handle parent) {
280-
return type_caster_generic::cast(
281-
src, policy, parent, src ? &typeid(*src) : nullptr, &typeid(type),
282-
make_copy_constructor(src), make_move_constructor(src));
310+
311+
const detail::type_info *ret_type = get_cached_ret_type<itype>(src ? &typeid(*src) : nullptr);
312+
if (!ret_type) return handle(); // PyErr is already set
313+
314+
return type_caster_generic::cast(src, policy, parent,
315+
ret_type, make_copy_constructor(src), make_move_constructor(src));
283316
}
284317

285318
template <typename T> using cast_op_type = pybind11::detail::cast_op_type<T>;
@@ -790,10 +823,12 @@ template <typename type, typename holder_type> class type_caster_holder : public
790823
#endif
791824

792825
static handle cast(const holder_type &src, return_value_policy, handle) {
793-
return type_caster_generic::cast(
794-
src.get(), return_value_policy::take_ownership, handle(),
795-
src.get() ? &typeid(*src.get()) : nullptr, &typeid(type),
796-
nullptr, nullptr, &src);
826+
auto *got = src.get();
827+
const detail::type_info *ret_type = type_caster_generic::get_cached_ret_type<intrinsic_t<type>>(got ? &typeid(*got) : nullptr);
828+
if (!ret_type) return handle(); // PyErr is already set
829+
830+
return type_caster_generic::cast(src.get(), return_value_policy::take_ownership, handle(),
831+
ret_type, nullptr, nullptr, &src);
797832
}
798833

799834
protected:

0 commit comments

Comments
 (0)