From 6d0dda77d362faafbb70fb23abfc1ac9c2198da8 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Sat, 6 Jun 2026 13:34:50 +0200 Subject: [PATCH 1/3] [RF] Less Cling usage in RooFactoryWSTool by caching `ctorArgs` results This speeds up the workspace factory language if the same constructors are used repeatedly, which is quite common when models are built programmatically. --- roofit/roofitcore/src/RooFactoryWSTool.cxx | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/roofit/roofitcore/src/RooFactoryWSTool.cxx b/roofit/roofitcore/src/RooFactoryWSTool.cxx index bbf8178d422b0..adcd24acf0889 100644 --- a/roofit/roofitcore/src/RooFactoryWSTool.cxx +++ b/roofit/roofitcore/src/RooFactoryWSTool.cxx @@ -211,7 +211,7 @@ namespace { return false; } - pair,unsigned int> ctorArgs(const char* classname, std::size_t nPassedArgs) { + pair,unsigned int> ctorArgsImpl(const char* classname, std::size_t nPassedArgs) { // Utility function for RooFactoryWSTool. Return arguments of 'first' non-default, non-copy constructor of any RooAbsArg // derived class. Only constructors that start with two `const char*` arguments (for name and title) are considered // The returned object contains @@ -269,6 +269,22 @@ namespace { gInterpreter->ClassInfo_Delete(cls); return pair,unsigned int>(ret,nreq); } + + pair,unsigned int> const & ctorArgs(const char* classname, std::size_t nPassedArgs) { + // Cache the result of ctorArgsImpl(). For a given (classname, nPassedArgs) + // the answer is determined by the static class definition and never changes + // at runtime, but ctorArgsImpl() drives the Cling interpreter to enumerate + // every constructor of the class. When the factory is invoked thousands of + // times (e.g. during HS3 JSON import of a large workspace), repeating that + // lookup dominates the import time. + static std::map, pair, unsigned int>> cache; + auto key = std::make_pair(string(classname), nPassedArgs); + auto it = cache.find(key); + if (it == cache.end()) { + it = cache.emplace(std::move(key), ctorArgsImpl(classname, nPassedArgs)).first; + } + return it->second; + } } //////////////////////////////////////////////////////////////////////////////// @@ -321,7 +337,7 @@ RooAbsArg* RooFactoryWSTool::createArg(const char* className, const char* objNam _args.push_back(tmp.substr(start_tok, end_tok)); // Try Cling interface - pair,unsigned int> ca = ctorArgs(className,_args.size()+2) ; + pair,unsigned int> const & ca = ctorArgs(className,_args.size()+2) ; if (ca.first.empty()) { coutE(ObjectHandling) << "RooFactoryWSTool::createArg() ERROR no suitable constructor found for class " << className << std::endl ; logError() ; @@ -352,7 +368,7 @@ RooAbsArg* RooFactoryWSTool::createArg(const char* className, const char* objNam try { Int_t i(0) ; - list::iterator ti = ca.first.begin() ; ++ti ; ++ti ; + list::const_iterator ti = ca.first.begin() ; ++ti ; ++ti ; for (vector::iterator ai = _args.begin() ; ai != _args.end() ; ++ai,++ti,++i) { if ((*ti)=="RooAbsReal&" || (*ti)=="const RooAbsReal&" || (*ti)=="RooAbsReal::Ref") { RooFactoryWSTool::as_FUNC(i) ; From 9c0cfa6777198eec84b957229c06abc90d97972d Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Sat, 6 Jun 2026 13:37:21 +0200 Subject: [PATCH 2/3] [RF][HS3] Fast-path importers for common RooFit classes Fast-path importers for RooProduct, RooAddition, and RooProdPdf that bypass the generic factory-expression mechanism, which is quite expensive because it uses the interpreter. This dramatically speeds up the import of models from JSON that use instances of these common classes in large numbers. --- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index c3b771a9ee033..90ffd950c7fe6 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -35,6 +35,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -135,6 +138,43 @@ class RooFormulaArgFactory : public RooFit::JSONIO::Importer { } }; +// Fast-path importers for RooProduct, RooAddition, and RooProdPdf that +// bypass the generic factory-expression mechanism. The default path +// generates a string expression and passes it to gROOT->ProcessLineFast(), +// which invokes the Cling JIT for every single call. For workspaces with +// thousands of product/sum nodes (a common shape for HistFactory models) +// that JIT cost dominates JSON import time. Constructing the RooFit object +// directly here keeps the work O(N) of cheap C++ calls. +class RooProductFactory : public RooFit::JSONIO::Importer { +public: + bool importArg(RooJSONFactoryWSTool *tool, const JSONNode &p) const override + { + std::string name(RooJSONFactoryWSTool::name(p)); + tool->wsEmplace(name, tool->requestArgList(p, "factors")); + return true; + } +}; + +class RooProdPdfFactory : public RooFit::JSONIO::Importer { +public: + bool importArg(RooJSONFactoryWSTool *tool, const JSONNode &p) const override + { + std::string name(RooJSONFactoryWSTool::name(p)); + tool->wsEmplace(name, tool->requestArgList(p, "factors")); + return true; + } +}; + +class RooAdditionFactory : public RooFit::JSONIO::Importer { +public: + bool importArg(RooJSONFactoryWSTool *tool, const JSONNode &p) const override + { + std::string name(RooJSONFactoryWSTool::name(p)); + tool->wsEmplace(name, tool->requestArgList(p, "summands")); + return true; + } +}; + class RooAddPdfFactory : public RooFit::JSONIO::Importer { public: bool importArg(RooJSONFactoryWSTool *tool, const JSONNode &p) const override @@ -1195,6 +1235,9 @@ DEFINE_EXPORTER_KEY(RooSplineStreamer, "spline"); STATIC_EXECUTE([]() { using namespace RooFit::JSONIO; + registerImporter("product", false); + registerImporter("product_dist", false); + registerImporter("sum", false); registerImporter("mixture_dist", false); registerImporter("mixture_model", false); registerImporter("binsampling_dist", false); From 32cc22eb7f44a765784bdf17b1b3e4b9cc7c4229 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Sat, 6 Jun 2026 13:38:33 +0200 Subject: [PATCH 3/3] [RF][HS3] Add name-keyed JSONNode cache for RooAbsaArgs Looking up by name in the JSON tree is quite slow and happens often during model import from JSON. Caching the pointers to the JSON nodes corresponding to the RooAbsArgs (distributions and functions) speeds up JSON import significantly. --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 8 +++ roofit/hs3/src/RooJSONFactoryWSTool.cxx | 49 ++++++++++++++----- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index d4e10d3ba1dc8..c58ce70154cfc 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace RooFit { namespace JSONIO { @@ -253,5 +254,12 @@ class RooJSONFactoryWSTool { std::unique_ptr _domains; std::vector _serversToExport; std::vector _serversToDelete; + + // Name-keyed indices over the top-level "functions" and "distributions" + // sequences of the input JSON. Built once at the start of importAllNodes() + // so that requestImpl() lookups become O(1) instead of an O(N) scan over + // every sibling node. + std::unordered_map _functionsByName; + std::unordered_map _distributionsByName; }; #endif diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 20d87473e408a..88ca1fd4d7f25 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -924,12 +924,11 @@ RooAbsPdf *RooJSONFactoryWSTool::requestImpl(const std::string &objna { if (RooAbsPdf *retval = _workspace.pdf(objname)) return retval; - if (const auto &distributionsNode = _rootnodeInput->find("distributions")) { - if (const auto &child = findNamedChild(*distributionsNode, objname)) { - this->importFunction(*child, true); - if (RooAbsPdf *retval = _workspace.pdf(objname)) - return retval; - } + auto it = _distributionsByName.find(objname); + if (it != _distributionsByName.end()) { + this->importFunction(*it->second, true); + if (RooAbsPdf *retval = _workspace.pdf(objname)) + return retval; } return nullptr; } @@ -945,12 +944,11 @@ RooAbsReal *RooJSONFactoryWSTool::requestImpl(const std::string &obj return pdf; if (RooRealVar *var = requestImpl(objname)) return var; - if (const auto &functionNode = _rootnodeInput->find("functions")) { - if (const auto &child = findNamedChild(*functionNode, objname)) { - this->importFunction(*child, true); - if (RooAbsReal *retval = _workspace.function(objname)) - return retval; - } + auto it = _functionsByName.find(objname); + if (it != _functionsByName.end()) { + this->importFunction(*it->second, true); + if (RooAbsReal *retval = _workspace.function(objname)) + return retval; } return nullptr; } @@ -2175,6 +2173,31 @@ void RooJSONFactoryWSTool::importAllNodes(const JSONNode &n) _attributesNode = findRooFitInternal(*_rootnodeInput, "attributes"); + // Build name-keyed indices over the "functions" and "distributions" + // sequences. Without these, every cross-reference resolved during import + // (e.g. dependencies of a PiecewiseInterpolation, or factory-expression + // arguments) triggers a linear scan over all sibling nodes via + // findNamedChild(), which becomes O(N^2) on workspaces with thousands of + // entries. Populating the maps up-front turns each lookup into O(1). + _functionsByName.clear(); + _distributionsByName.clear(); + if (auto seq = n.find("functions")) { + if (seq->is_seq()) { + _functionsByName.reserve(seq->num_children()); + for (const auto &p : seq->children()) { + _functionsByName.emplace(RooJSONFactoryWSTool::name(p), &p); + } + } + } + if (auto seq = n.find("distributions")) { + if (seq->is_seq()) { + _distributionsByName.reserve(seq->num_children()); + for (const auto &p : seq->children()) { + _distributionsByName.emplace(RooJSONFactoryWSTool::name(p), &p); + } + } + } + this->importDependants(n); if (auto paramPointsNode = n.find("parameter_points")) { @@ -2239,6 +2262,8 @@ void RooJSONFactoryWSTool::importAllNodes(const JSONNode &n) _rootnodeInput = nullptr; _domains.reset(); + _functionsByName.clear(); + _distributionsByName.clear(); } /**