From b29e01b92e3e25eadea5d9c2c2ecf0c5f2dff528 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 13 Oct 2026 23:35:11 +0000 Subject: [PATCH] Memoize type names resolved through the AppendTypesSlow trampoline AppendTypesSlow() resolves type names that Cpp::GetScope() cannot look up directly (template specializations spelled as strings, names relative to a parent scope) by declaring a trampoline variable in the interpreter: __cppjit_interop_AppendTypesSlow __cppjit_interop_sN; Each call parses and compiles code, commits a transaction and leaves one more declaration in the AST, so every call costs more than the one before. Nothing cached the outcome, while the same names come back constantly: overload ranking (CPPMethod::GetPriority) resolves every argument type of every candidate, and it runs each time an overload set is not memoized. Memoize successful resolutions by name and parent scope, as GetType() already does for its __typeof__ trampoline. A name that fails may still resolve once more declarations are available, so failures are retried. The integral values of resolved arguments are strings whose ownership passes to the caller, so the cache keeps and hands out copies. Motivation: in ROOT, every task of a distributed RDataFrame converts a Python tuple into a std::pair&& argument. The implicit conversion goes through the pair's converting constructor template, whose overloads are ranked on every call; each ranking declared the trampoline for std::pair several times. The conversion took 26 ms on the first call and 250 ms after 200 calls, and the distributed RDataFrame test suite spent 536 s out of 686 s of worker time building RDataFrame objects (root-project/root#23675). Reproducer: import time import ROOT df = ROOT.RDataFrame(100) node = ROOT.RDF.AsRNode(df) for block in range(4): start = time.perf_counter() for _ in range(50): ROOT.Internal.RDF.ChangeEmptyEntryRange(node, (0, 10)) print(f"{(time.perf_counter() - start) * 1000 / 50:.1f} ms per call") --- src/interop/interop_wrapper.cxx | 36 ++++++++++++++++++++++++++++++++- test/test_regression.py | 20 ++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/interop/interop_wrapper.cxx b/src/interop/interop_wrapper.cxx index d26697c8..62faa9d8 100644 --- a/src/interop/interop_wrapper.cxx +++ b/src/interop/interop_wrapper.cxx @@ -524,6 +524,36 @@ bool interop::AppendTypesSlow(const std::string& name, std::lock_guard Lock(InterOpMutex); + // Every lookup below declares code through the interpreter, which parses, + // commits a transaction and leaves a new declaration in the AST, so each one + // costs more than the last. The same names come back constantly (overload + // ranking resolves every argument type of every candidate on each call that + // is not memoized), so remember what a name resolved to. A name that fails + // may still resolve once more declarations are available, so only successes + // are cached. + using SlowTypesKey = std::pair; + static std::map> s_cache; + const SlowTypesKey cache_key{name, parent.data}; + // the integral values are strings whose ownership passes to the caller + // (which frees them), so the cache keeps and hands out copies of its own + auto copy_of = [](const Cpp::TemplateArgInfo& info) { + return Cpp::TemplateArgInfo(info.m_Type, info.m_IntegralValue + ? strdup(info.m_IntegralValue) + : nullptr); + }; + if (auto cached = s_cache.find(cache_key); cached != s_cache.end()) { + for (const Cpp::TemplateArgInfo& info : cached->second) + types.push_back(copy_of(info)); + return false; + } + const size_t n_types_before = types.size(); + auto remember = [&] { + std::vector resolved; + for (size_t i = n_types_before; i < types.size(); ++i) + resolved.push_back(copy_of(types[i])); + s_cache.emplace(cache_key, std::move(resolved)); + }; + // We might have an entire expression such as int, double. static unsigned long long struct_count = 0; std::string code = @@ -554,7 +584,10 @@ bool interop::AppendTypesSlow(const std::string& name, TCppScope_t instance_class = Cpp::GetScopeFromType(varN); size_t oldSize = types.size(); Cpp::GetClassTemplateInstantiationArgs(instance_class, types); - return oldSize == types.size(); + if (oldSize == types.size()) + return true; + remember(); + return false; } } @@ -593,6 +626,7 @@ bool interop::AppendTypesSlow(const std::string& name, strdup(std::to_string(Cpp::GetEnumConstantValue(scope)).c_str()); types.emplace_back(type.data, integral_value); } + remember(); return false; } diff --git a/test/test_regression.py b/test/test_regression.py index 2a2bda4a..1b3061aa 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -1727,3 +1727,23 @@ def test56_enum_arg_typedefed_underlying_type(self): assert ns.pick(ns.S.A) == 1 assert ns.pick(ns.S.B) == 5 + + def test57_repeated_enum_constant_template_argument(self): + """A template argument spelled as an enum constant resolves repeatedly""" + + import cppjit + + cppjit.cppdef("""\ + namespace RepeatedEnumArg { + enum E { A = 1, B = 2 }; + template struct T1 { static constexpr int v = (int)e; }; + template struct T2 { static constexpr int v = (int)e; }; + }""") + + ns = cppjit.gbl.RepeatedEnumArg + + # the resolved argument is memoized by name; its integral value is a + # string the first instantiation frees, so the second needs its own + assert ns.T1["RepeatedEnumArg::E::A"].v == 1 + assert ns.T2["RepeatedEnumArg::E::A"].v == 1 + assert ns.T2["RepeatedEnumArg::E::B"].v == 2