From 4e6614be97b332aadb98f5a89c4be56ae56aa2b1 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. (cherry picked from commit a0c799b581e7abd787fae1409c51b9758aa84fbf) --- 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 77fb6ab636a3fa8f2a2d4df23dd85682699d0e65 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. (cherry picked from commit fbf5e8749c91b47e8462318d11d9e4f4218bf26c) --- 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 622d28191f0d25dac72d3606130daaa3bffef812 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. (cherry picked from commit 149cca405ea343a3abe3dc5b6c0eccf742d78fc4) --- .../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(); } /**