From bbb5bdd1a09676076281f38fa91d66c6a4520a8e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 25 Jan 2022 22:02:01 +0100 Subject: [PATCH 01/74] feat: introduce a new implementation of IFactory which uses the project compiler to jit compile functors --- Phys/FunctorCore/CMakeLists.txt | 20 + .../src/Components/GigaFactory.cpp | 395 ++++++++++++++++++ 2 files changed, 415 insertions(+) create mode 100644 Phys/FunctorCore/src/Components/GigaFactory.cpp diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 13d00f7f1dd..5d88005832f 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -47,6 +47,7 @@ gaudi_add_module(FunctorCore SOURCES src/Components/ExampleAlg.cpp src/Components/Factory.cpp + src/Components/GigaFactory.cpp LINK Boost::headers FunctorCoreLib @@ -78,3 +79,22 @@ gaudi_install(PYTHON) gaudi_add_tests(QMTest) gaudi_add_tests(pytest ${CMAKE_CURRENT_SOURCE_DIR}/python) + +set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") + +message(STATUS "Generating JIT Wrapper at: ${JIT_CMD_FILE}") + +string(TOUPPER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_UPPER) + +get_property(tmp DIRECTORY PROPERTY COMPILE_DEFINITIONS) +set(defines) +if (tmp) + foreach(item ${tmp}) + list(APPEND defines "-D${item}") + endforeach() +endif() + +# FIXME how the hell does one get a complete list of flags!??!?!? :/ +file(WRITE ${JIT_CMD_FILE} "#!/bin/sh\n# Auto-generated script to create a jitter for the FunctorGigaFactory\nexec ${CMAKE_CXX_COMPILER} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared ${defines} \"$@\"\n") +execute_process(COMMAND chmod a+x ${JIT_CMD_FILE}) + diff --git a/Phys/FunctorCore/src/Components/GigaFactory.cpp b/Phys/FunctorCore/src/Components/GigaFactory.cpp new file mode 100644 index 00000000000..29df1b8286b --- /dev/null +++ b/Phys/FunctorCore/src/Components/GigaFactory.cpp @@ -0,0 +1,395 @@ +/*****************************************************************************\ +* (c) Copyright 2019-20 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include "Functors/Cache.h" +#include "Functors/FunctorDesc.h" +#include "Functors/IFactory.h" +#include "GaudiKernel/Service.h" +#include "boost/algorithm/string/erase.hpp" +#include "boost/algorithm/string/predicate.hpp" +#include "boost/algorithm/string/replace.hpp" +#include "boost/algorithm/string/split.hpp" +#include "boost/algorithm/string/trim.hpp" +#include "boost/format.hpp" +#include "boost/lexical_cast.hpp" +#include +#include +#include +#include +#include + +/** @file Factory.cpp + * @brief Definitions of non-templated functor factory functions. + */ + +namespace { + std::ostream& include( std::ostream& os, std::string_view header ) { + if ( header.empty() ) { throw GaudiException{"Got empty header name", "FunctorGigaFactory", StatusCode::FAILURE}; } + os << "#include "; + if ( header.front() == '<' && header.back() == '>' ) { + os << header; + } else { + os << '"' << header << '"'; + } + return os << '\n'; + } + + std::string make_default_cppname( std::string cppname ) { + if ( boost::algorithm::starts_with( cppname, "ToolSvc." ) ) { cppname.erase( 0, 8 ); } + std::replace_if( + cppname.begin(), cppname.end(), []( char c ) { return c == '.' || c == ' '; }, '_' ); + boost::algorithm::replace_all( cppname, "::", "__" ); + cppname.insert( 0, "FUNCTORS_" ); + return cppname; + } + std::unique_ptr openFile( std::string namebase, unsigned short findex, + std::vector const& lines, + std::vector const& headers ) { + // construct the file name + // 1) remove trailing .cpp + if ( boost::algorithm::ends_with( namebase, ".cpp" ) ) boost::algorithm::erase_tail( namebase, 4 ); + // 2) replace blanks by underscore + std::replace( namebase.begin(), namebase.end(), ' ', '_' ); + // 3) construct the name + boost::format fname( "%s_%04d.cpp" ); + fname % namebase % findex; + auto file = std::make_unique( fname.str() ); + // write the include directives + *file << "// Generic statements\n"; + for ( const auto& l : lines ) { *file << l << '\n'; } + *file << "// Explicitly declared include files:\n"; + for ( const auto& h : headers ) { include( *file, h ); } + return file; + } + + /** Write out the entry in a generated functor cache .cpp file for a single functor. + */ + std::ostream& makeCode( std::ostream& stream, Functors::Cache::HashType hash, std::string_view type, + std::string_view code ) { + boost::format declareFactory{R"_(namespace { + using t_%1% = %2%; + Gaudi::PluginService::DeclareFactory functor_%1%{ + Functors::Cache::id( %1% ), + []() -> std::unique_ptr { + return std::make_unique( + %3% + ); + } + }; +} +)_"}; + return stream << ( declareFactory % boost::io::group( std::showbase, std::hex, hash ) % type % code ); + } +} // namespace + +/** @class FunctorGigaFactory + * + * This service does all the heavy lifting behind compiling functors into + * type-erased Functor objects. It can do this either via ROOT's + * just-in-time compilation backend, or by using a pre-compiled functor + * cache. The tool is also responsible for generating the code that + * produces the precompiled functor cache. It is heavily inspired by Vanya + * Belyaev's LoKi::Hybrid::Base. + */ +struct FunctorGigaFactory : public extends { + using extends::extends; + + // pointer-to-function type we use when JIT compiling + using factory_function_ptr = functor_base_t ( * )(); + + functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, ThOr::FunctorDesc const& desc, + CompilationBehaviour compile ) override { + // Combine the 'compile' argument with the global settings to determine + // what compilation methods we should try + bool const fail_hard = compile & ExceptionOnFailure; + bool const use_jit{this->m_use_jit && ( compile & TryJIT )}; + bool const use_cache{this->m_use_cache && ( compile & TryCache )}; + + // Prepare the string that fully specifies the functor we want to retrieve -- basically the combination of + // input type, output type, functor string + // First, sort and de-duplicate the headers + auto headers = desc.headers; + std::sort( headers.begin(), headers.end() ); + headers.erase( std::unique( headers.begin(), headers.end() ), headers.end() ); + if ( msgLevel( MSG::DEBUG ) ) { + debug() << "Decoding " << desc.code << endmsg; + debug() << "With extra headers:"; + for ( auto const& header_line : headers ) { debug() << " " << header_line; } + debug() << endmsg; + } + + // FIXME it seems that having a quoted string in the middle of the + // string adds quotes at the start/end...need Gaudi!919 + std::size_t findex{desc.code.front() == '"'}; + std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; + auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); + + // This is basically Functor( PTCUT( ... ) ... ) + std::string full_code{functor_type}; + full_code.append( "( " ); + full_code.append( trimmed_code ); + full_code.append( " )" ); + + // Now we can calculate the hash + const auto hash = Functors::Cache::makeHash( full_code ); + + if ( msgLevel( MSG::VERBOSE ) ) { + verbose() << "Full string for hash: " << full_code << endmsg; + verbose() << "Resulting hash is " << std::hex << std::showbase << hash << endmsg; + } + + // The object we'll eventually return + functor_base_t functor; + + // See if we can magically load the functor from the cache (Gaudi magic!) + // Don't bother trying if we were told not to + if ( use_cache ) { + functor = ::Gaudi::PluginService::Factory::create( Functors::Cache::id( hash ) ); + if ( functor ) { + functor->setJITCompiled( false ); + } else if ( !functor && !use_jit ) { + // We print a different INFO message below if use_jit is true + info() << "Cache miss for functor: " << trimmed_code << endmsg; + } + } + + // Shorthand for throwing an informative exception + auto exception = [functor_type]( auto const& name ) { + std::string ourname{"FunctorGigaFactory::get<"}; + ourname.append( functor_type ); + ourname.append( ">" ); + return GaudiException{name, std::move( ourname ), StatusCode::FAILURE}; + }; + + if ( !functor && use_jit ) { + // See if we already JIT compiled this functor and can therefore reuse + // the factory function that we got before + auto iter = m_factories.find( hash ); + if ( iter != m_factories.end() ) { + // We had already JIT compiled this functor + info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; + auto factory_function = iter->second; + functor = factory_function(); + // don't set jit compiled, that is only a hack for cling crap + // functor->setJITCompiled( true ); + } else { + // Need to actually do the JIT compilation + if ( use_cache ) { + info() << "Cache miss for functor: " << trimmed_code << ", now trying JIT with headers " << headers << endmsg; + } else { + info() << "Using compiler for functor: " << trimmed_code << " with headers " << headers << endmsg; + } + + // The expression we ask the compiler to compile is not quite the same as + // 'full_code'. Instead of Functor( PT > ... ) we ask it to + // compile the declaration of a function returning functor_base_t that + // takes no arguments. We then ask dlsym to give us the address of + // this function and call it ourselves. This looks like: + // functor_base_t functor_0xdeadbeef() { + // return std::make_unique>( PT > ... ); + // } + + // FIXME + // FIXME + // 1. factor out the shared code with the other Factory + // + // 2. what to do about dlclose? technically as long as this service + // lives there could be someone who comes and asks again for a + // functor we already opened, so we shouldn't really dlclose before + // finalize ? But at finalize we will quit anyways so I guess for + // now I'll just leak :D Other option to avoid uncontrolled growing + // of memory would be to set a maximum of allowed open handles? + // We could implement some reference counting to keep track if a + // library is still in use and otherwise close it. + // + // 3. right now we create 1 lib per functor, can we somehow cluster + // this? + // + // + // FIXME + // FIXME + + std::ostringstream code; + +#ifdef USE_DD4HEP + code << "#define USE_DD4HEP\n"; +#endif + + // Include the required headers + for ( auto const& header : headers ) { include( code, header ); } + + // Get the name for the factory function. Add a suffix to avoid it + // matching the cache entries. + auto function_name = Functors::Cache::id( hash ) + "_jit"; + + code << "extern \"C\" {\n"; + + // Declare the factory function + code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " + << trimmed_code << " ); }\n"; + + code << "}"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } + + auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); + boost::replace_all( all_includes, ":", " -isystem" ); + + auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); + // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end + // if not, default to "/tmp/" + tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; + + auto const file_prefix = tmp_dir + function_name; + auto const cpp_filename = file_prefix + ".cpp"; + auto const lib_filename = file_prefix + ".so"; + { + std::ofstream out( cpp_filename ); + out << code.str(); + out.close(); + } + + // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags + // see Phys/FunctorCore/CMakeLists.txt + auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } + + FILE* pipe = popen( cmd.c_str(), "r" ); + if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } + + const size_t buffer_size = 128; + std::array buffer{}; + std::string cmd_out; + while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } + + auto returnCode = pclose( pipe ); + if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + + void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + if ( lib == nullptr ) { throw exception( dlerror() ); } + + auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); + if ( factory_function == nullptr ) { throw exception( dlerror() ); } + + // Save the factory function pointer + m_factories.emplace( hash, factory_function ); + + // Use the JITted factory function + functor = factory_function(); + functor->setJITCompiled( true ); + } + } + + if ( functor ) { + functor->bind( owner ); + } else if ( fail_hard && ( use_cache || use_jit ) ) { + // Don't emit too many messages while generating the functor caches. In + // that case both JIT and the cache are disabled, so we will never + // actually retrieve a functor here. + std::string error_message{"Couldn't load functor using ["}; + if ( use_cache && use_jit ) { + error_message += "cache, JIT"; + } else if ( use_cache ) { + error_message += "cache"; + } else { + error_message += "JIT"; + } + error_message += "]: " + desc.repr; + throw exception( error_message ); + } + + // If we're going to write out the .cpp files for creating the functor cache when we finalise then we need to + // store the relevant data in an internal structure + if ( this->m_makeCpp ) { + // Store the functor alongside others with the same headers + m_functors[std::move( headers )].emplace( hash, functor_type, trimmed_code ); + } + + return functor; + } + + /** Write out the C++ files needed to compile the functor cache if needed. + */ + StatusCode finalize() override { + if ( m_makeCpp ) { writeCpp(); } + return Service::finalize(); + } + +protected: + using HashType = Functors::Cache::HashType; + using FactoryCache = std::map; + using FunctorSet = std::map, std::set>>; + FunctorSet m_functors; // {headers: [{hash, type, code}, ...]} + FactoryCache m_factories; // {hash: factory function pointer, ...} + + /** @brief Generate the functor cache .cpp files. + * + * In order to expose as many bugs as possible, make sure that we generate a + * different .cpp file for every set of requested headers, so every functor + * is compiled with *exactly* the headers that were requested. + */ + void writeCpp() const { + /** The LoKi meaning of this parameter was: + * - positive: write N-files + * - negative: write N-functors per file + * - zero : write one file + * currently it is not fully supported, we just use it to check that CMake + * is aware of at least as many source files as the minimum we need. + * + * @todo When split > m_functors.size() then split the functors across + * more files until we are writing to all 'split' available source + * files. + */ + std::size_t split{0}; + if ( !boost::conversion::try_lexical_convert( System::getEnv( "LOKI_GENERATE_CPPCODE" ), split ) ) { split = 0; } + if ( m_functors.size() > split ) { + throw GaudiException{"Functor factory needs to generate at least " + std::to_string( m_functors.size() ) + + " source files, but LOKI_GENERATE_CPPCODE was set to " + std::to_string( split ) + + ". Increase the SPLIT setting in the call to loki_functors_cache() to at least " + + std::to_string( m_functors.size() ), + "FunctorGigaFactory", StatusCode::FAILURE}; + } + + /** We write one file for each unique set of headers + */ + unsigned short ifile{0}; + for ( auto const& [headers, func_set] : m_functors ) { + std::unique_ptr file; + unsigned short iwrite{0}; + for ( auto const& [hash, functor_type, brief_code] : func_set ) { + if ( !file ) { file = openFile( m_cppname, ++ifile, m_cpplines, headers ); } + + *file << '\n' << std::dec << std::noshowbase << "// FUNCTOR #" << ++iwrite << "/" << func_set.size() << '\n'; + + // write actual C++ code + ::makeCode( *file, hash, functor_type, brief_code ); + + *file << '\n'; + } + } + + // Make sure the remaining files are empty. This ensures generated code + // from previous builds (with more functors) is overwritten and does not + // interfere with the new build. + while ( ifile < split ) openFile( m_cppname, ++ifile, {}, {} ); + } + + // Flags to steer the use of JIT and the functor cache + Gaudi::Property m_use_cache{this, "UseCache", System::getEnv( "LOKI_DISABLE_CACHE" ) == "UNKNOWN"}; + Gaudi::Property m_use_jit{this, "UseJIT", System::getEnv( "LOKI_DISABLE_CLING" ) == "UNKNOWN"}; + Gaudi::Property m_makeCpp{this, "MakeCpp", System::getEnv( "LOKI_GENERATE_CPPCODE" ) != "UNKNOWN"}; + + // Properties steering the generated functor cache code + Gaudi::Property m_cppname{this, "CppFileName", make_default_cppname( this->name() )}; + Gaudi::Property> m_cpplines{this, "CppLines", {"#include \"Functors/Cache.h\""}}; +}; + +DECLARE_COMPONENT( FunctorGigaFactory ) -- GitLab From af782dd296766729d3ba76d0b785edc491389c3a Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 25 Jan 2022 22:02:28 +0100 Subject: [PATCH 02/74] add simple test for functor jit compilation --- .../tests/options/functor_jit_test.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 Phys/FunctorCore/tests/options/functor_jit_test.py diff --git a/Phys/FunctorCore/tests/options/functor_jit_test.py b/Phys/FunctorCore/tests/options/functor_jit_test.py new file mode 100644 index 00000000000..4b1478f5194 --- /dev/null +++ b/Phys/FunctorCore/tests/options/functor_jit_test.py @@ -0,0 +1,60 @@ +############################################################################### +# (c) Copyright 2021 CERN for the benefit of the LHCb Collaboration # +# # +# This software is distributed under the terms of the GNU General Public # +# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # +# # +# In applying this licence, CERN does not waive the privileges and immunities # +# granted to it by virtue of its status as an Intergovernmental Organization # +# or submit itself to any jurisdiction. # +############################################################################### + +from Configurables import (HLTControlFlowMgr, EvtStoreSvc, HiveDataBrokerSvc, + Gaudi__Examples__IntDataProducer as IDP, + FunctorExampleAlg_int) +from Functors import Ex_TimesTwo +from Gaudi.Configuration import ApplicationMgr, DEBUG + +evtMax = 1 +threads = 1 +evtslots = 40 + +# use direct calls to execute to circumvent sysExecute and AlgExecState +HLTControlFlowMgr().EnableLegacyMode = False + +# concurrency conf +HLTControlFlowMgr().ThreadPoolSize = threads + +whiteboard = EvtStoreSvc("EventDataSvc", EventSlots=evtslots) + +i1 = IDP(name="IDP1", OutputLevel=20, OutputLocation="/Event/SomeInt", Value=5) + +f_exp1 = Ex_TimesTwo + +for i in range(10): + f_exp1 = f_exp1 + Ex_TimesTwo + +use_cling = True + +if use_cling: + fea1 = FunctorExampleAlg_int( + "FEA1", Cut=f_exp1 > 9, Input0="/Event/SomeInt") +else: + fea1 = FunctorExampleAlg_int( + "FEA1", + Cut=f_exp1 > 9, + Input0="/Event/SomeInt", + Factory="FunctorGigaFactory") + +HLTControlFlowMgr().CompositeCFNodes = [('top_level', 'NONLAZY_OR', ['FEA1'], + False)] + +app = ApplicationMgr( + OutputLevel=1, + EvtMax=evtMax, + EvtSel='NONE', + ExtSvc=[whiteboard], + EventLoop=HLTControlFlowMgr(), + TopAlg=[i1, fea1]) + +HiveDataBrokerSvc().DataProducers = app.TopAlg -- GitLab From c20d7dc31bea67e7e8eed2c5bc24ccbe45d70f06 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Wed, 26 Jan 2022 09:11:25 +0100 Subject: [PATCH 03/74] replace cling factory with gcc factory to enable removable of cling hacks across codebase --- Phys/FunctorCore/CMakeLists.txt | 2 - Phys/FunctorCore/src/Components/Factory.cpp | 133 +++--- .../src/Components/GigaFactory.cpp | 395 ------------------ .../tests/options/functor_jit_test.py | 18 +- 4 files changed, 87 insertions(+), 461 deletions(-) delete mode 100644 Phys/FunctorCore/src/Components/GigaFactory.cpp diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 5d88005832f..4f09d9be45f 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -47,13 +47,11 @@ gaudi_add_module(FunctorCore SOURCES src/Components/ExampleAlg.cpp src/Components/Factory.cpp - src/Components/GigaFactory.cpp LINK Boost::headers FunctorCoreLib Gaudi::GaudiAlgLib Gaudi::GaudiKernel - ROOT::Core ) gaudi_add_executable(TestFunctors diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index e0bb91de24f..391890909c8 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -19,7 +19,8 @@ #include "boost/algorithm/string/trim.hpp" #include "boost/format.hpp" #include "boost/lexical_cast.hpp" -#include +#include +#include #include #include #include @@ -108,7 +109,7 @@ struct FunctorFactory : public extends { // Combine the 'compile' argument with the global settings to determine // what compilation methods we should try bool const fail_hard = compile & ExceptionOnFailure; - bool const use_cling{this->m_use_cling && ( compile & TryJIT )}; + bool const use_jit{this->m_use_jit && ( compile & TryJIT )}; bool const use_cache{this->m_use_cache && ( compile & TryCache )}; // Prepare the string that fully specifies the functor we want to retrieve -- basically the combination of @@ -126,7 +127,8 @@ struct FunctorFactory : public extends { // FIXME it seems that having a quoted string in the middle of the // string adds quotes at the start/end...need Gaudi!919 - std::size_t findex{desc.code.front() == '"'}, codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; + std::size_t findex{desc.code.front() == '"'}; + std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); // This is basically Functor( PTCUT( ... ) ... ) @@ -152,8 +154,8 @@ struct FunctorFactory : public extends { functor = ::Gaudi::PluginService::Factory::create( Functors::Cache::id( hash ) ); if ( functor ) { functor->setJITCompiled( false ); - } else if ( !functor && !use_cling ) { - // We print a different INFO message below if use_cling is true + } else if ( !functor && !use_jit ) { + // We print a different INFO message below if use_jit is true info() << "Cache miss for functor: " << trimmed_code << endmsg; } } @@ -166,50 +168,56 @@ struct FunctorFactory : public extends { return GaudiException{name, std::move( ourname ), StatusCode::FAILURE}; }; - if ( !functor && use_cling ) { + if ( !functor && use_jit ) { // See if we already JIT compiled this functor and can therefore reuse // the factory function that we got before auto iter = m_factories.find( hash ); if ( iter != m_factories.end() ) { // We had already JIT compiled this functor - info() << "Reusing cling compiled factory for functor: " << trimmed_code << endmsg; + info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; auto factory_function = iter->second; functor = factory_function(); - functor->setJITCompiled( true ); + // don't set jit compiled, that is only a hack for cling crap + // functor->setJITCompiled( true ); } else { // Need to actually do the JIT compilation if ( use_cache ) { - info() << "Cache miss for functor: " << trimmed_code << ", now trying cling with headers " << headers - << endmsg; + info() << "Cache miss for functor: " << trimmed_code << ", now trying JIT with headers " << headers << endmsg; } else { - info() << "Using cling for functor: " << trimmed_code << " with headers " << headers << endmsg; + info() << "Using compiler for functor: " << trimmed_code << " with headers " << headers << endmsg; } - // The expression we ask cling to compile is not quite the same as + // The expression we ask the compiler to compile is not quite the same as // 'full_code'. Instead of Functor( PT > ... ) we ask it to // compile the declaration of a function returning functor_base_t that - // takes no arguments. We then ask cling to give us the address of + // takes no arguments. We then ask dlsym to give us the address of // this function and call it ourselves. This looks like: // functor_base_t functor_0xdeadbeef() { // return std::make_unique>( PT > ... ); // } - std::ostringstream code; - // Enable -O3-like code optimisation - code << "#pragma cling optimize(3)\n"; + // FIXME + // FIXME + // 1. factor out the shared code with the other Factory + // + // 2. what to do about dlclose? technically as long as this service + // lives there could be someone who comes and asks again for a + // functor we already opened, so we shouldn't really dlclose before + // finalize ? But at finalize we will quit anyways so I guess for + // now I'll just leak :D Other option to avoid uncontrolled growing + // of memory would be to set a maximum of allowed open handles? + // We could implement some reference counting to keep track if a + // library is still in use and otherwise close it. + // + // 3. right now we create 1 lib per functor, can we somehow cluster + // this? + // + // + // FIXME + // FIXME - // Work around cling not auto-loading based on functions...? - code << "#pragma cling load(\"TrackKernel\")\n"; + std::ostringstream code; - // Workaround for cling errors along the lines of: - // unimplemented pure virtual method 'i_cast' in 'RelationWeighted' - // and several others. See https://gitlab.cern.ch/gaudi/Gaudi/issues/85 - // for discussion of a better solution. -#ifdef GAUDI_V20_COMPAT - code << "#ifndef GAUDI_V20_COMPAT\n"; - code << "#define GAUDI_V20_COMPAT\n"; - code << "#endif\n"; -#endif #ifdef USE_DD4HEP code << "#define USE_DD4HEP\n"; #endif @@ -219,32 +227,57 @@ struct FunctorFactory : public extends { // Get the name for the factory function. Add a suffix to avoid it // matching the cache entries. - auto function_name = Functors::Cache::id( hash ) + "_cling"; + auto function_name = Functors::Cache::id( hash ) + "_jit"; + + code << "extern \"C\" {\n"; // Declare the factory function code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " << trimmed_code << " ); }\n"; - // Assign its address to a variable to trigger compilation while the pragmas are active - code << "auto const " << function_name << "_factory_ptr = " << function_name + ";"; + code << "}"; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } - // Try and JIT compile this expression - if ( !gInterpreter->Declare( code.str().c_str() ) ) { - info() << "Code we attempted to JIT compile was:\n" << code.str() << endmsg; - throw exception( "Failed to JIT compile functor" ); + auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); + boost::replace_all( all_includes, ":", " -isystem" ); + + auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); + // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end + // if not, default to "/tmp/" + tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; + + auto const file_prefix = tmp_dir + function_name; + auto const cpp_filename = file_prefix + ".cpp"; + auto const lib_filename = file_prefix + ".so"; + { + std::ofstream out( cpp_filename ); + out << code.str(); + out.close(); } - // Get the address of the factory function we just JITted - TInterpreter::EErrorCode error_code{TInterpreter::kNoError}; - auto value = gInterpreter->Calc( ( function_name + "_factory_ptr" ).c_str(), &error_code ); - if ( error_code != TInterpreter::kNoError ) { - throw exception( "Failed to get the factory function address, error code " + std::to_string( error_code ) ); - } + // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags + // see Phys/FunctorCore/CMakeLists.txt + auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } + + FILE* pipe = popen( cmd.c_str(), "r" ); + if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } + + const size_t buffer_size = 128; + std::array buffer{}; + std::string cmd_out; + while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } + + auto returnCode = pclose( pipe ); + if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + + void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + if ( lib == nullptr ) { throw exception( dlerror() ); } - // Cast the function pointer to the correct type - auto factory_function = reinterpret_cast( value ); + auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); + if ( factory_function == nullptr ) { throw exception( dlerror() ); } // Save the factory function pointer m_factories.emplace( hash, factory_function ); @@ -257,20 +290,20 @@ struct FunctorFactory : public extends { if ( functor ) { functor->bind( owner ); - } else if ( fail_hard && ( use_cache || use_cling ) ) { + } else if ( fail_hard && ( use_cache || use_jit ) ) { // Don't emit too many messages while generating the functor caches. In - // that case both cling and the cache are disabled, so we will never + // that case both JIT and the cache are disabled, so we will never // actually retrieve a functor here. std::string error_message{"Couldn't load functor using ["}; - if ( use_cache && use_cling ) { - error_message += "cache, cling"; + if ( use_cache && use_jit ) { + error_message += "cache, JIT"; } else if ( use_cache ) { error_message += "cache"; } else { - error_message += "cling"; + error_message += "JIT"; } error_message += "]: " + desc.repr; - throw exception( std::move( error_message ) ); + throw exception( error_message ); } // If we're going to write out the .cpp files for creating the functor cache when we finalise then we need to @@ -349,9 +382,9 @@ protected: while ( ifile < split ) openFile( m_cppname, ++ifile, {}, {} ); } - // Flags to steer the use of cling and the functor cache + // Flags to steer the use of JIT and the functor cache Gaudi::Property m_use_cache{this, "UseCache", System::getEnv( "LOKI_DISABLE_CACHE" ) == "UNKNOWN"}; - Gaudi::Property m_use_cling{this, "UseCling", System::getEnv( "LOKI_DISABLE_CLING" ) == "UNKNOWN"}; + Gaudi::Property m_use_jit{this, "UseJIT", System::getEnv( "LOKI_DISABLE_CLING" ) == "UNKNOWN"}; Gaudi::Property m_makeCpp{this, "MakeCpp", System::getEnv( "LOKI_GENERATE_CPPCODE" ) != "UNKNOWN"}; // Properties steering the generated functor cache code diff --git a/Phys/FunctorCore/src/Components/GigaFactory.cpp b/Phys/FunctorCore/src/Components/GigaFactory.cpp deleted file mode 100644 index 29df1b8286b..00000000000 --- a/Phys/FunctorCore/src/Components/GigaFactory.cpp +++ /dev/null @@ -1,395 +0,0 @@ -/*****************************************************************************\ -* (c) Copyright 2019-20 CERN for the benefit of the LHCb Collaboration * -* * -* This software is distributed under the terms of the GNU General Public * -* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * -* * -* In applying this licence, CERN does not waive the privileges and immunities * -* granted to it by virtue of its status as an Intergovernmental Organization * -* or submit itself to any jurisdiction. * -\*****************************************************************************/ -#include "Functors/Cache.h" -#include "Functors/FunctorDesc.h" -#include "Functors/IFactory.h" -#include "GaudiKernel/Service.h" -#include "boost/algorithm/string/erase.hpp" -#include "boost/algorithm/string/predicate.hpp" -#include "boost/algorithm/string/replace.hpp" -#include "boost/algorithm/string/split.hpp" -#include "boost/algorithm/string/trim.hpp" -#include "boost/format.hpp" -#include "boost/lexical_cast.hpp" -#include -#include -#include -#include -#include - -/** @file Factory.cpp - * @brief Definitions of non-templated functor factory functions. - */ - -namespace { - std::ostream& include( std::ostream& os, std::string_view header ) { - if ( header.empty() ) { throw GaudiException{"Got empty header name", "FunctorGigaFactory", StatusCode::FAILURE}; } - os << "#include "; - if ( header.front() == '<' && header.back() == '>' ) { - os << header; - } else { - os << '"' << header << '"'; - } - return os << '\n'; - } - - std::string make_default_cppname( std::string cppname ) { - if ( boost::algorithm::starts_with( cppname, "ToolSvc." ) ) { cppname.erase( 0, 8 ); } - std::replace_if( - cppname.begin(), cppname.end(), []( char c ) { return c == '.' || c == ' '; }, '_' ); - boost::algorithm::replace_all( cppname, "::", "__" ); - cppname.insert( 0, "FUNCTORS_" ); - return cppname; - } - std::unique_ptr openFile( std::string namebase, unsigned short findex, - std::vector const& lines, - std::vector const& headers ) { - // construct the file name - // 1) remove trailing .cpp - if ( boost::algorithm::ends_with( namebase, ".cpp" ) ) boost::algorithm::erase_tail( namebase, 4 ); - // 2) replace blanks by underscore - std::replace( namebase.begin(), namebase.end(), ' ', '_' ); - // 3) construct the name - boost::format fname( "%s_%04d.cpp" ); - fname % namebase % findex; - auto file = std::make_unique( fname.str() ); - // write the include directives - *file << "// Generic statements\n"; - for ( const auto& l : lines ) { *file << l << '\n'; } - *file << "// Explicitly declared include files:\n"; - for ( const auto& h : headers ) { include( *file, h ); } - return file; - } - - /** Write out the entry in a generated functor cache .cpp file for a single functor. - */ - std::ostream& makeCode( std::ostream& stream, Functors::Cache::HashType hash, std::string_view type, - std::string_view code ) { - boost::format declareFactory{R"_(namespace { - using t_%1% = %2%; - Gaudi::PluginService::DeclareFactory functor_%1%{ - Functors::Cache::id( %1% ), - []() -> std::unique_ptr { - return std::make_unique( - %3% - ); - } - }; -} -)_"}; - return stream << ( declareFactory % boost::io::group( std::showbase, std::hex, hash ) % type % code ); - } -} // namespace - -/** @class FunctorGigaFactory - * - * This service does all the heavy lifting behind compiling functors into - * type-erased Functor objects. It can do this either via ROOT's - * just-in-time compilation backend, or by using a pre-compiled functor - * cache. The tool is also responsible for generating the code that - * produces the precompiled functor cache. It is heavily inspired by Vanya - * Belyaev's LoKi::Hybrid::Base. - */ -struct FunctorGigaFactory : public extends { - using extends::extends; - - // pointer-to-function type we use when JIT compiling - using factory_function_ptr = functor_base_t ( * )(); - - functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, ThOr::FunctorDesc const& desc, - CompilationBehaviour compile ) override { - // Combine the 'compile' argument with the global settings to determine - // what compilation methods we should try - bool const fail_hard = compile & ExceptionOnFailure; - bool const use_jit{this->m_use_jit && ( compile & TryJIT )}; - bool const use_cache{this->m_use_cache && ( compile & TryCache )}; - - // Prepare the string that fully specifies the functor we want to retrieve -- basically the combination of - // input type, output type, functor string - // First, sort and de-duplicate the headers - auto headers = desc.headers; - std::sort( headers.begin(), headers.end() ); - headers.erase( std::unique( headers.begin(), headers.end() ), headers.end() ); - if ( msgLevel( MSG::DEBUG ) ) { - debug() << "Decoding " << desc.code << endmsg; - debug() << "With extra headers:"; - for ( auto const& header_line : headers ) { debug() << " " << header_line; } - debug() << endmsg; - } - - // FIXME it seems that having a quoted string in the middle of the - // string adds quotes at the start/end...need Gaudi!919 - std::size_t findex{desc.code.front() == '"'}; - std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; - auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); - - // This is basically Functor( PTCUT( ... ) ... ) - std::string full_code{functor_type}; - full_code.append( "( " ); - full_code.append( trimmed_code ); - full_code.append( " )" ); - - // Now we can calculate the hash - const auto hash = Functors::Cache::makeHash( full_code ); - - if ( msgLevel( MSG::VERBOSE ) ) { - verbose() << "Full string for hash: " << full_code << endmsg; - verbose() << "Resulting hash is " << std::hex << std::showbase << hash << endmsg; - } - - // The object we'll eventually return - functor_base_t functor; - - // See if we can magically load the functor from the cache (Gaudi magic!) - // Don't bother trying if we were told not to - if ( use_cache ) { - functor = ::Gaudi::PluginService::Factory::create( Functors::Cache::id( hash ) ); - if ( functor ) { - functor->setJITCompiled( false ); - } else if ( !functor && !use_jit ) { - // We print a different INFO message below if use_jit is true - info() << "Cache miss for functor: " << trimmed_code << endmsg; - } - } - - // Shorthand for throwing an informative exception - auto exception = [functor_type]( auto const& name ) { - std::string ourname{"FunctorGigaFactory::get<"}; - ourname.append( functor_type ); - ourname.append( ">" ); - return GaudiException{name, std::move( ourname ), StatusCode::FAILURE}; - }; - - if ( !functor && use_jit ) { - // See if we already JIT compiled this functor and can therefore reuse - // the factory function that we got before - auto iter = m_factories.find( hash ); - if ( iter != m_factories.end() ) { - // We had already JIT compiled this functor - info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; - auto factory_function = iter->second; - functor = factory_function(); - // don't set jit compiled, that is only a hack for cling crap - // functor->setJITCompiled( true ); - } else { - // Need to actually do the JIT compilation - if ( use_cache ) { - info() << "Cache miss for functor: " << trimmed_code << ", now trying JIT with headers " << headers << endmsg; - } else { - info() << "Using compiler for functor: " << trimmed_code << " with headers " << headers << endmsg; - } - - // The expression we ask the compiler to compile is not quite the same as - // 'full_code'. Instead of Functor( PT > ... ) we ask it to - // compile the declaration of a function returning functor_base_t that - // takes no arguments. We then ask dlsym to give us the address of - // this function and call it ourselves. This looks like: - // functor_base_t functor_0xdeadbeef() { - // return std::make_unique>( PT > ... ); - // } - - // FIXME - // FIXME - // 1. factor out the shared code with the other Factory - // - // 2. what to do about dlclose? technically as long as this service - // lives there could be someone who comes and asks again for a - // functor we already opened, so we shouldn't really dlclose before - // finalize ? But at finalize we will quit anyways so I guess for - // now I'll just leak :D Other option to avoid uncontrolled growing - // of memory would be to set a maximum of allowed open handles? - // We could implement some reference counting to keep track if a - // library is still in use and otherwise close it. - // - // 3. right now we create 1 lib per functor, can we somehow cluster - // this? - // - // - // FIXME - // FIXME - - std::ostringstream code; - -#ifdef USE_DD4HEP - code << "#define USE_DD4HEP\n"; -#endif - - // Include the required headers - for ( auto const& header : headers ) { include( code, header ); } - - // Get the name for the factory function. Add a suffix to avoid it - // matching the cache entries. - auto function_name = Functors::Cache::id( hash ) + "_jit"; - - code << "extern \"C\" {\n"; - - // Declare the factory function - code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " - << trimmed_code << " ); }\n"; - - code << "}"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } - - auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); - boost::replace_all( all_includes, ":", " -isystem" ); - - auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); - // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end - // if not, default to "/tmp/" - tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; - - auto const file_prefix = tmp_dir + function_name; - auto const cpp_filename = file_prefix + ".cpp"; - auto const lib_filename = file_prefix + ".so"; - { - std::ofstream out( cpp_filename ); - out << code.str(); - out.close(); - } - - // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags - // see Phys/FunctorCore/CMakeLists.txt - auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } - - FILE* pipe = popen( cmd.c_str(), "r" ); - if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - - const size_t buffer_size = 128; - std::array buffer{}; - std::string cmd_out; - while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } - - auto returnCode = pclose( pipe ); - if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } - - void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); - if ( lib == nullptr ) { throw exception( dlerror() ); } - - auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); - if ( factory_function == nullptr ) { throw exception( dlerror() ); } - - // Save the factory function pointer - m_factories.emplace( hash, factory_function ); - - // Use the JITted factory function - functor = factory_function(); - functor->setJITCompiled( true ); - } - } - - if ( functor ) { - functor->bind( owner ); - } else if ( fail_hard && ( use_cache || use_jit ) ) { - // Don't emit too many messages while generating the functor caches. In - // that case both JIT and the cache are disabled, so we will never - // actually retrieve a functor here. - std::string error_message{"Couldn't load functor using ["}; - if ( use_cache && use_jit ) { - error_message += "cache, JIT"; - } else if ( use_cache ) { - error_message += "cache"; - } else { - error_message += "JIT"; - } - error_message += "]: " + desc.repr; - throw exception( error_message ); - } - - // If we're going to write out the .cpp files for creating the functor cache when we finalise then we need to - // store the relevant data in an internal structure - if ( this->m_makeCpp ) { - // Store the functor alongside others with the same headers - m_functors[std::move( headers )].emplace( hash, functor_type, trimmed_code ); - } - - return functor; - } - - /** Write out the C++ files needed to compile the functor cache if needed. - */ - StatusCode finalize() override { - if ( m_makeCpp ) { writeCpp(); } - return Service::finalize(); - } - -protected: - using HashType = Functors::Cache::HashType; - using FactoryCache = std::map; - using FunctorSet = std::map, std::set>>; - FunctorSet m_functors; // {headers: [{hash, type, code}, ...]} - FactoryCache m_factories; // {hash: factory function pointer, ...} - - /** @brief Generate the functor cache .cpp files. - * - * In order to expose as many bugs as possible, make sure that we generate a - * different .cpp file for every set of requested headers, so every functor - * is compiled with *exactly* the headers that were requested. - */ - void writeCpp() const { - /** The LoKi meaning of this parameter was: - * - positive: write N-files - * - negative: write N-functors per file - * - zero : write one file - * currently it is not fully supported, we just use it to check that CMake - * is aware of at least as many source files as the minimum we need. - * - * @todo When split > m_functors.size() then split the functors across - * more files until we are writing to all 'split' available source - * files. - */ - std::size_t split{0}; - if ( !boost::conversion::try_lexical_convert( System::getEnv( "LOKI_GENERATE_CPPCODE" ), split ) ) { split = 0; } - if ( m_functors.size() > split ) { - throw GaudiException{"Functor factory needs to generate at least " + std::to_string( m_functors.size() ) + - " source files, but LOKI_GENERATE_CPPCODE was set to " + std::to_string( split ) + - ". Increase the SPLIT setting in the call to loki_functors_cache() to at least " + - std::to_string( m_functors.size() ), - "FunctorGigaFactory", StatusCode::FAILURE}; - } - - /** We write one file for each unique set of headers - */ - unsigned short ifile{0}; - for ( auto const& [headers, func_set] : m_functors ) { - std::unique_ptr file; - unsigned short iwrite{0}; - for ( auto const& [hash, functor_type, brief_code] : func_set ) { - if ( !file ) { file = openFile( m_cppname, ++ifile, m_cpplines, headers ); } - - *file << '\n' << std::dec << std::noshowbase << "// FUNCTOR #" << ++iwrite << "/" << func_set.size() << '\n'; - - // write actual C++ code - ::makeCode( *file, hash, functor_type, brief_code ); - - *file << '\n'; - } - } - - // Make sure the remaining files are empty. This ensures generated code - // from previous builds (with more functors) is overwritten and does not - // interfere with the new build. - while ( ifile < split ) openFile( m_cppname, ++ifile, {}, {} ); - } - - // Flags to steer the use of JIT and the functor cache - Gaudi::Property m_use_cache{this, "UseCache", System::getEnv( "LOKI_DISABLE_CACHE" ) == "UNKNOWN"}; - Gaudi::Property m_use_jit{this, "UseJIT", System::getEnv( "LOKI_DISABLE_CLING" ) == "UNKNOWN"}; - Gaudi::Property m_makeCpp{this, "MakeCpp", System::getEnv( "LOKI_GENERATE_CPPCODE" ) != "UNKNOWN"}; - - // Properties steering the generated functor cache code - Gaudi::Property m_cppname{this, "CppFileName", make_default_cppname( this->name() )}; - Gaudi::Property> m_cpplines{this, "CppLines", {"#include \"Functors/Cache.h\""}}; -}; - -DECLARE_COMPONENT( FunctorGigaFactory ) diff --git a/Phys/FunctorCore/tests/options/functor_jit_test.py b/Phys/FunctorCore/tests/options/functor_jit_test.py index 4b1478f5194..6fe8e8020cb 100644 --- a/Phys/FunctorCore/tests/options/functor_jit_test.py +++ b/Phys/FunctorCore/tests/options/functor_jit_test.py @@ -13,11 +13,11 @@ from Configurables import (HLTControlFlowMgr, EvtStoreSvc, HiveDataBrokerSvc, Gaudi__Examples__IntDataProducer as IDP, FunctorExampleAlg_int) from Functors import Ex_TimesTwo -from Gaudi.Configuration import ApplicationMgr, DEBUG +from Gaudi.Configuration import ApplicationMgr, VERBOSE evtMax = 1 threads = 1 -evtslots = 40 +evtslots = 1 # use direct calls to execute to circumvent sysExecute and AlgExecState HLTControlFlowMgr().EnableLegacyMode = False @@ -34,23 +34,13 @@ f_exp1 = Ex_TimesTwo for i in range(10): f_exp1 = f_exp1 + Ex_TimesTwo -use_cling = True - -if use_cling: - fea1 = FunctorExampleAlg_int( - "FEA1", Cut=f_exp1 > 9, Input0="/Event/SomeInt") -else: - fea1 = FunctorExampleAlg_int( - "FEA1", - Cut=f_exp1 > 9, - Input0="/Event/SomeInt", - Factory="FunctorGigaFactory") +fea1 = FunctorExampleAlg_int("FEA1", Cut=f_exp1 > 9, Input0="/Event/SomeInt") HLTControlFlowMgr().CompositeCFNodes = [('top_level', 'NONLAZY_OR', ['FEA1'], False)] app = ApplicationMgr( - OutputLevel=1, + OutputLevel=VERBOSE, EvtMax=evtMax, EvtSel='NONE', ExtSvc=[whiteboard], -- GitLab From deacb75beb5edd465b3c04336ebd671c5a22d39d Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Wed, 26 Jan 2022 10:57:47 +0100 Subject: [PATCH 04/74] make sure we call dlclose on finalize. Bit more doc in the code --- Phys/FunctorCore/src/Components/Factory.cpp | 49 +++++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 391890909c8..b47900e30b8 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -92,11 +92,11 @@ namespace { /** @class FunctorFactory * * This service does all the heavy lifting behind compiling functors into - * type-erased Functor objects. It can do this either via ROOT's - * just-in-time compilation backend, or by using a pre-compiled functor - * cache. The tool is also responsible for generating the code that - * produces the precompiled functor cache. It is heavily inspired by Vanya - * Belyaev's LoKi::Hybrid::Base. + * type-erased Functor objects. It can do this either via on demand + * invoking the same compiler the project was compiled with, or by using a + * pre-compiled functor cache. The tool is also responsible for generating the + * code that produces the precompiled functor cache. It is heavily inspired by + * Vanya Belyaev's LoKi::Hybrid::Base. */ struct FunctorFactory : public extends { using extends::extends; @@ -112,9 +112,9 @@ struct FunctorFactory : public extends { bool const use_jit{this->m_use_jit && ( compile & TryJIT )}; bool const use_cache{this->m_use_cache && ( compile & TryCache )}; - // Prepare the string that fully specifies the functor we want to retrieve -- basically the combination of - // input type, output type, functor string - // First, sort and de-duplicate the headers + // Prepare the string that fully specifies the functor we want to retrieve + // -- basically the combination of input type, output type, functor string + // But first: sort and de-duplicate the headers auto headers = desc.headers; std::sort( headers.begin(), headers.end() ); headers.erase( std::unique( headers.begin(), headers.end() ), headers.end() ); @@ -175,7 +175,7 @@ struct FunctorFactory : public extends { if ( iter != m_factories.end() ) { // We had already JIT compiled this functor info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; - auto factory_function = iter->second; + auto factory_function = iter->second.first; functor = factory_function(); // don't set jit compiled, that is only a hack for cling crap // functor->setJITCompiled( true ); @@ -191,7 +191,7 @@ struct FunctorFactory : public extends { // 'full_code'. Instead of Functor( PT > ... ) we ask it to // compile the declaration of a function returning functor_base_t that // takes no arguments. We then ask dlsym to give us the address of - // this function and call it ourselves. This looks like: + // this function and call it ourselves. These functions looks like: // functor_base_t functor_0xdeadbeef() { // return std::make_unique>( PT > ... ); // } @@ -218,6 +218,8 @@ struct FunctorFactory : public extends { std::ostringstream code; + // FIXME we need to make sure this define is passed correctly to the + // compiler wrapper #ifdef USE_DD4HEP code << "#define USE_DD4HEP\n"; #endif @@ -229,6 +231,9 @@ struct FunctorFactory : public extends { // matching the cache entries. auto function_name = Functors::Cache::id( hash ) + "_jit"; + // extern "C" will use C linkage thus avoiding the mangling of function + // names. This makes our life easier when we need to pass the symbol + // name of the function to dlsym to retrieve it from the library. code << "extern \"C\" {\n"; // Declare the factory function @@ -258,6 +263,8 @@ struct FunctorFactory : public extends { // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags // see Phys/FunctorCore/CMakeLists.txt + // the "2>&1" is standard bash magic to redirect stderr to stdout such + // that we only need to read stdout to get the entire output auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } @@ -265,22 +272,34 @@ struct FunctorFactory : public extends { FILE* pipe = popen( cmd.c_str(), "r" ); if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - const size_t buffer_size = 128; + // this is going to feel a bit complicated but we are very much in + // C-Land here when using `popen` so reading the output from the + // process is a bit weird. fgets reads one chunk of output at a time + // this chunk is either up to an encountered "\n" char or until the + // passed buffer is full. thus we should choose a `buffer_size` that + // fits an entire line (512 should be good enough) + const size_t buffer_size = 512; std::array buffer{}; std::string cmd_out; + // as long as fgets has things to read (!= nullptr) read a line into + // buffer and add it to cmd_out while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } auto returnCode = pclose( pipe ); if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + // we don't want this lib to be used to resolve any symbols except what + // we manually get out via dlsym -> use RTLD_LOCAL (would be default + // but specified to make it obvious). RTLD_LAZY is to only load what we + // need (the .so has a bunch of other symbols in it) void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); if ( lib == nullptr ) { throw exception( dlerror() ); } auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); if ( factory_function == nullptr ) { throw exception( dlerror() ); } - // Save the factory function pointer - m_factories.emplace( hash, factory_function ); + // Save the factory function pointer and library handle + m_factories.emplace( hash, std::pair{factory_function, lib} ); // Use the JITted factory function functor = factory_function(); @@ -320,12 +339,14 @@ struct FunctorFactory : public extends { */ StatusCode finalize() override { if ( m_makeCpp ) { writeCpp(); } + for ( auto const& entry : m_factories ) { dlclose( entry.second.second ); } return Service::finalize(); } protected: + using LibHandle = void*; using HashType = Functors::Cache::HashType; - using FactoryCache = std::map; + using FactoryCache = std::map>; using FunctorSet = std::map, std::set>>; FunctorSet m_functors; // {headers: [{hash, type, code}, ...]} FactoryCache m_factories; // {hash: factory function pointer, ...} -- GitLab From 1e1994d8de2f210d2df3a268cf4ba97470f7d621 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 3 Feb 2022 17:35:07 +0100 Subject: [PATCH 05/74] tmp commit to share code --- Phys/FunctorCore/CMakeLists.txt | 109 +++++++++++++++++--- Phys/FunctorCore/src/Components/Factory.cpp | 12 ++- 2 files changed, 103 insertions(+), 18 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 4f09d9be45f..5a5227f0066 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -35,6 +35,7 @@ gaudi_add_library(FunctorCoreLib Rec::DaVinciInterfacesLib Rec::PrKernel Rec::SelKernelLib + Rec::ParticleCombinersLib Rec::SelToolsLib Rec::TrackKernel ROOT::RIO @@ -78,21 +79,103 @@ gaudi_install(PYTHON) gaudi_add_tests(QMTest) gaudi_add_tests(pytest ${CMAKE_CURRENT_SOURCE_DIR}/python) -set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") - -message(STATUS "Generating JIT Wrapper at: ${JIT_CMD_FILE}") string(TOUPPER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_UPPER) -get_property(tmp DIRECTORY PROPERTY COMPILE_DEFINITIONS) -set(defines) -if (tmp) - foreach(item ${tmp}) - list(APPEND defines "-D${item}") - endforeach() -endif() +set(generated_header_name "all_includes.h") +set(preprocessed_header_prefix "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/") +set(preprocessed_header_name "preprocessed_functorfactory_header.ii") +set(preprocessed_header "${preprocessed_header_prefix}${preprocessed_header_name}") + +# file(GENERATE +# OUTPUT ${generated_header_name} +# CONTENT "// Auto-generated header for the FunctorFactory +# #include +# // Functors +# #include \"Functors/Adapters.h\" +# #include \"Functors/Combination.h\" +# #include \"Functors/Composite.h\" +# #include \"Functors/Example.h\" +# #include \"Functors/Filter.h\" +# #include \"Functors/Function.h\" +# #include \"Functors/MVA.h\" +# #include \"Functors/Simulation.h\" +# #include \"Functors/TES.h\" +# #include \"Functors/TrackLike.h\" +# // PhysEvent +# #include \"Event/Particle.h\" +# #include \"Event/Particle_v2.h\" +# #include \"Event/Vertex.h\" +# // TrackEvent +# #include \"Event/PrFittedForwardTracks.h\" +# #include \"Event/Track_v1.h\" +# #include \"Event/Track_v2.h\" +# #include \"Event/Track_v3.h\" +# // TrackKernel +# #include \"TrackKernel/TrackCompactVertex.h\" +# // CombKernel +# #include \"CombKernel/ParticleCombination.h\" +# // SelKernel +# #include \"SelKernel/ParticleCombination.h\" +# #include \"SelKernel/VertexRelation.h\" +# // PrKernel +# #include \"PrKernel/PrSelection.h\" +# ") + +### This would work for our little example +file(GENERATE + OUTPUT ${generated_header_name} + CONTENT "// Auto-generated header for the FunctorFactory +#include \"Functors/Example.h\" +") + +# # generate temporary file because I don't want to waste more time tyring to figure out how to freaking handle stupid whitespace in generator expressions and lists +file(GENERATE + OUTPUT "tmp_preprocessor.sh" + CONTENT "# auto generated +exec ${CMAKE_CXX_COMPILER} -D$>, -D> ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -I$>, -I> -E ${generated_header_name} -o ${preprocessed_header}" +) + + +add_custom_command( + OUTPUT ${preprocessed_header} + COMMAND + sh tmp_preprocessor.sh + # ${CMAKE_CXX_COMPILER} + # # use a ; instead of space to avoid escaped space in command line + # "-D$>,;-D>" + # "-I$>,;-I>" + # # these just won't expand properly !!!!!!!!!!! + # ${CMAKE_CXX_FLAGS} + # ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} + # -E ${generated_header_name} + # -o ${preprocessed_header} + # DEPENDS ${generated_header_name} + # COMMAND_EXPAND_LISTS + # # VERBATIM doesn't see to help either + # VERBATIM + DEPENDS ${generated_header_name} +) +add_custom_target(PreProcHeader ALL DEPENDS ${preprocessed_header} ) + + +set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") +file(GENERATE + OUTPUT ${JIT_CMD_FILE} + CONTENT "#!/usr/bin/env python +# Auto-generated script to create a jitter for the FunctorFactory +import os +import subprocess as sp +import sys + +inc_dir = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__))), \"include\") + +if len(sys.argv) != 3: + raise Exception(\"expect two arguments! e.g. functor_jitter input.cpp output.so\") -# FIXME how the hell does one get a complete list of flags!??!?!? :/ -file(WRITE ${JIT_CMD_FILE} "#!/bin/sh\n# Auto-generated script to create a jitter for the FunctorGigaFactory\nexec ${CMAKE_CXX_COMPILER} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared ${defines} \"$@\"\n") -execute_process(COMMAND chmod a+x ${JIT_CMD_FILE}) +sp.call(\"${CMAKE_CXX_COMPILER} -D$>, -D> ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared -include {}/../include/${preprocessed_header_name} -o {} {}\".format(inc_dir, sys.argv[2], sys.argv[1] ), shell=True) +") +add_custom_target(MAKE_JIT_EXE ALL DEPENDS ${JIT_CMD_FILE} + COMMAND chmod +x ${JIT_CMD_FILE} + ) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index b47900e30b8..48010b30b93 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -225,7 +225,7 @@ struct FunctorFactory : public extends { #endif // Include the required headers - for ( auto const& header : headers ) { include( code, header ); } + // for ( auto const& header : headers ) { include( code, header ); } // Get the name for the factory function. Add a suffix to avoid it // matching the cache entries. @@ -244,8 +244,8 @@ struct FunctorFactory : public extends { if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } - auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); - boost::replace_all( all_includes, ":", " -isystem" ); + // auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); + // boost::replace_all( all_includes, ":", " -isystem" ); auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end @@ -265,7 +265,8 @@ struct FunctorFactory : public extends { // see Phys/FunctorCore/CMakeLists.txt // the "2>&1" is standard bash magic to redirect stderr to stdout such // that we only need to read stdout to get the entire output - auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } @@ -287,12 +288,13 @@ struct FunctorFactory : public extends { auto returnCode = pclose( pipe ); if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Process stdout and stderr:\n" << cmd_out << endmsg; } // we don't want this lib to be used to resolve any symbols except what // we manually get out via dlsym -> use RTLD_LOCAL (would be default // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) - void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + void* lib = dlopen( lib_filename.c_str(), RTLD_GLOBAL | RTLD_LAZY ); if ( lib == nullptr ) { throw exception( dlerror() ); } auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); -- GitLab From a8f229a5cdfa0054bce39fbf5916f65a23826f0e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 4 Feb 2022 15:52:50 +0100 Subject: [PATCH 06/74] include many headers and a few libs, python lauch script seems to be working --- Phys/FunctorCore/CMakeLists.txt | 117 ++++++++++++-------- Phys/FunctorCore/src/Components/Factory.cpp | 4 +- 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 5a5227f0066..631d9e4dd21 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -87,53 +87,63 @@ set(preprocessed_header_prefix "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDED set(preprocessed_header_name "preprocessed_functorfactory_header.ii") set(preprocessed_header "${preprocessed_header_prefix}${preprocessed_header_name}") -# file(GENERATE -# OUTPUT ${generated_header_name} -# CONTENT "// Auto-generated header for the FunctorFactory -# #include -# // Functors -# #include \"Functors/Adapters.h\" -# #include \"Functors/Combination.h\" -# #include \"Functors/Composite.h\" -# #include \"Functors/Example.h\" -# #include \"Functors/Filter.h\" -# #include \"Functors/Function.h\" -# #include \"Functors/MVA.h\" -# #include \"Functors/Simulation.h\" -# #include \"Functors/TES.h\" -# #include \"Functors/TrackLike.h\" -# // PhysEvent -# #include \"Event/Particle.h\" -# #include \"Event/Particle_v2.h\" -# #include \"Event/Vertex.h\" -# // TrackEvent -# #include \"Event/PrFittedForwardTracks.h\" -# #include \"Event/Track_v1.h\" -# #include \"Event/Track_v2.h\" -# #include \"Event/Track_v3.h\" -# // TrackKernel -# #include \"TrackKernel/TrackCompactVertex.h\" -# // CombKernel -# #include \"CombKernel/ParticleCombination.h\" -# // SelKernel -# #include \"SelKernel/ParticleCombination.h\" -# #include \"SelKernel/VertexRelation.h\" -# // PrKernel -# #include \"PrKernel/PrSelection.h\" -# ") - -### This would work for our little example -file(GENERATE - OUTPUT ${generated_header_name} +# this list needs to include enough headers such that any functor expression +# can compile +file(GENERATE + OUTPUT ${generated_header_name} CONTENT "// Auto-generated header for the FunctorFactory +#include +// Functors +#include \"Functors/Adapters.h\" +#include \"Functors/Combination.h\" +#include \"Functors/Composite.h\" #include \"Functors/Example.h\" +#include \"Functors/Filter.h\" +#include \"Functors/Function.h\" +#include \"Functors/MVA.h\" +#include \"Functors/Simulation.h\" +#include \"Functors/TES.h\" +#include \"Functors/TrackLike.h\" +// PhysEvent +#include \"Event/Particle.h\" +#include \"Event/Particle_v2.h\" +#include \"Event/Vertex.h\" +// TrackEvent +#include \"Event/PrFittedForwardTracks.h\" +#include \"Event/Track_v1.h\" +#include \"Event/Track_v2.h\" +#include \"Event/Track_v3.h\" +// TrackKernel +#include \"TrackKernel/TrackCompactVertex.h\" +// CombKernel +#include \"CombKernel/ParticleCombination.h\" +// SelTools +#include \"SelTools/MatrixNet.h\" +// SelKernel +#include \"SelKernel/ParticleCombination.h\" +#include \"SelKernel/VertexRelation.h\" +// PrKernel +#include \"PrKernel/PrSelection.h\" ") -# # generate temporary file because I don't want to waste more time tyring to figure out how to freaking handle stupid whitespace in generator expressions and lists + +# do we need the -pthread flag? it is in here: +#$> + +# # generate temporary file because I don't want to waste more time tyring to +# figure out how to freaking handle stupid whitespace in generator expressions +# and lists file(GENERATE OUTPUT "tmp_preprocessor.sh" CONTENT "# auto generated -exec ${CMAKE_CXX_COMPILER} -D$>, -D> ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -I$>, -I> -E ${generated_header_name} -o ${preprocessed_header}" +exec ${CMAKE_CXX_COMPILER} \ +-D$>, -D> \ +${CMAKE_CXX_FLAGS} \ +${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ +-I$>,INCLUDE,/Rec/>, -I> \ +-isystem $>,EXCLUDE,/Rec/>, -isystem > \ +-E ${generated_header_name} \ +-o ${preprocessed_header}" ) @@ -154,10 +164,15 @@ add_custom_command( # COMMAND_EXPAND_LISTS # # VERBATIM doesn't see to help either # VERBATIM - DEPENDS ${generated_header_name} + DEPENDS ${generated_header_name} "tmp_preprocessor.sh" ) add_custom_target(PreProcHeader ALL DEPENDS ${preprocessed_header} ) +# avoid "warning: style of line directive is a GCC extension" because we +# include a preprocessed header. Are there better solutions? We could first +# precompile the preprocessed header in initalize() and then use that pch... +# something for later +string(REPLACE " -pedantic" "" no_pedantic ${CMAKE_CXX_FLAGS}) set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") file(GENERATE @@ -168,14 +183,24 @@ import os import subprocess as sp import sys -inc_dir = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__))), \"include\") +base_dir = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) -if len(sys.argv) != 3: - raise Exception(\"expect two arguments! e.g. functor_jitter input.cpp output.so\") +# we know all our libs will be on the LD_LIBRARY_PATH so just point the linker there +my_env = os.environ.copy() +my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] -sp.call(\"${CMAKE_CXX_COMPILER} -D$>, -D> ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared -include {}/../include/${preprocessed_header_name} -o {} {}\".format(inc_dir, sys.argv[2], sys.argv[1] ), shell=True) +if len(sys.argv) != 3: + raise Exception('expect two arguments! e.g. functor_jitter input.cpp output.so') + +sp.call('${CMAKE_CXX_COMPILER} \ +-D$>, -D> \ +${no_pedantic} \ +${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ +-include {0}/include/${preprocessed_header_name} \ +-lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent \ +-o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env) ") add_custom_target(MAKE_JIT_EXE ALL DEPENDS ${JIT_CMD_FILE} COMMAND chmod +x ${JIT_CMD_FILE} - ) +) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 48010b30b93..70729a63f96 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -295,10 +295,10 @@ struct FunctorFactory : public extends { // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) void* lib = dlopen( lib_filename.c_str(), RTLD_GLOBAL | RTLD_LAZY ); - if ( lib == nullptr ) { throw exception( dlerror() ); } + if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); - if ( factory_function == nullptr ) { throw exception( dlerror() ); } + if ( factory_function == nullptr ) { throw exception( std::string("dlsym Error:\n") + dlerror() ); } // Save the factory function pointer and library handle m_factories.emplace( hash, std::pair{factory_function, lib} ); -- GitLab From bf5e28970c4c598595a89c37e93a4e0211ef7c61 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 11 Feb 2022 18:52:41 +0100 Subject: [PATCH 07/74] first prototype of working pooled compilation in start(), lots of cleanup left to do --- Phys/FunctorCore/include/Functors/IFactory.h | 29 ++++ .../FunctorCore/src/Components/ExampleAlg.cpp | 4 +- Phys/FunctorCore/src/Components/Factory.cpp | 142 +++++++++++++++++- 3 files changed, 172 insertions(+), 3 deletions(-) diff --git a/Phys/FunctorCore/include/Functors/IFactory.h b/Phys/FunctorCore/include/Functors/IFactory.h index 339d1e3074f..909c68bf6c9 100644 --- a/Phys/FunctorCore/include/Functors/IFactory.h +++ b/Phys/FunctorCore/include/Functors/IFactory.h @@ -61,6 +61,9 @@ namespace Functors { virtual functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; + virtual void do_register( std::function do_copy, std::string_view functor_type, + ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; + public: DeclareInterfaceID( IFactory, 1, 0 ); @@ -107,5 +110,31 @@ namespace Functors { // should not abort the application... return {}; } + + template + void register_functor( Gaudi::Algorithm* owner, FType& functor, ThOr::FunctorDesc const& desc, + CompilationBehaviour compile = DefaultCompilationBehaviour ) { + + auto do_copy = [owner, &functor]( Functors::AnyFunctor* b ) { + auto ftype_ptr = dynamic_cast( b ); // cast AnyFunctor* -> FType* (base -> derived) + + if ( !ftype_ptr ) { + // This should only happen if you have a bug (e.g. you used a + // SIMDWrapper type that has a different meaning depending on the + // compilation flags in the stack/cling). We can't fix that at + // runtime so let's just fail hard. + throw GaudiException{"Failed to cast factory return type (" + + System::typeinfoName( typeid( decltype( *b ) ) ) + ") to desired type (" + + System::typeinfoName( typeid( FType ) ) + "), rtype is (" + + System::typeinfoName( b->rtype() ) + ") and it " + + ( b->wasJITCompiled() ? "was" : "was not" ) + " JIT compiled", + "Functors::IFactory::get( owner, desc, compile )", StatusCode::FAILURE}; + } + functor = std::move( *ftype_ptr ); + functor.bind( owner ); + }; + + do_register( do_copy, System::typeinfoName( typeid( FType ) ), desc, compile ); + } }; } // namespace Functors diff --git a/Phys/FunctorCore/src/Components/ExampleAlg.cpp b/Phys/FunctorCore/src/Components/ExampleAlg.cpp index 07072d0b139..6f72783b50a 100644 --- a/Phys/FunctorCore/src/Components/ExampleAlg.cpp +++ b/Phys/FunctorCore/src/Components/ExampleAlg.cpp @@ -69,7 +69,7 @@ private: void decode() { m_factory.retrieve().ignore(); - m_pred = m_factory->get( this, m_functorproxy ); + m_factory->register_functor( this, m_pred, m_functorproxy ); } }; @@ -118,7 +118,7 @@ private: void decode() { m_factory.retrieve().ignore(); - m_pred = m_factory->get( this, m_functorproxy ); + m_factory->register_functor( this, m_pred, m_functorproxy ); } }; DECLARE_COMPONENT_WITH_ID( FunctorExampleAlg<>, "FunctorExampleAlg" ) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 70729a63f96..3f41aacffdd 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -104,6 +104,62 @@ struct FunctorFactory : public extends { // pointer-to-function type we use when JIT compiling using factory_function_ptr = functor_base_t ( * )(); + void do_register( std::function do_copy, std::string_view functor_type, + ThOr::FunctorDesc const& desc, CompilationBehaviour ) override { + // FIXME if we are called from somewhere after start() we need to throw. + // register needs to happen before start() + + // FIXME it seems that having a quoted string in the middle of the + // string adds quotes at the start/end...need Gaudi!919 + std::size_t findex{desc.code.front() == '"'}; + std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; + auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); + + // This is basically Functor( PTCUT( ... ) ... ) + std::string full_code{functor_type}; + full_code.append( "( " ); + full_code.append( trimmed_code ); + full_code.append( " )" ); + + // Now we can calculate the hash + const auto hash = Functors::Cache::makeHash( full_code ); + + if ( msgLevel( MSG::VERBOSE ) ) { + verbose() << "Full string for hash: " << full_code << endmsg; + verbose() << "Resulting hash is " << std::hex << std::showbase << hash << endmsg; + } + + // See if we already JIT compiled this functor and can therefore reuse + // the factory function that we got before + auto iter = m_all.find( hash ); + if ( iter != m_all.end() ) { + info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; + iter->second.first.push_back( do_copy ); + + } else { + + std::ostringstream code; + + // Get the name for the factory function. Add a suffix to avoid it + // matching the cache entries. + auto function_name = Functors::Cache::id( hash ); + + // extern "C" will use C linkage thus avoiding the mangling of function + // names. This makes our life easier when we need to pass the symbol + // name of the function to dlsym to retrieve it from the library. + // code << "extern \"C\" {\n"; + // code << "}"; + + // Declare the factory function + code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " + << trimmed_code << " ); }\n"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } + + m_all.emplace( hash, std::pair{std::vector{do_copy}, code.str()} ); + } + } + functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour compile ) override { // Combine the 'compile' argument with the global settings to determine @@ -298,7 +354,7 @@ struct FunctorFactory : public extends { if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); - if ( factory_function == nullptr ) { throw exception( std::string("dlsym Error:\n") + dlerror() ); } + if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } // Save the factory function pointer and library handle m_factories.emplace( hash, std::pair{factory_function, lib} ); @@ -345,6 +401,89 @@ struct FunctorFactory : public extends { return Service::finalize(); } + StatusCode start() override { + auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; + + std::cout << "START" << std::endl; + for ( auto const& entry : m_all ) { + info() << "Hash: " << entry.first << endmsg; + info() << "#registers " << entry.second.first.size() << endmsg; + info() << entry.second.second << endmsg; + } + + std::ostringstream code; + code << "extern \"C\" {\n"; + for ( auto const& entry : m_all ) { code << entry.second.second; } + code << "}"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } + + auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); + // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end + // if not, default to "/tmp/" + tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; + + // FIXME make the name a hash of all hashes and enable reuse + auto const file_prefix = tmp_dir + "FunctorJIT"; + auto const cpp_filename = file_prefix + ".cpp"; + auto const lib_filename = file_prefix + ".so"; + { + std::ofstream out( cpp_filename ); + out << code.str(); + out.close(); + } + + // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags + // see Phys/FunctorCore/CMakeLists.txt + // the "2>&1" is standard bash magic to redirect stderr to stdout such + // that we only need to read stdout to get the entire output + // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } + + FILE* pipe = popen( cmd.c_str(), "r" ); + if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } + + // this is going to feel a bit complicated but we are very much in + // C-Land here when using `popen` so reading the output from the + // process is a bit weird. fgets reads one chunk of output at a time + // this chunk is either up to an encountered "\n" char or until the + // passed buffer is full. thus we should choose a `buffer_size` that + // fits an entire line (512 should be good enough) + const size_t buffer_size = 512; + std::array buffer{}; + std::string cmd_out; + // as long as fgets has things to read (!= nullptr) read a line into + // buffer and add it to cmd_out + while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } + + auto returnCode = pclose( pipe ); + if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Process stdout and stderr:\n" << cmd_out << endmsg; } + + // we don't want this lib to be used to resolve any symbols except what + // we manually get out via dlsym -> use RTLD_LOCAL (would be default + // but specified to make it obvious). RTLD_LAZY is to only load what we + // need (the .so has a bunch of other symbols in it) + void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } + + for ( auto const& entry : m_all ) { + + auto function_name = Functors::Cache::id( entry.first ); + auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); + if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } + + for ( auto const& copy_func : entry.second.first ) { + auto functor = factory_function(); + copy_func( functor.get() ); + } + } + + return Service::start(); + } + protected: using LibHandle = void*; using HashType = Functors::Cache::HashType; @@ -352,6 +491,7 @@ protected: using FunctorSet = std::map, std::set>>; FunctorSet m_functors; // {headers: [{hash, type, code}, ...]} FactoryCache m_factories; // {hash: factory function pointer, ...} + std::map>, std::string>> m_all; /** @brief Generate the functor cache .cpp files. * -- GitLab From 274b8740ff0d8eeff6f018d973ce357ef0af3e8d Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 14 Feb 2022 16:31:17 +0100 Subject: [PATCH 08/74] disable FunctorCache --- Phys/FunctorCache/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Phys/FunctorCache/CMakeLists.txt b/Phys/FunctorCache/CMakeLists.txt index 50d88425a4b..280d0811474 100644 --- a/Phys/FunctorCache/CMakeLists.txt +++ b/Phys/FunctorCache/CMakeLists.txt @@ -13,7 +13,7 @@ Phys/FunctorCache ----------------- #]=======================================================================] -option(THOR_BUILD_TEST_FUNCTOR_CACHE "Build functor cache for THOR functors" ON) +option(THOR_BUILD_TEST_FUNCTOR_CACHE "Build functor cache for THOR functors" OFF) if(THOR_BUILD_TEST_FUNCTOR_CACHE) -- GitLab From 574d3be827eaf47ee5f3b7cf49cdd1a02b469555 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 14 Feb 2022 16:32:12 +0100 Subject: [PATCH 09/74] include missing header and fix python script to forward return code on compilation failure --- Phys/FunctorCore/CMakeLists.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 631d9e4dd21..61fc52fd398 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -119,6 +119,7 @@ file(GENERATE #include \"CombKernel/ParticleCombination.h\" // SelTools #include \"SelTools/MatrixNet.h\" +#include \"SelTools/SigmaNet.h\" // SelKernel #include \"SelKernel/ParticleCombination.h\" #include \"SelKernel/VertexRelation.h\" @@ -192,13 +193,13 @@ my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] if len(sys.argv) != 3: raise Exception('expect two arguments! e.g. functor_jitter input.cpp output.so') -sp.call('${CMAKE_CXX_COMPILER} \ +exit(sp.call('${CMAKE_CXX_COMPILER} \ -D$>, -D> \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -include {0}/include/${preprocessed_header_name} \ -lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent \ --o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env) +-o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env)) ") add_custom_target(MAKE_JIT_EXE ALL DEPENDS ${JIT_CMD_FILE} -- GitLab From 54eb84decaa287830178479e81dd47154e667c45 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 14 Feb 2022 16:39:22 +0100 Subject: [PATCH 10/74] improve pooled compilation to check for exisiting lib, remove old get<> interface --- Phys/FunctorCore/include/Functors/Cache.h | 3 +- Phys/FunctorCore/include/Functors/IFactory.h | 58 +- .../include/Functors/with_functor_maps.h | 13 +- .../include/Functors/with_functors.h | 12 +- Phys/FunctorCore/src/Cache.cpp | 6 +- Phys/FunctorCore/src/Components/Factory.cpp | 521 +++--------------- .../tests/options/test_functors.py | 1 - 7 files changed, 108 insertions(+), 506 deletions(-) diff --git a/Phys/FunctorCore/include/Functors/Cache.h b/Phys/FunctorCore/include/Functors/Cache.h index 66f61a60202..b493803bd8a 100644 --- a/Phys/FunctorCore/include/Functors/Cache.h +++ b/Phys/FunctorCore/include/Functors/Cache.h @@ -29,5 +29,6 @@ namespace Functors::Cache { /** @brief Generate a Gaudi component name from a hash. */ - std::string id( HashType hash ); + std::string hashToStr( HashType hash ); + std::string hashToFuncName( HashType hash ); } // namespace Functors::Cache diff --git a/Phys/FunctorCore/include/Functors/IFactory.h b/Phys/FunctorCore/include/Functors/IFactory.h index 909c68bf6c9..424cdd9b799 100644 --- a/Phys/FunctorCore/include/Functors/IFactory.h +++ b/Phys/FunctorCore/include/Functors/IFactory.h @@ -55,62 +55,25 @@ namespace Functors { protected: using functor_base_t = std::unique_ptr; constexpr static auto functor_base_t_str = "std::unique_ptr"; - /** Implementation method that gets an input/output-type-agnostic std::unique_ptr - * object from either cling or the cache. + /** Implementation method that registers an input/output-type-agnostic std::unique_ptr + * object with the factory */ - virtual functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, - ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; - virtual void do_register( std::function do_copy, std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; public: DeclareInterfaceID( IFactory, 1, 0 ); - /** Factory method to get a C++ functor object from a string, either from - * the cache or using JIT compilation. + /** Factory method to register a C++ functor object JIT compilation. * * @param owner The algorithm that owns the functor, this is needed to * set up the functor's data dependencies correctly. - * @param desc ThOr::FunctorDesc object holding the functor code, list - * of headers required to compile it and "pretty" - * representation. - * @param compile CompilationBehaviour enum value specifying what should - * be tried when compiling this functor. By default the - * functor cache will be tried first, and the factory will - * fall back on JIT compilation. This does not override the - * global settings of the factory. - * @tparam FType Functor type that will be returned. This - * specifies precisely how the functor is instantiated. - * @return Functor object of the given type, may be empty. + * @param functor Functor of type FType that will be set by in + * the FunctorFactories start() call. + * Note: The functor is not usable until after Factory->start()!! + * @param desc ThOr::FunctorDesc object holding the functor code + * and "pretty" representation. */ - template - FType get( Gaudi::Algorithm* owner, ThOr::FunctorDesc const& desc, - CompilationBehaviour compile = DefaultCompilationBehaviour ) { - auto any_functor = get_impl( owner, System::typeinfoName( typeid( FType ) ), desc, compile ); - if ( any_functor ) { // check the unique_ptr isn't empty - auto ftype_ptr = dynamic_cast( any_functor.get() ); // cast AnyFunctor* -> FType* (base -> derived) - if ( ftype_ptr ) { // check the AnyFunctor -> Functor conversion was OK - return std::move( *ftype_ptr ); // move the contents into the FType we return by value - } else { - // This should only happen if you have a bug (e.g. you used a - // SIMDWrapper type that has a different meaning depending on the - // compilation flags in the stack/cling). We can't fix that at - // runtime so let's just fail hard. - throw GaudiException{"Failed to cast factory return type (" + - System::typeinfoName( typeid( decltype( *any_functor.get() ) ) ) + - ") to desired type (" + System::typeinfoName( typeid( FType ) ) + "), rtype is (" + - System::typeinfoName( any_functor->rtype() ) + ") and it " + - ( any_functor->wasJITCompiled() ? "was" : "was not" ) + " JIT compiled", - "Functors::IFactory::get( owner, desc, compile )", StatusCode::FAILURE}; - } - } - // Return an empty FType object. This can happen if e.g. you disabled - // both cling and the cache, as is done during cache generation, so we - // should not abort the application... - return {}; - } - template void register_functor( Gaudi::Algorithm* owner, FType& functor, ThOr::FunctorDesc const& desc, CompilationBehaviour compile = DefaultCompilationBehaviour ) { @@ -126,9 +89,8 @@ namespace Functors { throw GaudiException{"Failed to cast factory return type (" + System::typeinfoName( typeid( decltype( *b ) ) ) + ") to desired type (" + System::typeinfoName( typeid( FType ) ) + "), rtype is (" + - System::typeinfoName( b->rtype() ) + ") and it " + - ( b->wasJITCompiled() ? "was" : "was not" ) + " JIT compiled", - "Functors::IFactory::get( owner, desc, compile )", StatusCode::FAILURE}; + System::typeinfoName( b->rtype() ) + ") ", + "Functors::IFactory::register_functor( owner, functor, desc)", StatusCode::FAILURE}; } functor = std::move( *ftype_ptr ); functor.bind( owner ); diff --git a/Phys/FunctorCore/include/Functors/with_functor_maps.h b/Phys/FunctorCore/include/Functors/with_functor_maps.h index f613a45e15f..26a170f2cd5 100644 --- a/Phys/FunctorCore/include/Functors/with_functor_maps.h +++ b/Phys/FunctorCore/include/Functors/with_functor_maps.h @@ -36,21 +36,14 @@ class with_functor_maps : public Functors::detail::with_functor_factory template void decode() { - using TagType = boost::mp11::mp_at_c; - using FunctorType = boost::mp11::mp_at_c; + using TagType = boost::mp11::mp_at_c; // This is the {nickname: decoded_functor} map we want to populate auto& functor_map = std::get( m_functors ); // Clean it up each time in case we re-decode functor_map.clear(); for ( auto const& [func_name, func_desc] : m_properties.template get() ) { - // Local copy we can add extra headers to - ThOr::FunctorDesc proxy{func_desc}; - if constexpr ( Functors::detail::has_extra_headers_v ) { - for ( auto const& h : TagType::ExtraHeaders ) { proxy.headers.emplace_back( h ); } - } - // Decode the functor - functor_map[func_name] = this->getFunctorFactory().template get( - this, proxy, Functors::detail::get_compilation_behaviour_v ); + this->getFunctorFactory().register_functor( this, functor_map[func_name], func_desc, + Functors::detail::get_compilation_behaviour_v ); } } diff --git a/Phys/FunctorCore/include/Functors/with_functors.h b/Phys/FunctorCore/include/Functors/with_functors.h index 4ef70af7018..2d47184d5da 100644 --- a/Phys/FunctorCore/include/Functors/with_functors.h +++ b/Phys/FunctorCore/include/Functors/with_functors.h @@ -147,16 +147,10 @@ private: template void decode() { - using TagType = std::tuple_element_t; - using FunctorType = std::tuple_element_t; + using TagType = std::tuple_element_t; // Make a copy, as we might need to add headers to it - ThOr::FunctorDesc proxy = m_properties.template get(); - // Add extra headers if needed - if constexpr ( Functors::detail::has_extra_headers_v ) { - for ( auto const& h : TagType::ExtraHeaders ) { proxy.headers.emplace_back( h ); } - } - std::get( m_functors ) = this->getFunctorFactory().template get( - this, proxy, Functors::detail::get_compilation_behaviour_v ); + this->getFunctorFactory().register_functor( this, std::get( m_functors ), m_properties.template get(), + Functors::detail::get_compilation_behaviour_v ); } // Storage for the decoded functors diff --git a/Phys/FunctorCore/src/Cache.cpp b/Phys/FunctorCore/src/Cache.cpp index 591e4c1d559..01913f2143b 100644 --- a/Phys/FunctorCore/src/Cache.cpp +++ b/Phys/FunctorCore/src/Cache.cpp @@ -25,9 +25,11 @@ namespace Functors::Cache { HashType makeHash( std::string_view data ) { return sv_hash( data ); } // Function used to generate the Gaudi component names from the hashes - std::string id( HashType hash ) { + std::string hashToStr( HashType hash ) { std::ostringstream oss; - oss << "functor_" << std::showbase << std::hex << hash; + oss << std::showbase << std::hex << hash; return oss.str(); } + std::string hashToFuncName( HashType hash ) { return "functor_" + hashToStr( hash ); } + } // namespace Functors::Cache diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 3f41aacffdd..40662f0d97a 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -12,13 +12,8 @@ #include "Functors/FunctorDesc.h" #include "Functors/IFactory.h" #include "GaudiKernel/Service.h" -#include "boost/algorithm/string/erase.hpp" -#include "boost/algorithm/string/predicate.hpp" -#include "boost/algorithm/string/replace.hpp" -#include "boost/algorithm/string/split.hpp" -#include "boost/algorithm/string/trim.hpp" -#include "boost/format.hpp" -#include "boost/lexical_cast.hpp" +#include +#include #include #include #include @@ -29,66 +24,6 @@ * @brief Definitions of non-templated functor factory functions. */ -namespace { - std::ostream& include( std::ostream& os, std::string_view header ) { - if ( header.empty() ) { throw GaudiException{"Got empty header name", "FunctorFactory", StatusCode::FAILURE}; } - os << "#include "; - if ( header.front() == '<' && header.back() == '>' ) { - os << header; - } else { - os << '"' << header << '"'; - } - return os << '\n'; - } - - std::string make_default_cppname( std::string cppname ) { - if ( boost::algorithm::starts_with( cppname, "ToolSvc." ) ) { cppname.erase( 0, 8 ); } - std::replace_if( - cppname.begin(), cppname.end(), []( char c ) { return c == '.' || c == ' '; }, '_' ); - boost::algorithm::replace_all( cppname, "::", "__" ); - cppname.insert( 0, "FUNCTORS_" ); - return cppname; - } - std::unique_ptr openFile( std::string namebase, unsigned short findex, - std::vector const& lines, - std::vector const& headers ) { - // construct the file name - // 1) remove trailing .cpp - if ( boost::algorithm::ends_with( namebase, ".cpp" ) ) boost::algorithm::erase_tail( namebase, 4 ); - // 2) replace blanks by underscore - std::replace( namebase.begin(), namebase.end(), ' ', '_' ); - // 3) construct the name - boost::format fname( "%s_%04d.cpp" ); - fname % namebase % findex; - auto file = std::make_unique( fname.str() ); - // write the include directives - *file << "// Generic statements\n"; - for ( const auto& l : lines ) { *file << l << '\n'; } - *file << "// Explicitly declared include files:\n"; - for ( const auto& h : headers ) { include( *file, h ); } - return file; - } - - /** Write out the entry in a generated functor cache .cpp file for a single functor. - */ - std::ostream& makeCode( std::ostream& stream, Functors::Cache::HashType hash, std::string_view type, - std::string_view code ) { - boost::format declareFactory{R"_(namespace { - using t_%1% = %2%; - Gaudi::PluginService::DeclareFactory functor_%1%{ - Functors::Cache::id( %1% ), - []() -> std::unique_ptr { - return std::make_unique( - %3% - ); - } - }; -} -)_"}; - return stream << ( declareFactory % boost::io::group( std::showbase, std::hex, hash ) % type % code ); - } -} // namespace - /** @class FunctorFactory * * This service does all the heavy lifting behind compiling functors into @@ -133,7 +68,7 @@ struct FunctorFactory : public extends { // the factory function that we got before auto iter = m_all.find( hash ); if ( iter != m_all.end() ) { - info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; + info() << "Found already registered functor: " << trimmed_code << endmsg; iter->second.first.push_back( do_copy ); } else { @@ -142,336 +77,113 @@ struct FunctorFactory : public extends { // Get the name for the factory function. Add a suffix to avoid it // matching the cache entries. - auto function_name = Functors::Cache::id( hash ); - - // extern "C" will use C linkage thus avoiding the mangling of function - // names. This makes our life easier when we need to pass the symbol - // name of the function to dlsym to retrieve it from the library. - // code << "extern \"C\" {\n"; - // code << "}"; + auto function_name = Functors::Cache::hashToFuncName( hash ); // Declare the factory function code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " << trimmed_code << " ); }\n"; + info() << "Registering functor: " << trimmed_code << endmsg; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } m_all.emplace( hash, std::pair{std::vector{do_copy}, code.str()} ); } } - functor_base_t get_impl( Gaudi::Algorithm* owner, std::string_view functor_type, ThOr::FunctorDesc const& desc, - CompilationBehaviour compile ) override { - // Combine the 'compile' argument with the global settings to determine - // what compilation methods we should try - bool const fail_hard = compile & ExceptionOnFailure; - bool const use_jit{this->m_use_jit && ( compile & TryJIT )}; - bool const use_cache{this->m_use_cache && ( compile & TryCache )}; - - // Prepare the string that fully specifies the functor we want to retrieve - // -- basically the combination of input type, output type, functor string - // But first: sort and de-duplicate the headers - auto headers = desc.headers; - std::sort( headers.begin(), headers.end() ); - headers.erase( std::unique( headers.begin(), headers.end() ), headers.end() ); - if ( msgLevel( MSG::DEBUG ) ) { - debug() << "Decoding " << desc.code << endmsg; - debug() << "With extra headers:"; - for ( auto const& header_line : headers ) { debug() << " " << header_line; } - debug() << endmsg; + StatusCode start() override { + auto sc = Service::start(); + if ( m_all.empty() ) { + return sc; + } else { + info() << m_all.size() << " functors were registered" << endmsg; } - // FIXME it seems that having a quoted string in the middle of the - // string adds quotes at the start/end...need Gaudi!919 - std::size_t findex{desc.code.front() == '"'}; - std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; - auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); - - // This is basically Functor( PTCUT( ... ) ... ) - std::string full_code{functor_type}; - full_code.append( "( " ); - full_code.append( trimmed_code ); - full_code.append( " )" ); - - // Now we can calculate the hash - const auto hash = Functors::Cache::makeHash( full_code ); + auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; if ( msgLevel( MSG::VERBOSE ) ) { - verbose() << "Full string for hash: " << full_code << endmsg; - verbose() << "Resulting hash is " << std::hex << std::showbase << hash << endmsg; - } - - // The object we'll eventually return - functor_base_t functor; - - // See if we can magically load the functor from the cache (Gaudi magic!) - // Don't bother trying if we were told not to - if ( use_cache ) { - functor = ::Gaudi::PluginService::Factory::create( Functors::Cache::id( hash ) ); - if ( functor ) { - functor->setJITCompiled( false ); - } else if ( !functor && !use_jit ) { - // We print a different INFO message below if use_jit is true - info() << "Cache miss for functor: " << trimmed_code << endmsg; - } - } - - // Shorthand for throwing an informative exception - auto exception = [functor_type]( auto const& name ) { - std::string ourname{"FunctorFactory::get<"}; - ourname.append( functor_type ); - ourname.append( ">" ); - return GaudiException{name, std::move( ourname ), StatusCode::FAILURE}; - }; - - if ( !functor && use_jit ) { - // See if we already JIT compiled this functor and can therefore reuse - // the factory function that we got before - auto iter = m_factories.find( hash ); - if ( iter != m_factories.end() ) { - // We had already JIT compiled this functor - info() << "Reusing jit compiled factory for functor: " << trimmed_code << endmsg; - auto factory_function = iter->second.first; - functor = factory_function(); - // don't set jit compiled, that is only a hack for cling crap - // functor->setJITCompiled( true ); - } else { - // Need to actually do the JIT compilation - if ( use_cache ) { - info() << "Cache miss for functor: " << trimmed_code << ", now trying JIT with headers " << headers << endmsg; - } else { - info() << "Using compiler for functor: " << trimmed_code << " with headers " << headers << endmsg; - } - - // The expression we ask the compiler to compile is not quite the same as - // 'full_code'. Instead of Functor( PT > ... ) we ask it to - // compile the declaration of a function returning functor_base_t that - // takes no arguments. We then ask dlsym to give us the address of - // this function and call it ourselves. These functions looks like: - // functor_base_t functor_0xdeadbeef() { - // return std::make_unique>( PT > ... ); - // } - - // FIXME - // FIXME - // 1. factor out the shared code with the other Factory - // - // 2. what to do about dlclose? technically as long as this service - // lives there could be someone who comes and asks again for a - // functor we already opened, so we shouldn't really dlclose before - // finalize ? But at finalize we will quit anyways so I guess for - // now I'll just leak :D Other option to avoid uncontrolled growing - // of memory would be to set a maximum of allowed open handles? - // We could implement some reference counting to keep track if a - // library is still in use and otherwise close it. - // - // 3. right now we create 1 lib per functor, can we somehow cluster - // this? - // - // - // FIXME - // FIXME - - std::ostringstream code; - - // FIXME we need to make sure this define is passed correctly to the - // compiler wrapper -#ifdef USE_DD4HEP - code << "#define USE_DD4HEP\n"; -#endif - - // Include the required headers - // for ( auto const& header : headers ) { include( code, header ); } - - // Get the name for the factory function. Add a suffix to avoid it - // matching the cache entries. - auto function_name = Functors::Cache::id( hash ) + "_jit"; - - // extern "C" will use C linkage thus avoiding the mangling of function - // names. This makes our life easier when we need to pass the symbol - // name of the function to dlsym to retrieve it from the library. - code << "extern \"C\" {\n"; - - // Declare the factory function - code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " - << trimmed_code << " ); }\n"; - - code << "}"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } - - // auto all_includes = "-isystem" + System::getEnv( "ROOT_INCLUDE_PATH" ); - // boost::replace_all( all_includes, ":", " -isystem" ); - - auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); - // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end - // if not, default to "/tmp/" - tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; - - auto const file_prefix = tmp_dir + function_name; - auto const cpp_filename = file_prefix + ".cpp"; - auto const lib_filename = file_prefix + ".so"; - { - std::ofstream out( cpp_filename ); - out << code.str(); - out.close(); - } - - // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags - // see Phys/FunctorCore/CMakeLists.txt - // the "2>&1" is standard bash magic to redirect stderr to stdout such - // that we only need to read stdout to get the entire output - // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; - auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } - - FILE* pipe = popen( cmd.c_str(), "r" ); - if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - - // this is going to feel a bit complicated but we are very much in - // C-Land here when using `popen` so reading the output from the - // process is a bit weird. fgets reads one chunk of output at a time - // this chunk is either up to an encountered "\n" char or until the - // passed buffer is full. thus we should choose a `buffer_size` that - // fits an entire line (512 should be good enough) - const size_t buffer_size = 512; - std::array buffer{}; - std::string cmd_out; - // as long as fgets has things to read (!= nullptr) read a line into - // buffer and add it to cmd_out - while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } - - auto returnCode = pclose( pipe ); - if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } - if ( msgLevel( MSG::DEBUG ) ) { debug() << "Process stdout and stderr:\n" << cmd_out << endmsg; } - - // we don't want this lib to be used to resolve any symbols except what - // we manually get out via dlsym -> use RTLD_LOCAL (would be default - // but specified to make it obvious). RTLD_LAZY is to only load what we - // need (the .so has a bunch of other symbols in it) - void* lib = dlopen( lib_filename.c_str(), RTLD_GLOBAL | RTLD_LAZY ); - if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } - - auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); - if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } - - // Save the factory function pointer and library handle - m_factories.emplace( hash, std::pair{factory_function, lib} ); - - // Use the JITted factory function - functor = factory_function(); - functor->setJITCompiled( true ); - } - } - - if ( functor ) { - functor->bind( owner ); - } else if ( fail_hard && ( use_cache || use_jit ) ) { - // Don't emit too many messages while generating the functor caches. In - // that case both JIT and the cache are disabled, so we will never - // actually retrieve a functor here. - std::string error_message{"Couldn't load functor using ["}; - if ( use_cache && use_jit ) { - error_message += "cache, JIT"; - } else if ( use_cache ) { - error_message += "cache"; - } else { - error_message += "JIT"; + for ( auto const& entry : m_all ) { + verbose() << "Hash: " << entry.first << endmsg; + verbose() << "#users " << entry.second.first.size() << endmsg; + verbose() << entry.second.second << endmsg; } - error_message += "]: " + desc.repr; - throw exception( error_message ); - } - - // If we're going to write out the .cpp files for creating the functor cache when we finalise then we need to - // store the relevant data in an internal structure - if ( this->m_makeCpp ) { - // Store the functor alongside others with the same headers - m_functors[std::move( headers )].emplace( hash, functor_type, trimmed_code ); - } - - return functor; - } - - /** Write out the C++ files needed to compile the functor cache if needed. - */ - StatusCode finalize() override { - if ( m_makeCpp ) { writeCpp(); } - for ( auto const& entry : m_factories ) { dlclose( entry.second.second ); } - return Service::finalize(); - } - - StatusCode start() override { - auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; - - std::cout << "START" << std::endl; - for ( auto const& entry : m_all ) { - info() << "Hash: " << entry.first << endmsg; - info() << "#registers " << entry.second.first.size() << endmsg; - info() << entry.second.second << endmsg; } - std::ostringstream code; - code << "extern \"C\" {\n"; - for ( auto const& entry : m_all ) { code << entry.second.second; } - code << "}"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } + std::string full_code{"extern \"C\" {\n"}; + for ( auto const& entry : m_all ) { full_code += entry.second.second; } + full_code += "}"; - auto tmp_dir = System::getEnv( "FUNCTOR_JIT_TMPDIR" ); - // if FUNCTOR_JIT_TMPDIR defined make sure we have a "/" at the end - // if not, default to "/tmp/" - tmp_dir = ( tmp_dir == "UNKNOWN" ) ? "/tmp/" : tmp_dir + "/"; + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to compile is:\n" << full_code << endmsg; } - // FIXME make the name a hash of all hashes and enable reuse - auto const file_prefix = tmp_dir + "FunctorJIT"; - auto const cpp_filename = file_prefix + ".cpp"; + auto const file_prefix = + m_jit_lib_dir + "FunctorJitLib_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_code ) ); auto const lib_filename = file_prefix + ".so"; - { - std::ofstream out( cpp_filename ); - out << code.str(); - out.close(); - } - - // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags - // see Phys/FunctorCore/CMakeLists.txt - // the "2>&1" is standard bash magic to redirect stderr to stdout such - // that we only need to read stdout to get the entire output - // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; - auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } - - FILE* pipe = popen( cmd.c_str(), "r" ); - if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - - // this is going to feel a bit complicated but we are very much in - // C-Land here when using `popen` so reading the output from the - // process is a bit weird. fgets reads one chunk of output at a time - // this chunk is either up to an encountered "\n" char or until the - // passed buffer is full. thus we should choose a `buffer_size` that - // fits an entire line (512 should be good enough) - const size_t buffer_size = 512; - std::array buffer{}; - std::string cmd_out; - // as long as fgets has things to read (!= nullptr) read a line into - // buffer and add it to cmd_out - while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } - - auto returnCode = pclose( pipe ); - if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } - if ( msgLevel( MSG::DEBUG ) ) { debug() << "Process stdout and stderr:\n" << cmd_out << endmsg; } + // we first try to check if the lib already exists and only create it if + // that check fails + // NOTE // we don't want this lib to be used to resolve any symbols except what // we manually get out via dlsym -> use RTLD_LOCAL (would be default // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + + // if for any reason we don't have a lib we try and create it + if ( lib != nullptr ) { + info() << "Reusing functor library: " << lib_filename << endmsg; + } else { + auto const cpp_filename = file_prefix + ".cpp"; + info() << "New functor library will be created based generated C++ file: " << cpp_filename << endmsg; + { + std::ofstream out( cpp_filename ); + out << full_code; + out.close(); + } + + // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags + // see Phys/FunctorCore/CMakeLists.txt + // the "2>&1" is standard bash magic to redirect stderr to stdout such + // that we only need to read stdout to get the entire output + // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; + auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } + + auto start_time = std::chrono::high_resolution_clock::now(); + FILE* pipe = popen( cmd.c_str(), "r" ); + if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } + auto total_time = + std::chrono::duration_cast( std::chrono::high_resolution_clock::now() - start_time ); + info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; + + // this is going to feel a bit complicated but we are very much in + // C-Land here when using `popen` so reading the output from the + // process is a bit weird. fgets reads one chunk of output at a time + // this chunk is either up to an encountered "\n" char or until the + // passed buffer is full. thus we should choose a `buffer_size` that + // fits an entire line (512 should be good enough) + const size_t buffer_size = 512; + std::array buffer{}; + std::string cmd_out; + // as long as fgets has things to read (!= nullptr) read a line into + // buffer and add it to cmd_out + while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } + + auto returnCode = pclose( pipe ); + if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } + if ( msgLevel( MSG::DEBUG ) ) { + debug() << "Return code: " << returnCode << "\nProcess stdout and stderr:\n" << cmd_out << endmsg; + } + + lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + } + if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } + // at this point we have compiled the functors so now it is time to make + // sure that we initialize each algorithm's functors for ( auto const& entry : m_all ) { - auto function_name = Functors::Cache::id( entry.first ); + auto function_name = Functors::Cache::hashToFuncName( entry.first ); auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } @@ -481,78 +193,17 @@ struct FunctorFactory : public extends { } } - return Service::start(); + return sc; } protected: - using LibHandle = void*; - using HashType = Functors::Cache::HashType; - using FactoryCache = std::map>; - using FunctorSet = std::map, std::set>>; - FunctorSet m_functors; // {headers: [{hash, type, code}, ...]} - FactoryCache m_factories; // {hash: factory function pointer, ...} + using LibHandle = void*; + using HashType = Functors::Cache::HashType; std::map>, std::string>> m_all; - - /** @brief Generate the functor cache .cpp files. - * - * In order to expose as many bugs as possible, make sure that we generate a - * different .cpp file for every set of requested headers, so every functor - * is compiled with *exactly* the headers that were requested. - */ - void writeCpp() const { - /** The LoKi meaning of this parameter was: - * - positive: write N-files - * - negative: write N-functors per file - * - zero : write one file - * currently it is not fully supported, we just use it to check that CMake - * is aware of at least as many source files as the minimum we need. - * - * @todo When split > m_functors.size() then split the functors across - * more files until we are writing to all 'split' available source - * files. - */ - std::size_t split{0}; - if ( !boost::conversion::try_lexical_convert( System::getEnv( "LOKI_GENERATE_CPPCODE" ), split ) ) { split = 0; } - if ( m_functors.size() > split ) { - throw GaudiException{"Functor factory needs to generate at least " + std::to_string( m_functors.size() ) + - " source files, but LOKI_GENERATE_CPPCODE was set to " + std::to_string( split ) + - ". Increase the SPLIT setting in the call to loki_functors_cache() to at least " + - std::to_string( m_functors.size() ), - "FunctorFactory", StatusCode::FAILURE}; - } - - /** We write one file for each unique set of headers - */ - unsigned short ifile{0}; - for ( auto const& [headers, func_set] : m_functors ) { - std::unique_ptr file; - unsigned short iwrite{0}; - for ( auto const& [hash, functor_type, brief_code] : func_set ) { - if ( !file ) { file = openFile( m_cppname, ++ifile, m_cpplines, headers ); } - - *file << '\n' << std::dec << std::noshowbase << "// FUNCTOR #" << ++iwrite << "/" << func_set.size() << '\n'; - - // write actual C++ code - ::makeCode( *file, hash, functor_type, brief_code ); - - *file << '\n'; - } - } - - // Make sure the remaining files are empty. This ensures generated code - // from previous builds (with more functors) is overwritten and does not - // interfere with the new build. - while ( ifile < split ) openFile( m_cppname, ++ifile, {}, {} ); - } - - // Flags to steer the use of JIT and the functor cache - Gaudi::Property m_use_cache{this, "UseCache", System::getEnv( "LOKI_DISABLE_CACHE" ) == "UNKNOWN"}; - Gaudi::Property m_use_jit{this, "UseJIT", System::getEnv( "LOKI_DISABLE_CLING" ) == "UNKNOWN"}; - Gaudi::Property m_makeCpp{this, "MakeCpp", System::getEnv( "LOKI_GENERATE_CPPCODE" ) != "UNKNOWN"}; - - // Properties steering the generated functor cache code - Gaudi::Property m_cppname{this, "CppFileName", make_default_cppname( this->name() )}; - Gaudi::Property> m_cpplines{this, "CppLines", {"#include \"Functors/Cache.h\""}}; + Gaudi::Property m_jit_lib_dir{ + this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), + [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( m_jit_lib_dir == "UNKNOWN" ) ? "/tmp/" : m_jit_lib_dir + "/"; }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; }; DECLARE_COMPONENT( FunctorFactory ) diff --git a/Phys/FunctorCore/tests/options/test_functors.py b/Phys/FunctorCore/tests/options/test_functors.py index a53db6ed008..0bac80c903e 100644 --- a/Phys/FunctorCore/tests/options/test_functors.py +++ b/Phys/FunctorCore/tests/options/test_functors.py @@ -213,7 +213,6 @@ app.EvtMax = 0 # these options, so if we *didn't* disable the cache then the test that cling # can handle all of these functors would be bypassed. from Configurables import FunctorFactory -FunctorFactory().UseCache = False # Simple instantiation test: are the templates working? # -- GitLab From 64f02ff33c0024b286112aef0c6a93b3eb3dc811 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Feb 2022 13:05:57 +0100 Subject: [PATCH 11/74] adapt with_output_tree to new FunctorFactory --- Phys/FunctorCore/include/Functors/with_output_tree.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Phys/FunctorCore/include/Functors/with_output_tree.h b/Phys/FunctorCore/include/Functors/with_output_tree.h index aa80f5d2bd6..d84be1272a4 100644 --- a/Phys/FunctorCore/include/Functors/with_output_tree.h +++ b/Phys/FunctorCore/include/Functors/with_output_tree.h @@ -173,17 +173,21 @@ struct with_output_tree : public Functors::detail::with_output_tree::mixin_base< using mixin_base::mixin_base; StatusCode initialize() override { - // Delegate to the base class method, this makes sure our functors are - // available. - auto sc = mixin_base::initialize(); // Open the ROOT file and create the TTree m_root_file.reset( TFile::Open( m_root_file_name.value().c_str(), "recreate" ) ); m_root_file->cd(); // m_root_tree gets managed by m_root_file, this isn't a dangling pointer m_root_tree = new TTree( m_root_tree_name.value().c_str(), "" ); + return mixin_base::initialize(); + } + + StatusCode start() override { // Set up our vectors of branch-filling helpers that go with those functors + // we can not call this in initialize() because the current implementation + // relies on calling functor->rtype() thus we need to wait unilt after the + // FunctorFactory's start() call. ( initialize(), ... ); - return sc; + return mixin_base::start(); } StatusCode finalize() override { -- GitLab From 5eac951e5313a09de3cf3d911cac851f572e816d Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Feb 2022 16:18:05 +0100 Subject: [PATCH 12/74] fix timing printout --- Phys/FunctorCore/src/Components/Factory.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 40662f0d97a..18f8c9a46f8 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -151,9 +151,6 @@ struct FunctorFactory : public extends { auto start_time = std::chrono::high_resolution_clock::now(); FILE* pipe = popen( cmd.c_str(), "r" ); if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - auto total_time = - std::chrono::duration_cast( std::chrono::high_resolution_clock::now() - start_time ); - info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; // this is going to feel a bit complicated but we are very much in // C-Land here when using `popen` so reading the output from the @@ -169,6 +166,9 @@ struct FunctorFactory : public extends { while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } auto returnCode = pclose( pipe ); + auto total_time = + std::chrono::duration_cast( std::chrono::high_resolution_clock::now() - start_time ); + info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } if ( msgLevel( MSG::DEBUG ) ) { debug() << "Return code: " << returnCode << "\nProcess stdout and stderr:\n" << cmd_out << endmsg; -- GitLab From 9cf6ed8819c5d46c3ce402656d8c8ce3710fb7f2 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 17 Feb 2022 12:06:10 +0100 Subject: [PATCH 13/74] always regenerate preprocessed header --- Phys/FunctorCore/CMakeLists.txt | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 61fc52fd398..60ac828fe2e 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -147,27 +147,10 @@ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -o ${preprocessed_header}" ) - -add_custom_command( - OUTPUT ${preprocessed_header} - COMMAND - sh tmp_preprocessor.sh - # ${CMAKE_CXX_COMPILER} - # # use a ; instead of space to avoid escaped space in command line - # "-D$>,;-D>" - # "-I$>,;-I>" - # # these just won't expand properly !!!!!!!!!!! - # ${CMAKE_CXX_FLAGS} - # ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} - # -E ${generated_header_name} - # -o ${preprocessed_header} - # DEPENDS ${generated_header_name} - # COMMAND_EXPAND_LISTS - # # VERBATIM doesn't see to help either - # VERBATIM - DEPENDS ${generated_header_name} "tmp_preprocessor.sh" -) -add_custom_target(PreProcHeader ALL DEPENDS ${preprocessed_header} ) +# I think it's safest if we always regenerate this header it's quick and thus +# avoids the possibility of a bug due to the preprocessed header being out of +# date. (trust me those bugs are annoying and hard to find...) +add_custom_target(PreProcHeader ALL COMMAND sh tmp_preprocessor.sh ) # avoid "warning: style of line directive is a GCC extension" because we # include a preprocessed header. Are there better solutions? We could first -- GitLab From 94c7b3c208a5a9453fe64070e815b61c81215f25 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 17 Feb 2022 12:07:11 +0100 Subject: [PATCH 14/74] refactor(functors): simplify FunctorDesc operator<< --- Phys/FunctorCore/src/FunctorDesc.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Phys/FunctorCore/src/FunctorDesc.cpp b/Phys/FunctorCore/src/FunctorDesc.cpp index 5fecd637862..766218e09ae 100644 --- a/Phys/FunctorCore/src/FunctorDesc.cpp +++ b/Phys/FunctorCore/src/FunctorDesc.cpp @@ -12,10 +12,11 @@ namespace std { std::ostream& operator<<( std::ostream& o, ThOr::FunctorDesc const& f ) { - return GaudiUtils::details::ostream_joiner( o << "\"(" << std::quoted( f.code, '\'' ) << ", " - << "['", - f.headers, "', '" ) - << "']" - << ", " << std::quoted( f.repr, '\'' ) << ")\""; + using GaudiUtils::operator<<; + return o << "\"(" << std::quoted( f.code, '\'' ) << ", " + << ", " << f.headers + << ", " << std::quoted( f.repr, '\'' ) + << ")\"" + ; } } // namespace std -- GitLab From 249ccdeb8b7aded8552f38f09e004f473999fcd7 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 17 Feb 2022 13:21:21 +0100 Subject: [PATCH 15/74] make Functor DataHandles work with late binding in `start()`, see LHCb!3430 --- Phys/FunctorCore/include/Functors/Utilities.h | 34 ++++++++++++- Phys/FunctorCore/src/Components/Factory.cpp | 6 ++- Phys/FunctorCore/src/FunctorDesc.cpp | 5 +- .../tests/options/functor_datahandle_test.py | 49 +++++++++++++++++++ 4 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 Phys/FunctorCore/tests/options/functor_datahandle_test.py diff --git a/Phys/FunctorCore/include/Functors/Utilities.h b/Phys/FunctorCore/include/Functors/Utilities.h index 4befcbc05ef..71284821e70 100644 --- a/Phys/FunctorCore/include/Functors/Utilities.h +++ b/Phys/FunctorCore/include/Functors/Utilities.h @@ -112,7 +112,39 @@ namespace Functors::detail { void bind_helper( Algorithm* alg, std::index_sequence ) { static_assert( std::is_base_of_v, "You must include the full declaration of the owning algorithm type!" ); - ( std::get( m_handles ).emplace( m_tes_locs[Is], alg ), ... ); + + if ( alg->msgLevel( MSG::DEBUG ) ) { + alg->info() << "Init of DataHandles of Functor (" + get_name( m_f ) + "):" << endmsg; + } + ( init_data_handle( std::get( m_handles ).emplace( m_tes_locs[Is], alg ), alg ), ... ); + } + + /** + * @brief Initialize a TES DataHandle and check that the owning algorithm + * was configured correctly and already holds our input in ExtraInputs + * + * For more info on the logic please see the detailed explanation of how + * Functors obtain their data dependencies in FIXME(Link to where?) + * + * @param handle This handle will be initialized + * @param alg Algorithm/Tool which owns this functor + */ + template + void init_data_handle( DataObjectReadHandle& handle, Algorithm* alg ) { + if ( alg->msgLevel( MSG::DEBUG ) ) { alg->info() << " + Init of DataHandle " << handle.objKey() << endmsg; } + + if ( alg->extraInputDeps().count( handle.objKey() ) == 0 ) { + throw GaudiException{"Usage of DataHandle[\"" + handle.objKey() + "\"] in Functor (" + get_name( m_f ) + + ") requires that owning algorithm " + alg->name() + + " contains this TES location inside the ExtraInputs property. This is likely a " + "Configuration/PyConf bug!", + get_name( m_f ), StatusCode::FAILURE}; + } + + // DataObjectReadHandle has a protected `init()` so we need to call it + // through it's base class. This is the same thing Gaudi::Algorithm does in + // sysInitialize() + static_cast( &handle )->init(); } /** Make a tuple of references to the result of dereferencing each diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 18f8c9a46f8..4d58d49ce1e 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -114,6 +114,10 @@ struct FunctorFactory : public extends { if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to compile is:\n" << full_code << endmsg; } + // FIXME We should extend this to somehow include the hash of the + // precompiled header, otherwise library reusing is a bit of a dangerous + // game during development as you need to remember to delete the *.so + // objects auto const file_prefix = m_jit_lib_dir + "FunctorJitLib_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_code ) ); auto const lib_filename = file_prefix + ".so"; @@ -132,7 +136,7 @@ struct FunctorFactory : public extends { info() << "Reusing functor library: " << lib_filename << endmsg; } else { auto const cpp_filename = file_prefix + ".cpp"; - info() << "New functor library will be created based generated C++ file: " << cpp_filename << endmsg; + info() << "New functor library will be created with generated C++ file: " << cpp_filename << endmsg; { std::ofstream out( cpp_filename ); out << full_code; diff --git a/Phys/FunctorCore/src/FunctorDesc.cpp b/Phys/FunctorCore/src/FunctorDesc.cpp index 766218e09ae..8f504b3fd19 100644 --- a/Phys/FunctorCore/src/FunctorDesc.cpp +++ b/Phys/FunctorCore/src/FunctorDesc.cpp @@ -14,9 +14,6 @@ namespace std { std::ostream& operator<<( std::ostream& o, ThOr::FunctorDesc const& f ) { using GaudiUtils::operator<<; return o << "\"(" << std::quoted( f.code, '\'' ) << ", " - << ", " << f.headers - << ", " << std::quoted( f.repr, '\'' ) - << ")\"" - ; + << ", " << f.headers << ", " << std::quoted( f.repr, '\'' ) << ")\""; } } // namespace std diff --git a/Phys/FunctorCore/tests/options/functor_datahandle_test.py b/Phys/FunctorCore/tests/options/functor_datahandle_test.py new file mode 100644 index 00000000000..fa7032dea5c --- /dev/null +++ b/Phys/FunctorCore/tests/options/functor_datahandle_test.py @@ -0,0 +1,49 @@ +############################################################################### +# (c) Copyright 2019 CERN for the benefit of the LHCb Collaboration # +# # +# This software is distributed under the terms of the GNU General Public # +# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # +# # +# In applying this licence, CERN does not waive the privileges and immunities # +# granted to it by virtue of its status as an Intergovernmental Organization # +# or submit itself to any jurisdiction. # +############################################################################### +from Gaudi.Configuration import ApplicationMgr, VERBOSE +from Configurables import Gaudi__Monitoring__MessageSvcSink as MessageSvcSink +from Configurables import EvtStoreSvc +from Functors import SIZE + +# algorithms are coming from PyConf because we need to use DataHandles etc. +from PyConf.Algorithms import FunctorExampleAlg as FEA, Gaudi__Examples__VectorDataProducer as VDP +from PyConf.dataflow import dataflow_config + +app = ApplicationMgr(OutputLevel=VERBOSE) +# FEA has counters so we need a sink +app.ExtSvc.append(MessageSvcSink()) +# why does the default EventDataSvc not work? good question!!! +# FIXME make an issue for that +whiteboard = EvtStoreSvc("EventDataSvc", EventSlots=1) +app.ExtSvc.append(whiteboard) + +vdp = VDP(name="VDP") +fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation) < 5, OutputLevel=VERBOSE) + +# FIXME +# make a test out of the below, which works in pyconf as long as we allow strings as DataHandles... +# fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation.location) < 5, OutputLevel=VERBOSE) +# But is correctly caught on the C++ side and throws: +# ERROR TES::Size Usage of DataHandle["/Event/VDP/OutputLocation"] in Functor (TES::Size) requires that +# owning algorithm FEA contains this TES location inside the ExtraInputs property. This is likely a Configuration/PyConf bug! + +c = dataflow_config() +c.update(fea.configuration()) +algs, _ = c.apply() +app.TopAlg = algs + +# - Event +app.EvtMax = 1 +app.EvtSel = "NONE" +app.HistogramPersistency = "NONE" + +# FIXME +# 1. Add another test like the above with the HLTScheduler to show/test that algs get scheduled correctly -- GitLab From b9c7494f6dcd83a4fffba925a900abad02e57cc2 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 17 Feb 2022 18:32:55 +0100 Subject: [PATCH 16/74] Reduce amount of printout to a reasonable level --- Phys/FunctorCore/include/Functors/Utilities.h | 8 +-- Phys/FunctorCore/src/Components/Factory.cpp | 50 ++++++------------- 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/Phys/FunctorCore/include/Functors/Utilities.h b/Phys/FunctorCore/include/Functors/Utilities.h index 71284821e70..d8178c18908 100644 --- a/Phys/FunctorCore/include/Functors/Utilities.h +++ b/Phys/FunctorCore/include/Functors/Utilities.h @@ -114,7 +114,7 @@ namespace Functors::detail { "You must include the full declaration of the owning algorithm type!" ); if ( alg->msgLevel( MSG::DEBUG ) ) { - alg->info() << "Init of DataHandles of Functor (" + get_name( m_f ) + "):" << endmsg; + alg->debug() << "Init of DataHandles of Functor: " + get_name( m_f ) << endmsg; } ( init_data_handle( std::get( m_handles ).emplace( m_tes_locs[Is], alg ), alg ), ... ); } @@ -131,11 +131,11 @@ namespace Functors::detail { */ template void init_data_handle( DataObjectReadHandle& handle, Algorithm* alg ) { - if ( alg->msgLevel( MSG::DEBUG ) ) { alg->info() << " + Init of DataHandle " << handle.objKey() << endmsg; } + if ( alg->msgLevel( MSG::DEBUG ) ) { alg->debug() << " + " << handle.objKey() << endmsg; } if ( alg->extraInputDeps().count( handle.objKey() ) == 0 ) { - throw GaudiException{"Usage of DataHandle[\"" + handle.objKey() + "\"] in Functor (" + get_name( m_f ) + - ") requires that owning algorithm " + alg->name() + + throw GaudiException{"Usage of DataHandle[\"" + handle.objKey() + "\"] in Functor: " + get_name( m_f ) + + ", requires that owning algorithm " + alg->name() + " contains this TES location inside the ExtraInputs property. This is likely a " "Configuration/PyConf bug!", get_name( m_f ), StatusCode::FAILURE}; diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 4d58d49ce1e..32dc1db2200 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -50,43 +50,33 @@ struct FunctorFactory : public extends { std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); - // This is basically Functor( PTCUT( ... ) ... ) - std::string full_code{functor_type}; - full_code.append( "( " ); - full_code.append( trimmed_code ); - full_code.append( " )" ); - + // This is basically std::make_unique>( PTCUT( ... ) ... ) + auto func_body = + "return std::make_unique<" + std::string( functor_type ) + ">(" + std::string( trimmed_code ) + ");"; // Now we can calculate the hash - const auto hash = Functors::Cache::makeHash( full_code ); + const auto hash = Functors::Cache::makeHash( func_body ); if ( msgLevel( MSG::VERBOSE ) ) { - verbose() << "Full string for hash: " << full_code << endmsg; - verbose() << "Resulting hash is " << std::hex << std::showbase << hash << endmsg; + verbose() << "Registering functor: " << desc.repr << " with hash: " << Functors::Cache::hashToStr( hash ) + << endmsg; } // See if we already JIT compiled this functor and can therefore reuse // the factory function that we got before auto iter = m_all.find( hash ); if ( iter != m_all.end() ) { - info() << "Found already registered functor: " << trimmed_code << endmsg; + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Found already registered functor: " << desc.repr << endmsg; } iter->second.first.push_back( do_copy ); - } else { - std::ostringstream code; - // Get the name for the factory function. Add a suffix to avoid it // matching the cache entries. auto function_name = Functors::Cache::hashToFuncName( hash ); // Declare the factory function - code << functor_base_t_str << " " << function_name << "() { return std::make_unique<" << functor_type << ">( " - << trimmed_code << " ); }\n"; + auto cpp_code = std::string{functor_base_t_str} + " " + function_name + "(){ " + func_body + "}\n"; - info() << "Registering functor: " << trimmed_code << endmsg; - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to JIT is:\n" << code.str() << endmsg; } - - m_all.emplace( hash, std::pair{std::vector{do_copy}, code.str()} ); + m_all.emplace( hash, std::pair{std::vector{do_copy}, cpp_code} ); } } @@ -100,26 +90,16 @@ struct FunctorFactory : public extends { auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; - if ( msgLevel( MSG::VERBOSE ) ) { - for ( auto const& entry : m_all ) { - verbose() << "Hash: " << entry.first << endmsg; - verbose() << "#users " << entry.second.first.size() << endmsg; - verbose() << entry.second.second << endmsg; - } - } - - std::string full_code{"extern \"C\" {\n"}; - for ( auto const& entry : m_all ) { full_code += entry.second.second; } - full_code += "}"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Full code to compile is:\n" << full_code << endmsg; } + std::string full_lib_code{"extern \"C\" {\n"}; + for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } + full_lib_code += "}"; // FIXME We should extend this to somehow include the hash of the // precompiled header, otherwise library reusing is a bit of a dangerous // game during development as you need to remember to delete the *.so // objects auto const file_prefix = - m_jit_lib_dir + "FunctorJitLib_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_code ) ); + m_jit_lib_dir + "FunctorJitLib_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); auto const lib_filename = file_prefix + ".so"; // we first try to check if the lib already exists and only create it if @@ -139,7 +119,7 @@ struct FunctorFactory : public extends { info() << "New functor library will be created with generated C++ file: " << cpp_filename << endmsg; { std::ofstream out( cpp_filename ); - out << full_code; + out << full_lib_code; out.close(); } @@ -175,7 +155,7 @@ struct FunctorFactory : public extends { info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } if ( msgLevel( MSG::DEBUG ) ) { - debug() << "Return code: " << returnCode << "\nProcess stdout and stderr:\n" << cmd_out << endmsg; + debug() << "Return code: " << returnCode << ", process stdout and stderr:\n" << cmd_out << endmsg; } lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); -- GitLab From 59812a5217880a14f51b0d7581fa17a5074ec3ef Mon Sep 17 00:00:00 2001 From: nnolte Date: Thu, 17 Feb 2022 23:57:17 -0500 Subject: [PATCH 17/74] modify tests to use pyconf algorithms (needed with new functor factory) --- .../tests/options/test_functors.py | 20 ++++++++++-------- .../tests/options/test_vector_functors.py | 21 +++++++++++-------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Phys/FunctorCore/tests/options/test_functors.py b/Phys/FunctorCore/tests/options/test_functors.py index 0bac80c903e..37d54ba6372 100644 --- a/Phys/FunctorCore/tests/options/test_functors.py +++ b/Phys/FunctorCore/tests/options/test_functors.py @@ -14,11 +14,10 @@ # @author Saverio Mariani ## # ============================================================================= -import Configurables +from PyConf import Algorithms from Configurables import (ApplicationMgr, LHCbApp) from Functors import * from Functors.tests.categories import DUMMY_DATA_DEP -from Functors.utils import pack_dict import Functors.math as fmath from GaudiKernel.SystemOfUnits import GeV @@ -192,13 +191,16 @@ particle_functors = [ def test_functors(alg_name_suffix, functors_to_test, SkipCut=False): - algo = getattr(Configurables, 'InstantiateFunctors__' + alg_name_suffix) - test = algo('Test' + alg_name_suffix) - test.Functions = pack_dict( - {functor.code_repr(): functor - for functor in functors_to_test}) - if not SkipCut: test.Cut = FILTER(ALL) - ApplicationMgr().TopAlg.append(test) + algo = getattr(Algorithms, 'InstantiateFunctors__' + alg_name_suffix) + test = algo( + name='Test' + alg_name_suffix, + Functions={ + functor.code_repr(): functor + for functor in functors_to_test + }, + Cut=FILTER(ALL) if not SkipCut else None) + algs, _ = test.configuration().apply() + ApplicationMgr().TopAlg.append(algs[-1]) def test_pr(prname, functors, only_unwrapped_functors=[]): diff --git a/Phys/FunctorCore/tests/options/test_vector_functors.py b/Phys/FunctorCore/tests/options/test_vector_functors.py index d8fbb6487c9..a19faf06ae3 100644 --- a/Phys/FunctorCore/tests/options/test_vector_functors.py +++ b/Phys/FunctorCore/tests/options/test_vector_functors.py @@ -10,11 +10,10 @@ ############################################################################### import os import hashlib -import Configurables +from PyConf import Algorithms import GaudiKernel.Configurable from Gaudi.Configuration import (ApplicationMgr, DEBUG) from Functors import (ALL, FILTER, POD) -from Functors.utils import pack_dict from Functors.tests.categories import functors_for_class from Configurables import EvtStoreSvc, FunctorFactory, CondDB, DDDBConf from PyConf.Algorithms import UniqueIDGeneratorAlg, ProducePrFittedForwardTracks @@ -45,16 +44,20 @@ if not inside_cache_generation: def add_algorithm(type_suffix, eval_or_init, functors, producer=None): - algo_type = getattr(Configurables, + algo_type = getattr(Algorithms, 'TestThOrFunctor' + eval_or_init + '__' + type_suffix) functors = {f.code_repr(): f for f in functors} - algo_instance = algo_type( - Functors=pack_dict(functors), PODFunctors=pack_dict(functors, POD)) - if producer is not None: algo_instance.Input = producer.location - if hasattr(algo_instance, 'Cut'): - algo_instance.Cut = pack_dict({'': FILTER(ALL)}) + kwargs = {} + if producer is not None: + kwargs["Input"] = producer if not inside_cache_generation: - algo_instance.OutputLevel = DEBUG + kwargs["OutputLevel"] = DEBUG + + algo_instance = algo_type( + Functors=functors, + PODFunctors={k: POD(v) + for k, v in functors.items()}, + **kwargs).configuration().apply()[0][-1] ApplicationMgr().TopAlg.append(algo_instance) -- GitLab From 87ae7cfb07162308413990f6f25f71316c619d16 Mon Sep 17 00:00:00 2001 From: nnolte Date: Fri, 18 Feb 2022 00:03:29 -0500 Subject: [PATCH 18/74] remove pack_dict --- Phys/FunctorCore/python/Functors/utils.py | 29 ------------------- .../tests/options/test_dump_container.py | 11 ++++--- 2 files changed, 5 insertions(+), 35 deletions(-) delete mode 100644 Phys/FunctorCore/python/Functors/utils.py diff --git a/Phys/FunctorCore/python/Functors/utils.py b/Phys/FunctorCore/python/Functors/utils.py deleted file mode 100644 index cda3c777022..00000000000 --- a/Phys/FunctorCore/python/Functors/utils.py +++ /dev/null @@ -1,29 +0,0 @@ -############################################################################### -# (c) Copyright 2020 CERN for the benefit of the LHCb Collaboration # -# # -# This software is distributed under the terms of the GNU General Public # -# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # -# # -# In applying this licence, CERN does not waive the privileges and immunities # -# granted to it by virtue of its status as an Intergovernmental Organization # -# or submit itself to any jurisdiction. # -############################################################################### -def pack_dict(input_dict, wrap=None): - """Given a string-keyed BoundFunctor-valued dictionary, pack that into a - dictionary-of-vectors property added by the with_functor_maps C++ mixin. - - This is basically a workaround for missing support for - Gaudi::Property> - - The optional `wrap` argument is an extra functor adapter that will be used - to wrap all of the functors in the dictionary. The canonical usage of this - feature is to add a wrapping `POD` functor to ensure plain, scalar data - types are returned. - """ - if input_dict is None: return {} - if wrap is not None: - input_dict = {k: wrap(v) for k, v in input_dict.items()} - return { - k: (v.code(), v.headers(), v.code_repr()) - for k, v in input_dict.items() - } diff --git a/Phys/SelAlgorithms/tests/options/test_dump_container.py b/Phys/SelAlgorithms/tests/options/test_dump_container.py index e9f7761976d..aa5ae58e286 100644 --- a/Phys/SelAlgorithms/tests/options/test_dump_container.py +++ b/Phys/SelAlgorithms/tests/options/test_dump_container.py @@ -10,7 +10,6 @@ ############################################################################### from Gaudi.Configuration import ApplicationMgr from Functors import (PT, ETA, PHI, POD) -from Functors.utils import pack_dict from Configurables import (EvtStoreSvc, ProducePrFittedForwardTracks, DumpContainer__PrFittedForwardTracks, UniqueIDGeneratorAlg) @@ -20,11 +19,11 @@ unique_id_gen = UniqueIDGeneratorAlg() produce = ProducePrFittedForwardTracks(Output='Fake/Tracks') consume = DumpContainer__PrFittedForwardTracks( Input=produce.Output, - Branches=pack_dict({ - 'PT': PT, - 'ETA': ETA, - 'PHI': PHI - }, wrap=POD), + Branches={ + 'PT': POD(PT), + 'ETA': POD(ETA), + 'PHI': POD(PHI) + }, VoidBranches={}, DumpFileName='DumpContainer.root', DumpTreeName='DumpTree') -- GitLab From 58d0633cab0a1778956d6dae38e2b604016de117 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 18 Feb 2022 12:01:53 +0100 Subject: [PATCH 19/74] make sure c++ streaming matches python Functor repr --- Phys/FunctorCore/src/FunctorDesc.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/Phys/FunctorCore/src/FunctorDesc.cpp b/Phys/FunctorCore/src/FunctorDesc.cpp index 8f504b3fd19..39966a082d3 100644 --- a/Phys/FunctorCore/src/FunctorDesc.cpp +++ b/Phys/FunctorCore/src/FunctorDesc.cpp @@ -11,9 +11,26 @@ #include "Functors/FunctorDesc.h" namespace std { + /** + * @brief operator<< specialization for a Functor Description (FuntorDesc) + * + * Output should match the python repr result, e.g for the PT Functor: + * "('::Functors::Track::TransverseMomentum{}', ['Functors/TrackLike.h'], 'PT')" + * + * @param o stream to output into + * @param f FunctorDesc to stream into o + * @return ostream& filled with the string representation of f + */ std::ostream& operator<<( std::ostream& o, ThOr::FunctorDesc const& f ) { - using GaudiUtils::operator<<; - return o << "\"(" << std::quoted( f.code, '\'' ) << ", " - << ", " << f.headers << ", " << std::quoted( f.repr, '\'' ) << ")\""; + // we can't use the default operator<< for the std::vector of headers + // because we need the single quotes around the header to match the python + // repr output of a Funtor see above + o << "\"(" << std::quoted( f.code, '\'' ) << ", ["; + if ( !f.headers.empty() ) { + // this if is to avoid having [''] instead of [] if f.headers is empty + GaudiUtils::details::ostream_joiner( o << "'", f.headers, "', '" ) << "'"; + } + o << "], " << std::quoted( f.repr, '\'' ) << ")\""; + return o; } } // namespace std -- GitLab From 8a247d2a2a6bbac753ef08524cb586c6dc1e63ea Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 18 Feb 2022 17:32:07 +0100 Subject: [PATCH 20/74] fix cmake dependencies --- Phys/FunctorCore/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 60ac828fe2e..a5a2a79188a 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -147,10 +147,13 @@ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -o ${preprocessed_header}" ) +file(MAKE_DIRECTORY ${preprocessed_header_prefix}) + # I think it's safest if we always regenerate this header it's quick and thus # avoids the possibility of a bug due to the preprocessed header being out of # date. (trust me those bugs are annoying and hard to find...) -add_custom_target(PreProcHeader ALL COMMAND sh tmp_preprocessor.sh ) +add_custom_target(PreProcHeader ALL COMMAND sh tmp_preprocessor.sh + DEPENDS ${generated_header_name} "tmp_preprocessor.sh" ${preprocessed_header_prefix} ) # avoid "warning: style of line directive is a GCC extension" because we # include a preprocessed header. Are there better solutions? We could first @@ -188,3 +191,5 @@ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ add_custom_target(MAKE_JIT_EXE ALL DEPENDS ${JIT_CMD_FILE} COMMAND chmod +x ${JIT_CMD_FILE} ) + +add_dependencies(FunctorCoreLib PreProcHeader) -- GitLab From 08556f8d228249e97adff2a4681d74dfae7b3bf3 Mon Sep 17 00:00:00 2001 From: nnolte Date: Tue, 22 Feb 2022 13:12:17 -0500 Subject: [PATCH 21/74] don't use toolchain file from rec build area in functor_jitter --- Phys/FunctorCore/CMakeLists.txt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index a5a2a79188a..14644f2d038 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -161,9 +161,14 @@ add_custom_target(PreProcHeader ALL COMMAND sh tmp_preprocessor.sh # something for later string(REPLACE " -pedantic" "" no_pedantic ${CMAKE_CXX_FLAGS}) +file(READ "${CMAKE_CXX_COMPILER}" CMAKE_CXX_COMPILER_CONTENT) +string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONTENT}) +string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) +message(STATUS "CMAKE_CXX_COMPILER_CONTENT: ${CMAKE_CXX_COMPILER_CONTENT}") + set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") -file(GENERATE - OUTPUT ${JIT_CMD_FILE} +file(GENERATE + OUTPUT ${JIT_CMD_FILE} CONTENT "#!/usr/bin/env python # Auto-generated script to create a jitter for the FunctorFactory import os @@ -179,7 +184,10 @@ my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] if len(sys.argv) != 3: raise Exception('expect two arguments! e.g. functor_jitter input.cpp output.so') -exit(sp.call('${CMAKE_CXX_COMPILER} \ +cmd = ''' +${CMAKE_CXX_COMPILER_CONTENT}''' + +exit(sp.call(cmd + ' \ -D$>, -D> \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -- GitLab From 5e9d7e6ffb50c3500750f24bda0e565f31f80d42 Mon Sep 17 00:00:00 2001 From: nnolte Date: Tue, 22 Feb 2022 15:41:41 -0500 Subject: [PATCH 22/74] remove particlecomb --- Phys/FunctorCore/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 14644f2d038..19916ee8c43 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -115,8 +115,6 @@ file(GENERATE #include \"Event/Track_v3.h\" // TrackKernel #include \"TrackKernel/TrackCompactVertex.h\" -// CombKernel -#include \"CombKernel/ParticleCombination.h\" // SelTools #include \"SelTools/MatrixNet.h\" #include \"SelTools/SigmaNet.h\" -- GitLab From a5f8dbdbc5370a1d7a299c26e4bf15d543fb6d69 Mon Sep 17 00:00:00 2001 From: nnolte Date: Wed, 23 Feb 2022 09:28:17 -0500 Subject: [PATCH 23/74] explicit standard to make clang work --- Phys/FunctorCore/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 19916ee8c43..a2ca55bf856 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -162,7 +162,6 @@ string(REPLACE " -pedantic" "" no_pedantic ${CMAKE_CXX_FLAGS}) file(READ "${CMAKE_CXX_COMPILER}" CMAKE_CXX_COMPILER_CONTENT) string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONTENT}) string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) -message(STATUS "CMAKE_CXX_COMPILER_CONTENT: ${CMAKE_CXX_COMPILER_CONTENT}") set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") file(GENERATE @@ -185,7 +184,7 @@ if len(sys.argv) != 3: cmd = ''' ${CMAKE_CXX_COMPILER_CONTENT}''' -exit(sp.call(cmd + ' \ +exit(sp.call(cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ -D$>, -D> \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -- GitLab From 306d57fea07b9909adf709036ca1fb7628f57eaf Mon Sep 17 00:00:00 2001 From: nnolte Date: Wed, 23 Feb 2022 15:57:26 -0500 Subject: [PATCH 24/74] c linkage --- Phys/FunctorCore/CMakeLists.txt | 2 +- Phys/FunctorCore/src/Components/Factory.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index a2ca55bf856..041127cbb4c 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -135,7 +135,7 @@ file(GENERATE file(GENERATE OUTPUT "tmp_preprocessor.sh" CONTENT "# auto generated -exec ${CMAKE_CXX_COMPILER} \ +exec ${CMAKE_CXX_COMPILER} -std=c++${GAUDI_CXX_STANDARD} \ -D$>, -D> \ ${CMAKE_CXX_FLAGS} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 32dc1db2200..413fa5acc9f 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -90,9 +90,12 @@ struct FunctorFactory : public extends { auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; - std::string full_lib_code{"extern \"C\" {\n"}; + std::string full_lib_code{"#pragma clang diagnostic push\n"}; + full_lib_code += "#pragma clang diagnostic ignored \"-Wreturn-type-c-linkage\"\n"; + full_lib_code += "extern \"C\" {\n"; for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; + full_lib_code += "\n#pragma clang diagnostic pop\n"; // FIXME We should extend this to somehow include the hash of the // precompiled header, otherwise library reusing is a bit of a dangerous -- GitLab From 328ceeb5f8c75d41d414843941e301a4b78389d4 Mon Sep 17 00:00:00 2001 From: nnolte Date: Thu, 24 Feb 2022 12:41:48 -0500 Subject: [PATCH 25/74] exclude /usr/include from -isystem list --- Phys/FunctorCore/CMakeLists.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 041127cbb4c..7e38ca742b5 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -126,9 +126,6 @@ file(GENERATE ") -# do we need the -pthread flag? it is in here: -#$> - # # generate temporary file because I don't want to waste more time tyring to # figure out how to freaking handle stupid whitespace in generator expressions # and lists @@ -140,7 +137,7 @@ exec ${CMAKE_CXX_COMPILER} -std=c++${GAUDI_CXX_STANDARD} \ ${CMAKE_CXX_FLAGS} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -I$>,INCLUDE,/Rec/>, -I> \ --isystem $>,EXCLUDE,/Rec/>, -isystem > \ +-isystem $>,EXCLUDE,/usr/include>,EXCLUDE,/Rec/>, -isystem > \ -E ${generated_header_name} \ -o ${preprocessed_header}" ) -- GitLab From f7a14c18149611ff0139077ebdd63a9b1bb823f1 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 14:57:33 +0100 Subject: [PATCH 26/74] add hash of precompiled header to jit library name to avoid incorrect reuse after changes in header --- Phys/FunctorCore/CMakeLists.txt | 2 ++ Phys/FunctorCore/src/Components/Factory.cpp | 27 ++++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 7e38ca742b5..2486071a520 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -87,6 +87,8 @@ set(preprocessed_header_prefix "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDED set(preprocessed_header_name "preprocessed_functorfactory_header.ii") set(preprocessed_header "${preprocessed_header_prefix}${preprocessed_header_name}") +lhcb_env(SET FUNCTORFACTORY_PREPROCESSED_HEADER ${preprocessed_header}) + # this list needs to include enough headers such that any functor expression # can compile file(GENERATE diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 413fa5acc9f..35da4ef87f6 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -80,6 +80,23 @@ struct FunctorFactory : public extends { } } + StatusCode initialize() override { + auto sc = Service::initialize(); + + // runtime environment variable which points to the preprocessed header + auto const path_to_header = System::getEnv( "FUNCTORFACTORY_PREPROCESSED_HEADER" ); + if ( path_to_header == "UNKNOWN" ) { + error() << "Could not retrieve path to preprocessed header from env var: FUNCTORFACTORY_PREPROCESSED_HEADER" + << endmsg; + return StatusCode::FAILURE; + } + + std::stringstream buffer; + buffer << std::ifstream{path_to_header}.rdbuf(); + m_header_hash = Functors::Cache::hashToStr( Functors::Cache::makeHash( buffer.str() ) ); + debug() << "FunctorFactory initialized with header hash: " << m_header_hash << endmsg; + return sc; + } StatusCode start() override { auto sc = Service::start(); if ( m_all.empty() ) { @@ -97,12 +114,8 @@ struct FunctorFactory : public extends { full_lib_code += "}"; full_lib_code += "\n#pragma clang diagnostic pop\n"; - // FIXME We should extend this to somehow include the hash of the - // precompiled header, otherwise library reusing is a bit of a dangerous - // game during development as you need to remember to delete the *.so - // objects - auto const file_prefix = - m_jit_lib_dir + "FunctorJitLib_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); + auto const file_prefix = m_jit_lib_dir + "FunctorJitLib_" + m_header_hash + "_" + + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); auto const lib_filename = file_prefix + ".so"; // we first try to check if the lib already exists and only create it if @@ -191,6 +204,8 @@ protected: this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( m_jit_lib_dir == "UNKNOWN" ) ? "/tmp/" : m_jit_lib_dir + "/"; }, Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; + + std::string m_header_hash{}; }; DECLARE_COMPONENT( FunctorFactory ) -- GitLab From 786763ebe03e2c66234a8b1110d896da2ce92521 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 15:12:02 +0100 Subject: [PATCH 27/74] fix gcc warning due to clang diagnostic pragma --- Phys/FunctorCore/src/Components/Factory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 35da4ef87f6..ffd280b70b2 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -107,12 +107,12 @@ struct FunctorFactory : public extends { auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; - std::string full_lib_code{"#pragma clang diagnostic push\n"}; + std::string full_lib_code{"#if defined(__clang__)\n"}; full_lib_code += "#pragma clang diagnostic ignored \"-Wreturn-type-c-linkage\"\n"; + full_lib_code += "#endif\n"; full_lib_code += "extern \"C\" {\n"; for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; - full_lib_code += "\n#pragma clang diagnostic pop\n"; auto const file_prefix = m_jit_lib_dir + "FunctorJitLib_" + m_header_hash + "_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); -- GitLab From 241d9d6daef994828b6009192222bd18111f833f Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 15:59:15 +0100 Subject: [PATCH 28/74] use std::system to simplify invocation of functor_jitter --- Phys/FunctorCore/src/Components/Factory.cpp | 41 +++++---------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index ffd280b70b2..73d91414071 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -139,40 +139,19 @@ struct FunctorFactory : public extends { out.close(); } - // functor_jitter is a shell script generated by cmake to invoke the correct compiler with the correct flags - // see Phys/FunctorCore/CMakeLists.txt - // the "2>&1" is standard bash magic to redirect stderr to stdout such - // that we only need to read stdout to get the entire output - // auto cmd = "functor_jitter -o " + lib_filename + " " + all_includes + " " + cpp_filename + " 2>&1"; - auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename + " 2>&1"; - - if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "command passed to popen:\n" << cmd << endmsg; } - - auto start_time = std::chrono::high_resolution_clock::now(); - FILE* pipe = popen( cmd.c_str(), "r" ); - if ( pipe == nullptr ) { throw exception( "Couldn't start command." ); } - - // this is going to feel a bit complicated but we are very much in - // C-Land here when using `popen` so reading the output from the - // process is a bit weird. fgets reads one chunk of output at a time - // this chunk is either up to an encountered "\n" char or until the - // passed buffer is full. thus we should choose a `buffer_size` that - // fits an entire line (512 should be good enough) - const size_t buffer_size = 512; - std::array buffer{}; - std::string cmd_out; - // as long as fgets has things to read (!= nullptr) read a line into - // buffer and add it to cmd_out - while ( fgets( buffer.data(), buffer_size, pipe ) != nullptr ) { cmd_out += buffer.data(); } - - auto returnCode = pclose( pipe ); + // functor_jitter is a shell script generated by cmake to invoke the + // correct compiler with the correct flags see: + // Phys/FunctorCore/CMakeLists.txt + auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename; + + if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Command that will be executed:\n" << cmd << endmsg; } + + auto start_time = std::chrono::high_resolution_clock::now(); + auto return_code = std::system( cmd.c_str() ); auto total_time = std::chrono::duration_cast( std::chrono::high_resolution_clock::now() - start_time ); info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; - if ( returnCode != 0 ) { throw exception( "Non zero return code!\n" + cmd_out ); } - if ( msgLevel( MSG::DEBUG ) ) { - debug() << "Return code: " << returnCode << ", process stdout and stderr:\n" << cmd_out << endmsg; - } + if ( return_code != 0 ) { throw exception( "Non zero return code!\n" ); } lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); } -- GitLab From 05f066ff02b78688761b380b16094ee3edae8e87 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 16:07:41 +0100 Subject: [PATCH 29/74] use custom deleter unique_ptr to make sure dlclose is called on libraries --- Phys/FunctorCore/src/Components/Factory.cpp | 30 ++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 73d91414071..530c94e19ed 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -20,6 +20,17 @@ #include #include +namespace { + void lib_closer( void* handle ) { + if ( handle != nullptr ) { dlclose( handle ); } + }; + // special type to hold the void* returned by dlopen which will take care of + // calling dlclose() on the libraries upon destruction. Most important for the + // scenario where we potentially reinitialize many times and thus maybe create + // and open many functor libraries. + using LibHandle = std::unique_ptr; +} // namespace + /** @file Factory.cpp * @brief Definitions of non-templated functor factory functions. */ @@ -125,10 +136,10 @@ struct FunctorFactory : public extends { // we manually get out via dlsym -> use RTLD_LOCAL (would be default // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) - void* lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; // if for any reason we don't have a lib we try and create it - if ( lib != nullptr ) { + if ( m_lib_handle != nullptr ) { info() << "Reusing functor library: " << lib_filename << endmsg; } else { auto const cpp_filename = file_prefix + ".cpp"; @@ -153,17 +164,18 @@ struct FunctorFactory : public extends { info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( return_code != 0 ) { throw exception( "Non zero return code!\n" ); } - lib = dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ); + m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; } - if ( lib == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } + if ( m_lib_handle == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } // at this point we have compiled the functors so now it is time to make // sure that we initialize each algorithm's functors for ( auto const& entry : m_all ) { - auto function_name = Functors::Cache::hashToFuncName( entry.first ); - auto factory_function = reinterpret_cast( dlsym( lib, function_name.c_str() ) ); + auto function_name = Functors::Cache::hashToFuncName( entry.first ); + auto factory_function = + reinterpret_cast( dlsym( m_lib_handle.get(), function_name.c_str() ) ); if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } for ( auto const& copy_func : entry.second.first ) { @@ -175,9 +187,8 @@ struct FunctorFactory : public extends { return sc; } -protected: - using LibHandle = void*; - using HashType = Functors::Cache::HashType; +private: + using HashType = Functors::Cache::HashType; std::map>, std::string>> m_all; Gaudi::Property m_jit_lib_dir{ this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), @@ -185,6 +196,7 @@ protected: Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; std::string m_header_hash{}; + LibHandle m_lib_handle{nullptr, lib_closer}; }; DECLARE_COMPONENT( FunctorFactory ) -- GitLab From 4dbdd5e6879584d3486dbb14c37c39db564661c7 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 16:51:11 +0100 Subject: [PATCH 30/74] add search for lib in LD_LIBRARY_PATH to enable easy shipping of precompiled caches. --- Phys/FunctorCore/src/Components/Factory.cpp | 34 +++++++++++++-------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 530c94e19ed..e3a48c58e01 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -125,24 +125,34 @@ struct FunctorFactory : public extends { for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; - auto const file_prefix = m_jit_lib_dir + "FunctorJitLib_" + m_header_hash + "_" + + auto const file_prefix = "FunctorJitLib_" + m_header_hash + "_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); - auto const lib_filename = file_prefix + ".so"; - - // we first try to check if the lib already exists and only create it if - // that check fails + auto const lib_filename = file_prefix + ".so"; + auto const lib_full_path = m_jit_lib_dir + lib_filename; + + // we first check in 2 ways if the lib already exists and only create it if + // both check fails. + // 1. we check via plain dlopen(libname.so) which searches the + // LD_LIBRARY_PATH. This is meant to find a cached library which was for + // example put inside Rec/InstallArea/BINARY_TAG/lib which is the + // recommened way to ship precompiled caches. + // 2. We use the full path to the lib inside the FunctorJitLibDir (e.g. + // /tmp/libname.so) to see if we have already compiled this library in a + // previous execution of the same job // NOTE // we don't want this lib to be used to resolve any symbols except what // we manually get out via dlsym -> use RTLD_LOCAL (would be default // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) - m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; - // if for any reason we don't have a lib we try and create it - if ( m_lib_handle != nullptr ) { - info() << "Reusing functor library: " << lib_filename << endmsg; + if ( m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; + m_lib_handle != nullptr ) { + info() << "Using functor library: " << lib_filename << ", which was found via LD_LIBRARY_PATH" << endmsg; + } else if ( m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; + m_lib_handle != nullptr ) { + info() << "Reusing functor library: " << lib_full_path << endmsg; } else { - auto const cpp_filename = file_prefix + ".cpp"; + auto const cpp_filename = m_jit_lib_dir + file_prefix + ".cpp"; info() << "New functor library will be created with generated C++ file: " << cpp_filename << endmsg; { std::ofstream out( cpp_filename ); @@ -153,7 +163,7 @@ struct FunctorFactory : public extends { // functor_jitter is a shell script generated by cmake to invoke the // correct compiler with the correct flags see: // Phys/FunctorCore/CMakeLists.txt - auto cmd = "functor_jitter " + cpp_filename + " " + lib_filename; + auto cmd = "functor_jitter " + cpp_filename + " " + lib_full_path; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Command that will be executed:\n" << cmd << endmsg; } @@ -164,7 +174,7 @@ struct FunctorFactory : public extends { info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( return_code != 0 ) { throw exception( "Non zero return code!\n" ); } - m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; + m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; } if ( m_lib_handle == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } -- GitLab From 668d8e7567f92f981343f296521da0cd62f25c26 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 17:41:32 +0100 Subject: [PATCH 31/74] make sure do_register is not called from a RUNNING state --- Phys/FunctorCore/src/Components/Factory.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index e3a48c58e01..3f6bec1def5 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -40,9 +40,7 @@ namespace { * This service does all the heavy lifting behind compiling functors into * type-erased Functor objects. It can do this either via on demand * invoking the same compiler the project was compiled with, or by using a - * pre-compiled functor cache. The tool is also responsible for generating the - * code that produces the precompiled functor cache. It is heavily inspired by - * Vanya Belyaev's LoKi::Hybrid::Base. + * pre-compiled functor cache. */ struct FunctorFactory : public extends { using extends::extends; @@ -52,8 +50,13 @@ struct FunctorFactory : public extends { void do_register( std::function do_copy, std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour ) override { - // FIXME if we are called from somewhere after start() we need to throw. - // register needs to happen before start() + // If we are already in RUNNING state we won't go through start() thus a + // do_register call doesn't make sense as the library won't be compiled and + // the Functor won't get resolved. See GaudiKernel/StateMachine.h for info + // on possible states and transitions. + if ( FSMState() == Gaudi::StateMachine::RUNNING ) { + throw GaudiException{"do_register needs to be called before start()", "FunctorFactory", StatusCode::FAILURE}; + } // FIXME it seems that having a quoted string in the middle of the // string adds quotes at the start/end...need Gaudi!919 @@ -108,6 +111,7 @@ struct FunctorFactory : public extends { debug() << "FunctorFactory initialized with header hash: " << m_header_hash << endmsg; return sc; } + StatusCode start() override { auto sc = Service::start(); if ( m_all.empty() ) { @@ -198,9 +202,10 @@ struct FunctorFactory : public extends { } private: - using HashType = Functors::Cache::HashType; - std::map>, std::string>> m_all; - Gaudi::Property m_jit_lib_dir{ + std::map>, std::string>> + m_all; + + Gaudi::Property m_jit_lib_dir{ this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( m_jit_lib_dir == "UNKNOWN" ) ? "/tmp/" : m_jit_lib_dir + "/"; }, Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; -- GitLab From a128dc4db9e373cb4686df3cf0375ea879605061 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 1 Mar 2022 19:07:36 +0100 Subject: [PATCH 32/74] set lang to c++ in header preprocessing to avoid clang warning --- Phys/FunctorCore/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 2486071a520..bf99731eb3f 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -134,7 +134,7 @@ file(GENERATE file(GENERATE OUTPUT "tmp_preprocessor.sh" CONTENT "# auto generated -exec ${CMAKE_CXX_COMPILER} -std=c++${GAUDI_CXX_STANDARD} \ +exec ${CMAKE_CXX_COMPILER} -x c++ -std=c++${GAUDI_CXX_STANDARD} \ -D$>, -D> \ ${CMAKE_CXX_FLAGS} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -- GitLab From 1b65336b1cf62fe1bd5ee264a12e90fd59214ad3 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Wed, 2 Mar 2022 10:25:35 +0100 Subject: [PATCH 33/74] add test for Functor DataHandle registration --- .../tests/options/functor_datahandle_test.py | 27 +++++----- .../tests/options/functor_jit_test.py | 50 ------------------- .../tests/qmtest/test_functor_datahandle.qmt | 27 ++++++++++ .../qmtest/test_functor_string_datahandle.qmt | 45 +++++++++++++++++ 4 files changed, 87 insertions(+), 62 deletions(-) delete mode 100644 Phys/FunctorCore/tests/options/functor_jit_test.py create mode 100644 Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt create mode 100644 Phys/FunctorCore/tests/qmtest/test_functor_string_datahandle.qmt diff --git a/Phys/FunctorCore/tests/options/functor_datahandle_test.py b/Phys/FunctorCore/tests/options/functor_datahandle_test.py index fa7032dea5c..99ae757ee2c 100644 --- a/Phys/FunctorCore/tests/options/functor_datahandle_test.py +++ b/Phys/FunctorCore/tests/options/functor_datahandle_test.py @@ -8,6 +8,7 @@ # granted to it by virtue of its status as an Intergovernmental Organization # # or submit itself to any jurisdiction. # ############################################################################### +import os from Gaudi.Configuration import ApplicationMgr, VERBOSE from Configurables import Gaudi__Monitoring__MessageSvcSink as MessageSvcSink from Configurables import EvtStoreSvc @@ -17,23 +18,28 @@ from Functors import SIZE from PyConf.Algorithms import FunctorExampleAlg as FEA, Gaudi__Examples__VectorDataProducer as VDP from PyConf.dataflow import dataflow_config +# user verbose so we can see the DH registration in the output app = ApplicationMgr(OutputLevel=VERBOSE) # FEA has counters so we need a sink app.ExtSvc.append(MessageSvcSink()) -# why does the default EventDataSvc not work? good question!!! -# FIXME make an issue for that +# why does the default EventDataSvc not work? good question -> Gaudi#218 whiteboard = EvtStoreSvc("EventDataSvc", EventSlots=1) app.ExtSvc.append(whiteboard) vdp = VDP(name="VDP") -fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation) < 5, OutputLevel=VERBOSE) -# FIXME -# make a test out of the below, which works in pyconf as long as we allow strings as DataHandles... -# fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation.location) < 5, OutputLevel=VERBOSE) -# But is correctly caught on the C++ side and throws: -# ERROR TES::Size Usage of DataHandle["/Event/VDP/OutputLocation"] in Functor (TES::Size) requires that -# owning algorithm FEA contains this TES location inside the ExtraInputs property. This is likely a Configuration/PyConf bug! +# this env var is set from inside the test_functor_string_datahandle.qmt file. +# For that test we pass a string instead of the datahandle into the functor, +# which works in pyconf as long as we allow strings as DataHandles... +# But this behaviour is not supported for functors and we check via this test +# that this is correctly caught on the C++ side and throws: +# ERROR TES::Size Usage of DataHandle["/Event/VDP/OutputLocation"] +# in Functor (TES::Size) requires that owning algorithm FEA contains this TES +# location inside the ExtraInputs property. This is likely a Configuration/PyConf bug! +if os.environ.get("TEST_FUNCTORS_DH_USE_STRING", False): + fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation.location) < 5) +else: + fea = FEA(name="FEA", Cut=SIZE(vdp.OutputLocation) < 5) c = dataflow_config() c.update(fea.configuration()) @@ -44,6 +50,3 @@ app.TopAlg = algs app.EvtMax = 1 app.EvtSel = "NONE" app.HistogramPersistency = "NONE" - -# FIXME -# 1. Add another test like the above with the HLTScheduler to show/test that algs get scheduled correctly diff --git a/Phys/FunctorCore/tests/options/functor_jit_test.py b/Phys/FunctorCore/tests/options/functor_jit_test.py deleted file mode 100644 index 6fe8e8020cb..00000000000 --- a/Phys/FunctorCore/tests/options/functor_jit_test.py +++ /dev/null @@ -1,50 +0,0 @@ -############################################################################### -# (c) Copyright 2021 CERN for the benefit of the LHCb Collaboration # -# # -# This software is distributed under the terms of the GNU General Public # -# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # -# # -# In applying this licence, CERN does not waive the privileges and immunities # -# granted to it by virtue of its status as an Intergovernmental Organization # -# or submit itself to any jurisdiction. # -############################################################################### - -from Configurables import (HLTControlFlowMgr, EvtStoreSvc, HiveDataBrokerSvc, - Gaudi__Examples__IntDataProducer as IDP, - FunctorExampleAlg_int) -from Functors import Ex_TimesTwo -from Gaudi.Configuration import ApplicationMgr, VERBOSE - -evtMax = 1 -threads = 1 -evtslots = 1 - -# use direct calls to execute to circumvent sysExecute and AlgExecState -HLTControlFlowMgr().EnableLegacyMode = False - -# concurrency conf -HLTControlFlowMgr().ThreadPoolSize = threads - -whiteboard = EvtStoreSvc("EventDataSvc", EventSlots=evtslots) - -i1 = IDP(name="IDP1", OutputLevel=20, OutputLocation="/Event/SomeInt", Value=5) - -f_exp1 = Ex_TimesTwo - -for i in range(10): - f_exp1 = f_exp1 + Ex_TimesTwo - -fea1 = FunctorExampleAlg_int("FEA1", Cut=f_exp1 > 9, Input0="/Event/SomeInt") - -HLTControlFlowMgr().CompositeCFNodes = [('top_level', 'NONLAZY_OR', ['FEA1'], - False)] - -app = ApplicationMgr( - OutputLevel=VERBOSE, - EvtMax=evtMax, - EvtSel='NONE', - ExtSvc=[whiteboard], - EventLoop=HLTControlFlowMgr(), - TopAlg=[i1, fea1]) - -HiveDataBrokerSvc().DataProducers = app.TopAlg diff --git a/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt b/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt new file mode 100644 index 00000000000..8c10694947f --- /dev/null +++ b/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt @@ -0,0 +1,27 @@ + + + + + gaudirun.py + + ../options/functor_datahandle_test.py + + + +countErrorLines({"FATAL":0, "WARNING":2, "ERROR":0}) + + diff --git a/Phys/FunctorCore/tests/qmtest/test_functor_string_datahandle.qmt b/Phys/FunctorCore/tests/qmtest/test_functor_string_datahandle.qmt new file mode 100644 index 00000000000..a1b32c0177b --- /dev/null +++ b/Phys/FunctorCore/tests/qmtest/test_functor_string_datahandle.qmt @@ -0,0 +1,45 @@ + + + + + gaudirun.py + + ../options/functor_datahandle_test.py + + + TEST_FUNCTORS_DH_USE_STRING=1 + + 1 + + +expected_error = 'ERROR TES::Size Usage of DataHandle["/Event/VDP/OutputLocation"] in Functor: TES::Size, requires that owning algorithm FEA contains this TES location inside the ExtraInputs property. This is likely a Configuration/PyConf bug! StatusCode=FAILURE' +if stdout.find(expected_error) == -1: + causes.append("Functor DataHandle check in C++ should have thrown!") + +countErrorLines({"FATAL":1, "WARNING":2, "ERROR":4}) + + -- GitLab From fb3d475cf4aad154a90c8bdaed42083e4967138e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 4 Mar 2022 12:05:40 +0100 Subject: [PATCH 34/74] protect multi-job jit compilation from race conditions --- Phys/FunctorCore/src/Components/Factory.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 3f6bec1def5..ce2b27c96f9 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -164,10 +165,17 @@ struct FunctorFactory : public extends { out.close(); } + // In a multi job scenario we want to avoid many jobs directly writing to + // the same file and potentially causing problems. Thus we first create a + // uniquely named library and then later rename it. The rename is atomic + // in the sense that a dlopen call will either load the already existing + // file or the newly renamed one. But it can't fail due to e.g. the file + // not having been fully overwritten or similar weird things. + auto const lib_tmp_full_path = m_jit_lib_dir + "FunctorJitLib_tmp_" + std::to_string( getpid() ); // functor_jitter is a shell script generated by cmake to invoke the // correct compiler with the correct flags see: // Phys/FunctorCore/CMakeLists.txt - auto cmd = "functor_jitter " + cpp_filename + " " + lib_full_path; + auto cmd = "functor_jitter " + cpp_filename + " " + lib_tmp_full_path; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Command that will be executed:\n" << cmd << endmsg; } @@ -178,6 +186,11 @@ struct FunctorFactory : public extends { info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( return_code != 0 ) { throw exception( "Non zero return code!\n" ); } + if ( msgLevel( MSG::VERBOSE ) ) { + verbose() << "Rename " << lib_tmp_full_path << " to " << lib_full_path << endmsg; + } + std::filesystem::rename( lib_tmp_full_path, lib_full_path ); + m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; } -- GitLab From 583916ec95fe049ac5d5e0f365f471496631942e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 4 Mar 2022 15:50:07 +0100 Subject: [PATCH 35/74] only regenerate preprocessed header when needed, requirement for functorcache --- Phys/FunctorCore/CMakeLists.txt | 77 ++++++------------- .../include/Functors/JIT_includes.h | 50 ++++++++++++ .../src/functor_jit_dummy/test_includes.cpp | 13 ++++ 3 files changed, 87 insertions(+), 53 deletions(-) create mode 100644 Phys/FunctorCore/include/Functors/JIT_includes.h create mode 100644 Phys/FunctorCore/src/functor_jit_dummy/test_includes.cpp diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index bf99731eb3f..fbe8727e4de 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -74,6 +74,17 @@ gaudi_add_executable(InstantiateFunctors TEST ) +# This target only exists to try and have a reliable way to figure out when to +# rebuild the preprocessed header +gaudi_add_executable(JIT_INCLUDES_TEST + SOURCES src/functor_jit_dummy/test_includes.cpp + LINK FunctorCoreLib + LHCb::PhysEvent + LHCb::TrackEvent + LHCb::MCEvent + Rec::ParticleCombinersLib +) + gaudi_install(PYTHON) gaudi_add_tests(QMTest) @@ -82,52 +93,11 @@ gaudi_add_tests(pytest ${CMAKE_CURRENT_SOURCE_DIR}/python) string(TOUPPER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_UPPER) -set(generated_header_name "all_includes.h") -set(preprocessed_header_prefix "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/") set(preprocessed_header_name "preprocessed_functorfactory_header.ii") -set(preprocessed_header "${preprocessed_header_prefix}${preprocessed_header_name}") +set(preprocessed_header "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/${preprocessed_header_name}") lhcb_env(SET FUNCTORFACTORY_PREPROCESSED_HEADER ${preprocessed_header}) -# this list needs to include enough headers such that any functor expression -# can compile -file(GENERATE - OUTPUT ${generated_header_name} - CONTENT "// Auto-generated header for the FunctorFactory -#include -// Functors -#include \"Functors/Adapters.h\" -#include \"Functors/Combination.h\" -#include \"Functors/Composite.h\" -#include \"Functors/Example.h\" -#include \"Functors/Filter.h\" -#include \"Functors/Function.h\" -#include \"Functors/MVA.h\" -#include \"Functors/Simulation.h\" -#include \"Functors/TES.h\" -#include \"Functors/TrackLike.h\" -// PhysEvent -#include \"Event/Particle.h\" -#include \"Event/Particle_v2.h\" -#include \"Event/Vertex.h\" -// TrackEvent -#include \"Event/PrFittedForwardTracks.h\" -#include \"Event/Track_v1.h\" -#include \"Event/Track_v2.h\" -#include \"Event/Track_v3.h\" -// TrackKernel -#include \"TrackKernel/TrackCompactVertex.h\" -// SelTools -#include \"SelTools/MatrixNet.h\" -#include \"SelTools/SigmaNet.h\" -// SelKernel -#include \"SelKernel/ParticleCombination.h\" -#include \"SelKernel/VertexRelation.h\" -// PrKernel -#include \"PrKernel/PrSelection.h\" -") - - # # generate temporary file because I don't want to waste more time tyring to # figure out how to freaking handle stupid whitespace in generator expressions # and lists @@ -140,17 +110,17 @@ ${CMAKE_CXX_FLAGS} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -I$>,INCLUDE,/Rec/>, -I> \ -isystem $>,EXCLUDE,/usr/include>,EXCLUDE,/Rec/>, -isystem > \ --E ${generated_header_name} \ +-E ${CMAKE_CURRENT_SOURCE_DIR}/include/Functors/JIT_includes.h \ -o ${preprocessed_header}" ) -file(MAKE_DIRECTORY ${preprocessed_header_prefix}) -# I think it's safest if we always regenerate this header it's quick and thus -# avoids the possibility of a bug due to the preprocessed header being out of -# date. (trust me those bugs are annoying and hard to find...) -add_custom_target(PreProcHeader ALL COMMAND sh tmp_preprocessor.sh - DEPENDS ${generated_header_name} "tmp_preprocessor.sh" ${preprocessed_header_prefix} ) +# generate the preprocessed header which depends on JIT_INCLUDE_TEST +add_custom_command(OUTPUT ${preprocessed_header} + COMMAND sh tmp_preprocessor.sh + DEPENDS ${generated_header_name} "tmp_preprocessor.sh" + JIT_INCLUDES_TEST + ) # avoid "warning: style of line directive is a GCC extension" because we # include a preprocessed header. Are there better solutions? We could first @@ -162,9 +132,8 @@ file(READ "${CMAKE_CXX_COMPILER}" CMAKE_CXX_COMPILER_CONTENT) string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONTENT}) string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) -set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") file(GENERATE - OUTPUT ${JIT_CMD_FILE} + OUTPUT "functor_jitter" CONTENT "#!/usr/bin/env python # Auto-generated script to create a jitter for the FunctorFactory import os @@ -192,8 +161,10 @@ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env)) ") -add_custom_target(MAKE_JIT_EXE ALL DEPENDS ${JIT_CMD_FILE} +set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") +add_custom_command(OUTPUT ${JIT_CMD_FILE} DEPENDS "functor_jitter" + COMMAND cp "functor_jitter" ${JIT_CMD_FILE} COMMAND chmod +x ${JIT_CMD_FILE} ) -add_dependencies(FunctorCoreLib PreProcHeader) +add_custom_target(FUNCTOR_CORE_JIT ALL DEPENDS ${preprocessed_header} ${JIT_CMD_FILE}) diff --git a/Phys/FunctorCore/include/Functors/JIT_includes.h b/Phys/FunctorCore/include/Functors/JIT_includes.h new file mode 100644 index 00000000000..0bbe245721e --- /dev/null +++ b/Phys/FunctorCore/include/Functors/JIT_includes.h @@ -0,0 +1,50 @@ +/*****************************************************************************\ +* (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ + +/* + * + * This header only exists to capture all includes which are necessary for JIT + * compilation via the FunctorFactory. + * + * Do NOT include this in any cpp files!! + * + */ +#include +// Functors +#include "Functors/Adapters.h" +#include "Functors/Combination.h" +#include "Functors/Composite.h" +#include "Functors/Example.h" +#include "Functors/Filter.h" +#include "Functors/Function.h" +#include "Functors/MVA.h" +#include "Functors/Simulation.h" +#include "Functors/TES.h" +#include "Functors/TrackLike.h" +// PhysEvent +#include "Event/Particle.h" +#include "Event/Particle_v2.h" +#include "Event/Vertex.h" +// TrackEvent +#include "Event/PrFittedForwardTracks.h" +#include "Event/Track_v1.h" +#include "Event/Track_v2.h" +#include "Event/Track_v3.h" +// TrackKernel +#include "TrackKernel/TrackCompactVertex.h" +// SelTools +#include "SelTools/MatrixNet.h" +#include "SelTools/SigmaNet.h" +// SelKernel +#include "SelKernel/ParticleCombination.h" +#include "SelKernel/VertexRelation.h" +// PrKernel +#include "PrKernel/PrSelection.h" diff --git a/Phys/FunctorCore/src/functor_jit_dummy/test_includes.cpp b/Phys/FunctorCore/src/functor_jit_dummy/test_includes.cpp new file mode 100644 index 00000000000..bb61df9677a --- /dev/null +++ b/Phys/FunctorCore/src/functor_jit_dummy/test_includes.cpp @@ -0,0 +1,13 @@ +/*****************************************************************************\ +* (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include "Functors/JIT_includes.h" + +int main() { return 0; } -- GitLab From 642374ba1f7ba37213d19f06946d2db263a98942 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 7 Mar 2022 17:49:35 +0100 Subject: [PATCH 36/74] FunctorFactory add property to disable JIT functionality --- Phys/FunctorCore/src/Components/Factory.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index ce2b27c96f9..12b3b471c3d 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -156,6 +156,14 @@ struct FunctorFactory : public extends { } else if ( m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; m_lib_handle != nullptr ) { info() << "Reusing functor library: " << lib_full_path << endmsg; + } else if ( m_disable_jit ) { + // This scenario is only here to avoid that the LoKi FunctorCache + // creation leads to unnecessary creation of this jit library + info() << "Current configuration requires new functor library but property DisableJIT is enabled! Processing of " + "events will most likely fail!" + << endmsg; + return sc; + } else { auto const cpp_filename = m_jit_lib_dir + file_prefix + ".cpp"; info() << "New functor library will be created with generated C++ file: " << cpp_filename << endmsg; @@ -218,6 +226,7 @@ private: std::map>, std::string>> m_all; + Gaudi::Property m_disable_jit{this, "DisableJIT", System::getEnv( "THOR_DISABLE_JIT" ) != "UNKNOWN"}; Gaudi::Property m_jit_lib_dir{ this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( m_jit_lib_dir == "UNKNOWN" ) ? "/tmp/" : m_jit_lib_dir + "/"; }, -- GitLab From 960d9de2d145bcce03dbdac9bba74b13508ad5a4 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 7 Mar 2022 17:51:58 +0100 Subject: [PATCH 37/74] introduce new functor cache configuration --- Phys/FunctorCache/CMakeLists.txt | 49 +++++++---------- .../FunctorCache/cmake/ThOrFunctorCache.cmake | 55 +++++++++++++++++++ .../options/DisableLoKiCacheFunctors.py | 13 ----- Phys/FunctorCore/CMakeLists.txt | 4 +- 4 files changed, 77 insertions(+), 44 deletions(-) create mode 100644 Phys/FunctorCache/cmake/ThOrFunctorCache.cmake delete mode 100644 Phys/FunctorCache/options/DisableLoKiCacheFunctors.py diff --git a/Phys/FunctorCache/CMakeLists.txt b/Phys/FunctorCache/CMakeLists.txt index 280d0811474..78269d560b7 100644 --- a/Phys/FunctorCache/CMakeLists.txt +++ b/Phys/FunctorCache/CMakeLists.txt @@ -13,13 +13,15 @@ Phys/FunctorCache ----------------- #]=======================================================================] -option(THOR_BUILD_TEST_FUNCTOR_CACHE "Build functor cache for THOR functors" OFF) +gaudi_install(CMAKE + cmake/ThOrFunctorCache.cmake +) -if(THOR_BUILD_TEST_FUNCTOR_CACHE) +option(THOR_BUILD_TEST_FUNCTOR_CACHE "Build functor cache for THOR functors" ON) - # Import the cache creation module - include(LoKiFunctorsCache) +if(THOR_BUILD_TEST_FUNCTOR_CACHE) + # need to make sure the FunctorFactory is built set(cache_deps FunctorCore) # make sure GaudiConfig2 database has been correctly completed @@ -38,7 +40,10 @@ if(THOR_BUILD_TEST_FUNCTOR_CACHE) endforeach() endforeach() - # Also make sure that FunctorFactory is available + # Also make sure that FunctorFactory is available FIXME Since the new + # FunctorFactory in Rec!2699, I'm not sure if the directory is still needed + # but the comment never said "why" it was in the first place so it's hard + # to judge. (Same comment for SelAlgorithms...) if(NOT EXISTS ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/CMakeLists.txt) message(FATAL_ERROR "Functor test cache can be build only if Phys/FunctorCore is present in the current project too") endif() @@ -49,31 +54,15 @@ if(THOR_BUILD_TEST_FUNCTOR_CACHE) list(APPEND cache_deps SelAlgorithms) endif() - # Disable LoKi-specific hacks in LoKiFunctorsCachePostActionOpts.py - # For now there is no need for a ThOr-specific alternative. - set(LOKI_FUNCTORS_CACHE_POST_ACTION_OPTS) - - loki_functors_cache(FunctorVectorTestCache - options/FlagInsideCacheGeneration.py - options/DisableLoKiCacheFunctors.py - options/SilenceErrors.py - options/SuppressLogMessages.py - ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/test_vector_functors.py - FACTORIES FunctorFactory - LINK_LIBRARIES Rec::FunctorCoreLib - DEPENDS ${cache_deps} - SPLIT 75 - ) + include(cmake/ThOrFunctorCache.cmake) - loki_functors_cache(FunctorTestCache - options/DisableLoKiCacheFunctors.py - options/SilenceErrors.py - options/SuppressLogMessages.py - ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/test_functors.py - FACTORIES FunctorFactory - LINK_LIBRARIES Rec::FunctorCoreLib - DEPENDS ${cache_deps} - SPLIT 75 - ) + thor_functor_cache(functor_datahandle_test + OPTIONS + ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/functor_datahandle_test.py + ${PROJECT_SOURCE_DIR}/Phys/FunctorCache/options/SilenceErrors.py + ${PROJECT_SOURCE_DIR}/Phys/FunctorCache/options/SuppressLogMessages.py + DEPENDS + ${cache_deps} + ) endif(THOR_BUILD_TEST_FUNCTOR_CACHE) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake new file mode 100644 index 00000000000..d92cec43b0c --- /dev/null +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -0,0 +1,55 @@ +############################################################################### +# (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration # +# # +# This software is distributed under the terms of the GNU General Public # +# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # +# # +# In applying this licence, CERN does not waive the privileges and immunities # +# granted to it by virtue of its status as an Intergovernmental Organization # +# or submit itself to any jurisdiction. # +############################################################################### + + +#[========================================================================[.rst: +.. command:: thor_functor_cache + + .. code-block:: cmake + + thor_functor_cache(cache_name + OPTIONS optionfile1 [optionfile2...] + [DEPENDS target1 target2 ....]) + + This function builds a functor cache using the passed in options files. A + target with the name "FUNCTOR_CACHE_${cache_name} is created. Optional + dependencies can be specified in the DEPENDS filed and will be added to the + created target. + +#]========================================================================] +function(thor_functor_cache cache_name) + + set(CACHE_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}) + + CMAKE_PARSE_ARGUMENTS(ARG "" "" "OPTIONS;DEPENDS" ${ARGN}) + + # validate all options files exist + foreach(arg IN LISTS ARG_OPTIONS ) + if(NOT EXISTS ${arg}) + message(FATAL_ERROR "thor_functor_cache() options file \""${arg}"\" could not be found!") + endif() + endforeach() + + message(STATUS "Generating Functorcache: " ${cache_name}) + + add_custom_command(OUTPUT ${cache_name}.stamp + DEPENDS ${ARG_OPTIONS} ${ARG_DEPENDS} + # make sure that the intall directory already exists otherwise the + # compilation job will fail! + COMMAND mkdir -p ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} + COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} + # This file is only created as dependency proxy as we don't know the output + # file name the JIT library + COMMAND touch ${cache_name}.stamp + ) + + add_custom_target(FUNCTOR_CACHE_${cache_name} ALL DEPENDS ${cache_name}.stamp) +endfunction() diff --git a/Phys/FunctorCache/options/DisableLoKiCacheFunctors.py b/Phys/FunctorCache/options/DisableLoKiCacheFunctors.py deleted file mode 100644 index 6f9a917acef..00000000000 --- a/Phys/FunctorCache/options/DisableLoKiCacheFunctors.py +++ /dev/null @@ -1,13 +0,0 @@ -############################################################################### -# (c) Copyright 2000-2018 CERN for the benefit of the LHCb Collaboration # -# # -# This software is distributed under the terms of the GNU General Public # -# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # -# # -# In applying this licence, CERN does not waive the privileges and immunities # -# granted to it by virtue of its status as an Intergovernmental Organization # -# or submit itself to any jurisdiction. # -############################################################################### -from Configurables import ApplicationMgr -ApplicationMgr().Environment['LOKI_DISABLE_CACHE'] = '1' -ApplicationMgr().Environment['LOKI_DISABLE_CLING'] = '1' diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index fbe8727e4de..09c490f763a 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -167,4 +167,6 @@ add_custom_command(OUTPUT ${JIT_CMD_FILE} DEPENDS "functor_jitter" COMMAND chmod +x ${JIT_CMD_FILE} ) -add_custom_target(FUNCTOR_CORE_JIT ALL DEPENDS ${preprocessed_header} ${JIT_CMD_FILE}) +add_custom_target(FunctorCoreJit ALL DEPENDS ${preprocessed_header} ${JIT_CMD_FILE}) +# this is only here to handle dependencies of a FunctorCache outside Rec e.g. in Moore +add_dependencies(FunctorCore FunctorCoreJit) -- GitLab From 9b6dc65b3bd16f14a41c4e611c7060b14c022a33 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 8 Mar 2022 09:58:51 +0100 Subject: [PATCH 38/74] JIT_includes add new includes after rebase and update linker command --- Phys/FunctorCore/CMakeLists.txt | 2 +- Phys/FunctorCore/include/Functors/JIT_includes.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 09c490f763a..598c095cebd 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -157,7 +157,7 @@ exit(sp.call(cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -include {0}/include/${preprocessed_header_name} \ --lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent \ +-lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent -lHltEvent \ -o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env)) ") diff --git a/Phys/FunctorCore/include/Functors/JIT_includes.h b/Phys/FunctorCore/include/Functors/JIT_includes.h index 0bbe245721e..6ec78dd0bb4 100644 --- a/Phys/FunctorCore/include/Functors/JIT_includes.h +++ b/Phys/FunctorCore/include/Functors/JIT_includes.h @@ -26,6 +26,7 @@ #include "Functors/Filter.h" #include "Functors/Function.h" #include "Functors/MVA.h" +#include "Functors/NeutralLike.h" #include "Functors/Simulation.h" #include "Functors/TES.h" #include "Functors/TrackLike.h" -- GitLab From 39f2b44b3f3a2229026f8523c63d5384d1c63529 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 8 Mar 2022 14:42:32 +0100 Subject: [PATCH 39/74] ensure a env vars are not expanded in a functor cache --- Phys/FunctorCache/cmake/ThOrFunctorCache.cmake | 5 ++++- Phys/FunctorCache/options/disable_expandvars.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 Phys/FunctorCache/options/disable_expandvars.py diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake index d92cec43b0c..351cd0a3a1e 100644 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -27,6 +27,9 @@ #]========================================================================] function(thor_functor_cache cache_name) + # Workaround for gaudi/Gaudi#117 + set(DISABLE_EXPANDVARS_OPTS ${CMAKE_CURRENT_SOURCE_DIR}/options/disable_expandvars.py) + set(CACHE_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}) CMAKE_PARSE_ARGUMENTS(ARG "" "" "OPTIONS;DEPENDS" ${ARGN}) @@ -45,7 +48,7 @@ function(thor_functor_cache cache_name) # make sure that the intall directory already exists otherwise the # compilation job will fail! COMMAND mkdir -p ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} - COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} + COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${DISABLE_EXPANDVARS_OPTS} ${ARG_OPTIONS} # This file is only created as dependency proxy as we don't know the output # file name the JIT library COMMAND touch ${cache_name}.stamp diff --git a/Phys/FunctorCache/options/disable_expandvars.py b/Phys/FunctorCache/options/disable_expandvars.py new file mode 100644 index 00000000000..1affc6e3ecb --- /dev/null +++ b/Phys/FunctorCache/options/disable_expandvars.py @@ -0,0 +1,14 @@ +############################################################################### +# (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration # +# # +# This software is distributed under the terms of the GNU General Public # +# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # +# # +# In applying this licence, CERN does not waive the privileges and immunities # +# granted to it by virtue of its status as an Intergovernmental Organization # +# or submit itself to any jurisdiction. # +############################################################################### +import GaudiKernel.Configurable + +# Workaround for gaudi/Gaudi#117 that monkey patches gaudirun.py +GaudiKernel.Configurable.expandvars = lambda x: x -- GitLab From 6e6d4573f5d9b15681837df5e8127545ebb0567e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 8 Mar 2022 14:47:28 +0100 Subject: [PATCH 40/74] dlopen with RTLD_GLOBAL --- Phys/FunctorCore/src/Components/Factory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 12b3b471c3d..e18aab37944 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -150,7 +150,7 @@ struct FunctorFactory : public extends { // but specified to make it obvious). RTLD_LAZY is to only load what we // need (the .so has a bunch of other symbols in it) - if ( m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; + if ( m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_GLOBAL | RTLD_LAZY ), lib_closer}; m_lib_handle != nullptr ) { info() << "Using functor library: " << lib_filename << ", which was found via LD_LIBRARY_PATH" << endmsg; } else if ( m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; -- GitLab From f9f6fa0542f40f41442dd15182011e6a10f5ba5b Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 8 Mar 2022 15:27:34 +0100 Subject: [PATCH 41/74] fix inclusion of disable_expandvars.py --- Phys/FunctorCache/CMakeLists.txt | 5 +++-- Phys/FunctorCache/cmake/ThOrFunctorCache.cmake | 5 +---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Phys/FunctorCache/CMakeLists.txt b/Phys/FunctorCache/CMakeLists.txt index 78269d560b7..715df54d34d 100644 --- a/Phys/FunctorCache/CMakeLists.txt +++ b/Phys/FunctorCache/CMakeLists.txt @@ -59,8 +59,9 @@ if(THOR_BUILD_TEST_FUNCTOR_CACHE) thor_functor_cache(functor_datahandle_test OPTIONS ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/functor_datahandle_test.py - ${PROJECT_SOURCE_DIR}/Phys/FunctorCache/options/SilenceErrors.py - ${PROJECT_SOURCE_DIR}/Phys/FunctorCache/options/SuppressLogMessages.py + ${CMAKE_CURRENT_SOURCE_DIR}/options/SilenceErrors.py + ${CMAKE_CURRENT_SOURCE_DIR}/options/SuppressLogMessages.py + ${CMAKE_CURRENT_SOURCE_DIR}/options/disable_expandvars.py DEPENDS ${cache_deps} ) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake index 351cd0a3a1e..d92cec43b0c 100644 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -27,9 +27,6 @@ #]========================================================================] function(thor_functor_cache cache_name) - # Workaround for gaudi/Gaudi#117 - set(DISABLE_EXPANDVARS_OPTS ${CMAKE_CURRENT_SOURCE_DIR}/options/disable_expandvars.py) - set(CACHE_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}) CMAKE_PARSE_ARGUMENTS(ARG "" "" "OPTIONS;DEPENDS" ${ARGN}) @@ -48,7 +45,7 @@ function(thor_functor_cache cache_name) # make sure that the intall directory already exists otherwise the # compilation job will fail! COMMAND mkdir -p ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} - COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${DISABLE_EXPANDVARS_OPTS} ${ARG_OPTIONS} + COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} # This file is only created as dependency proxy as we don't know the output # file name the JIT library COMMAND touch ${cache_name}.stamp -- GitLab From 41917e98a0fa515882af1de7f6f99c4f2b3d5616 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 8 Mar 2022 16:49:27 +0100 Subject: [PATCH 42/74] fix CMake warning "Argument not separated from preceding token by whitespace" --- Phys/FunctorCache/cmake/ThOrFunctorCache.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake index d92cec43b0c..f99f858fd41 100644 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -34,7 +34,7 @@ function(thor_functor_cache cache_name) # validate all options files exist foreach(arg IN LISTS ARG_OPTIONS ) if(NOT EXISTS ${arg}) - message(FATAL_ERROR "thor_functor_cache() options file \""${arg}"\" could not be found!") + message(FATAL_ERROR "thor_functor_cache() options file \"" ${arg} "\" could not be found!") endif() endforeach() -- GitLab From 9fd0a90f570f97530a24edddb0ade760a3a1c6e5 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Wed, 9 Mar 2022 11:43:44 +0100 Subject: [PATCH 43/74] disable debuginfo for JIT compiled functors unless explicitly required --- Phys/FunctorCache/cmake/ThOrFunctorCache.cmake | 3 ++- Phys/FunctorCore/CMakeLists.txt | 14 +++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake index f99f858fd41..c1df2d524da 100644 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -45,7 +45,8 @@ function(thor_functor_cache cache_name) # make sure that the intall directory already exists otherwise the # compilation job will fail! COMMAND mkdir -p ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} - COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} + # set FUNCTOR_JIT_EXTRA_ARGS to overwrite the default value of "-g0" + COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_EXTRA_ARGS="" FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} # This file is only created as dependency proxy as we don't know the output # file name the JIT library COMMAND touch ${cache_name}.stamp diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 598c095cebd..56114e3b90f 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -149,16 +149,24 @@ my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] if len(sys.argv) != 3: raise Exception('expect two arguments! e.g. functor_jitter input.cpp output.so') +# debug info is only needed for debugging or the throughput tests. Those jobs +# should set this env var otherwise if not set we explicitly force the debug +# level to zero to reduce memory and compilation time overhead of JIT by a +# factor of >2 +extra_args = os.environ.get('FUNCTOR_JIT_EXTRA_ARGS', '-g0') + cmd = ''' ${CMAKE_CXX_COMPILER_CONTENT}''' -exit(sp.call(cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ +cmd = cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ -D$>, -D> \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ --include {0}/include/${preprocessed_header_name} \ +{3} -include {0}/include/${preprocessed_header_name} \ -lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent -lHltEvent \ --o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1] ), shell=True, env=my_env)) +-o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1], extra_args) + +exit(sp.call(cmd, shell=True, env=my_env)) ") set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") -- GitLab From ade9ace8711099a7966d67b540e0e9a45f137a77 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 10 Mar 2022 13:52:30 +0100 Subject: [PATCH 44/74] reduce messages in INFO stream --- Phys/FunctorCore/src/Components/Factory.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index e18aab37944..7bd397bb325 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -115,11 +115,9 @@ struct FunctorFactory : public extends { StatusCode start() override { auto sc = Service::start(); - if ( m_all.empty() ) { - return sc; - } else { - info() << m_all.size() << " functors were registered" << endmsg; - } + if ( m_all.empty() ) { return sc; } + + if ( msgLevel( MSG::DEBUG ) ) { debug() << m_all.size() << " functors were registered" << endmsg; } auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; @@ -166,7 +164,9 @@ struct FunctorFactory : public extends { } else { auto const cpp_filename = m_jit_lib_dir + file_prefix + ".cpp"; - info() << "New functor library will be created with generated C++ file: " << cpp_filename << endmsg; + info() << "New functor library will be created." << endmsg; + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Generated C++ filename: " << cpp_filename << endmsg; } + { std::ofstream out( cpp_filename ); out << full_lib_code; -- GitLab From 420c6018c0437143e31de649a1f04c89fc7bd1a6 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 11 Mar 2022 08:40:59 +0100 Subject: [PATCH 45/74] clarify FIXME and point to related issues --- Phys/FunctorCore/include/Functors/Utilities.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Phys/FunctorCore/include/Functors/Utilities.h b/Phys/FunctorCore/include/Functors/Utilities.h index d8178c18908..72b81b8318c 100644 --- a/Phys/FunctorCore/include/Functors/Utilities.h +++ b/Phys/FunctorCore/include/Functors/Utilities.h @@ -124,7 +124,10 @@ namespace Functors::detail { * was configured correctly and already holds our input in ExtraInputs * * For more info on the logic please see the detailed explanation of how - * Functors obtain their data dependencies in FIXME(Link to where?) + * Functors obtain their data dependencies in... + * FIXME A global overview documentation of the functor framework is still + * being worked on see Moore#284, FunctorFactory specific follow up in + * Rec#304 * * @param handle This handle will be initialized * @param alg Algorithm/Tool which owns this functor -- GitLab From 8fb4d0c2633cf5a50a1eb26d7a3f0c5e1d9f903c Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Fri, 11 Mar 2022 09:14:24 +0100 Subject: [PATCH 46/74] use std::filesystem::temp_directory_path instead of hardcoding "/tmp/", thanks @rmatev! --- Phys/FunctorCore/src/Components/Factory.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 7bd397bb325..75587277343 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -227,10 +227,14 @@ private: m_all; Gaudi::Property m_disable_jit{this, "DisableJIT", System::getEnv( "THOR_DISABLE_JIT" ) != "UNKNOWN"}; - Gaudi::Property m_jit_lib_dir{ - this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), - [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( m_jit_lib_dir == "UNKNOWN" ) ? "/tmp/" : m_jit_lib_dir + "/"; }, - Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; + Gaudi::Property m_jit_lib_dir{this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), + [this]( auto& /*unused*/ ) { + m_jit_lib_dir = ( ( m_jit_lib_dir == "UNKNOWN" ) + ? std::filesystem::temp_directory_path().string() + : m_jit_lib_dir.value() ) + + "/"; + }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; std::string m_header_hash{}; LibHandle m_lib_handle{nullptr, lib_closer}; -- GitLab From 4146842ab8d0faed523855ce486cf4579c6684de Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 15:48:16 +0100 Subject: [PATCH 47/74] check if file opening failed --- Phys/FunctorCore/src/Components/Factory.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 75587277343..09f635717d7 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -165,10 +165,14 @@ struct FunctorFactory : public extends { } else { auto const cpp_filename = m_jit_lib_dir + file_prefix + ".cpp"; info() << "New functor library will be created." << endmsg; - if ( msgLevel( MSG::DEBUG ) ) { debug() << "Generated C++ filename: " << cpp_filename << endmsg; } + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Generating C++ file: " << cpp_filename << endmsg; } { - std::ofstream out( cpp_filename ); + std::ofstream out{cpp_filename}; + if ( !out.is_open() ) { + throw GaudiException{ "Failed to open file " + cpp_filename, "FunctorFactory", + StatusCode::FAILURE }; + } out << full_lib_code; out.close(); } -- GitLab From cc6787abb547a0c64bbd2b5e05d503d0f08d3e73 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 16:20:00 +0100 Subject: [PATCH 48/74] CMake refactoring to support monobuild --- .../FunctorCache/cmake/ThOrFunctorCache.cmake | 15 ++++-- Phys/FunctorCore/CMakeLists.txt | 50 ++++++++++++++----- Phys/FunctorCore/src/Components/Factory.cpp | 3 +- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake index c1df2d524da..1e19a9992df 100644 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake @@ -27,7 +27,7 @@ #]========================================================================] function(thor_functor_cache cache_name) - set(CACHE_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}) + set(CACHE_LIB_DIR "${CMAKE_CURRENT_BINARY_DIR}/${cache_name}") CMAKE_PARSE_ARGUMENTS(ARG "" "" "OPTIONS;DEPENDS" ${ARGN}) @@ -42,9 +42,9 @@ function(thor_functor_cache cache_name) add_custom_command(OUTPUT ${cache_name}.stamp DEPENDS ${ARG_OPTIONS} ${ARG_DEPENDS} - # make sure that the intall directory already exists otherwise the - # compilation job will fail! - COMMAND mkdir -p ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} + # make sure that the directory already exists otherwise the compilation job + # will fail! + COMMAND mkdir -p ${CACHE_LIB_DIR} # set FUNCTOR_JIT_EXTRA_ARGS to overwrite the default value of "-g0" COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_EXTRA_ARGS="" FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} # This file is only created as dependency proxy as we don't know the output @@ -52,5 +52,12 @@ function(thor_functor_cache cache_name) COMMAND touch ${cache_name}.stamp ) +install( + DIRECTORY "${CACHE_LIB_DIR}/" + TYPE LIB + # we don't need to install the *.cpp files + PATTERN "*.cpp" EXCLUDE + PATTERN "*.so" PERMISSIONS OWNER_EXECUTE OWNER_WRITE OWNER_READ GROUP_EXECUTE GROUP_READ WORLD_EXECUTE WORLD_READ +) add_custom_target(FUNCTOR_CACHE_${cache_name} ALL DEPENDS ${cache_name}.stamp) endfunction() diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 56114e3b90f..1afa775809d 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -94,9 +94,26 @@ gaudi_add_tests(pytest ${CMAKE_CURRENT_SOURCE_DIR}/python) string(TOUPPER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_UPPER) set(preprocessed_header_name "preprocessed_functorfactory_header.ii") -set(preprocessed_header "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/${preprocessed_header_name}") -lhcb_env(SET FUNCTORFACTORY_PREPROCESSED_HEADER ${preprocessed_header}) +# When building the FunctorCache or when using a monobuild there is no install +# directory, thus we need to support running from the build directory and from +# the InstallArea. Executables like the below functor_jitter script and +# libraries are easy because the `CMAKE_CURRENT_BINARY_DIR` and the +# InstallArea are automatically in the `PATH` and `LD_LIBRARY_PATH` env +# variables. But to find the preprocessed header we need to play a small trick: +# The lhcb_env command sets an env variable for the build environment and the +# installed project, so that's what we do first and point to the InstallArea. +# Then we issue the command again but pass the PRIVATE flag which will only set +# the variable for the build directory thus overwriting the previously set env +# var for the build directory only. +lhcb_env(SET + FUNCTORFACTORY_PREPROCESSED_HEADER + "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/${preprocessed_header_name}" +) +lhcb_env(PRIVATE SET + FUNCTORFACTORY_PREPROCESSED_HEADER + "${CMAKE_CURRENT_BINARY_DIR}/${preprocessed_header_name}" +) # # generate temporary file because I don't want to waste more time tyring to # figure out how to freaking handle stupid whitespace in generator expressions @@ -111,17 +128,19 @@ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} \ -I$>,INCLUDE,/Rec/>, -I> \ -isystem $>,EXCLUDE,/usr/include>,EXCLUDE,/Rec/>, -isystem > \ -E ${CMAKE_CURRENT_SOURCE_DIR}/include/Functors/JIT_includes.h \ --o ${preprocessed_header}" +-o ${preprocessed_header_name}" ) # generate the preprocessed header which depends on JIT_INCLUDE_TEST -add_custom_command(OUTPUT ${preprocessed_header} +add_custom_command(OUTPUT ${preprocessed_header_name} COMMAND sh tmp_preprocessor.sh DEPENDS ${generated_header_name} "tmp_preprocessor.sh" JIT_INCLUDES_TEST ) +install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${preprocessed_header_name}" TYPE INCLUDE) + # avoid "warning: style of line directive is a GCC extension" because we # include a preprocessed header. Are there better solutions? We could first # precompile the preprocessed header in initalize() and then use that pch... @@ -133,14 +152,14 @@ string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONT string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) file(GENERATE - OUTPUT "functor_jitter" + OUTPUT "functor_jitter_tmp" CONTENT "#!/usr/bin/env python # Auto-generated script to create a jitter for the FunctorFactory import os import subprocess as sp import sys -base_dir = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) +header = os.environ['FUNCTORFACTORY_PREPROCESSED_HEADER'] # we know all our libs will be on the LD_LIBRARY_PATH so just point the linker there my_env = os.environ.copy() @@ -162,19 +181,24 @@ cmd = cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ -D$>, -D> \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -{3} -include {0}/include/${preprocessed_header_name} \ +{3} -include {0} \ -lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent -lHltEvent \ --o {1} {2}'.format(base_dir, sys.argv[2], sys.argv[1], extra_args) +-o {1} {2}'.format(header, sys.argv[2], sys.argv[1], extra_args) exit(sp.call(cmd, shell=True, env=my_env)) ") -set(JIT_CMD_FILE "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/functor_jitter") -add_custom_command(OUTPUT ${JIT_CMD_FILE} DEPENDS "functor_jitter" - COMMAND cp "functor_jitter" ${JIT_CMD_FILE} - COMMAND chmod +x ${JIT_CMD_FILE} +# we don't yet have cmake 3.20 so file(GENERATE) doesn't accept permissions yet +# thus we add a proxy command that copies the generated file and makes it +# executable +add_custom_command(OUTPUT "functor_jitter" DEPENDS "functor_jitter_tmp" + COMMAND cp "functor_jitter_tmp" "functor_jitter" + COMMAND chmod a+x "functor_jitter" ) -add_custom_target(FunctorCoreJit ALL DEPENDS ${preprocessed_header} ${JIT_CMD_FILE}) + +install(PROGRAMS "${CMAKE_CURRENT_BINARY_DIR}/functor_jitter" TYPE BIN) + +add_custom_target(FunctorCoreJit ALL DEPENDS ${preprocessed_header_name} "functor_jitter") # this is only here to handle dependencies of a FunctorCache outside Rec e.g. in Moore add_dependencies(FunctorCore FunctorCoreJit) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 09f635717d7..0f5744875f8 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -170,8 +170,7 @@ struct FunctorFactory : public extends { { std::ofstream out{cpp_filename}; if ( !out.is_open() ) { - throw GaudiException{ "Failed to open file " + cpp_filename, "FunctorFactory", - StatusCode::FAILURE }; + throw GaudiException{"Failed to open file " + cpp_filename, "FunctorFactory", StatusCode::FAILURE}; } out << full_lib_code; out.close(); -- GitLab From 39d91dd999dc8de5251f2510b1e16b1bcb864a23 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 17:36:00 +0100 Subject: [PATCH 49/74] Add comment for assumption of compiler wrapper --- Phys/FunctorCore/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 1afa775809d..40522883fee 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -147,6 +147,9 @@ install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${preprocessed_header_name}" TYPE INC # something for later string(REPLACE " -pedantic" "" no_pedantic ${CMAKE_CXX_FLAGS}) +# the below logic assumes that `CMAKE_CXX_COMPILER` points to a compiler +# wrapper. These wrappers are created by the cmake logic defined in +# lcg-toolchains file(READ "${CMAKE_CXX_COMPILER}" CMAKE_CXX_COMPILER_CONTENT) string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONTENT}) string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) -- GitLab From f3260166b2cdfa97530925b18bac14fc1613762e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 18:01:03 +0100 Subject: [PATCH 50/74] functor_jitter generation, move link libraries into variable and add documentation --- Phys/FunctorCore/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 40522883fee..2c51514298a 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -154,6 +154,11 @@ file(READ "${CMAKE_CXX_COMPILER}" CMAKE_CXX_COMPILER_CONTENT) string(REPLACE " \"$@\"" "" CMAKE_CXX_COMPILER_CONTENT ${CMAKE_CXX_COMPILER_CONTENT}) string(STRIP ${CMAKE_CXX_COMPILER_CONTENT} CMAKE_CXX_COMPILER_CONTENT) +# Specify the libraries a JIT compiled functor library will link against. The +# list here is defined based upon the includes present in +# `FunctorCore/include/JIT_includes.h` +set(JIT_LINK_LIBS "-lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent -lHltEvent") + file(GENERATE OUTPUT "functor_jitter_tmp" CONTENT "#!/usr/bin/env python @@ -185,7 +190,7 @@ cmd = cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ ${no_pedantic} \ ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ {3} -include {0} \ --lFunctorCore -lParticleCombiners -lTrackEvent -lPhysEvent -lMCEvent -lRecEvent -lHltEvent \ +${JIT_LINK_LIBS} \ -o {1} {2}'.format(header, sys.argv[2], sys.argv[1], extra_args) exit(sp.call(cmd, shell=True, env=my_env)) -- GitLab From 305f7da1608947af2ef4616fddc59d094b4b99da Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 18:45:15 +0100 Subject: [PATCH 51/74] turn DisableJIT info message into a warning --- Phys/FunctorCore/src/Components/Factory.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 0f5744875f8..b747314ccfd 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -157,9 +157,9 @@ struct FunctorFactory : public extends { } else if ( m_disable_jit ) { // This scenario is only here to avoid that the LoKi FunctorCache // creation leads to unnecessary creation of this jit library - info() << "Current configuration requires new functor library but property DisableJIT is enabled! Processing of " - "events will most likely fail!" - << endmsg; + warning() << "Current configuration requires new functor library but property DisableJIT is enabled! Functors " + "will not be initialized!" + << endmsg; return sc; } else { -- GitLab From 94dbc43ed183a8468c6ba1d57ea3aa64c381cdcc Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 18:55:28 +0100 Subject: [PATCH 52/74] improve documenation, thanks to @rmatev --- Phys/FunctorCore/CMakeLists.txt | 5 ++++- Phys/FunctorCore/include/Functors/Utilities.h | 4 +++- Phys/FunctorCore/src/Components/Factory.cpp | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 2c51514298a..3bcc71251be 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -208,5 +208,8 @@ add_custom_command(OUTPUT "functor_jitter" DEPENDS "functor_jitter_tmp" install(PROGRAMS "${CMAKE_CURRENT_BINARY_DIR}/functor_jitter" TYPE BIN) add_custom_target(FunctorCoreJit ALL DEPENDS ${preprocessed_header_name} "functor_jitter") -# this is only here to handle dependencies of a FunctorCache outside Rec e.g. in Moore +# this is only here to handle dependencies of a FunctorCache outside Rec e.g. +# in Moore TODO this is technically a hack since `FunctorCoreJit` is only a +# runtime dependency but at the moment, there is no appropriate target for +# runtime dependencies to which `FunctorCoreJit` should be added. add_dependencies(FunctorCore FunctorCoreJit) diff --git a/Phys/FunctorCore/include/Functors/Utilities.h b/Phys/FunctorCore/include/Functors/Utilities.h index 72b81b8318c..3c920e1be2d 100644 --- a/Phys/FunctorCore/include/Functors/Utilities.h +++ b/Phys/FunctorCore/include/Functors/Utilities.h @@ -146,7 +146,9 @@ namespace Functors::detail { // DataObjectReadHandle has a protected `init()` so we need to call it // through it's base class. This is the same thing Gaudi::Algorithm does in - // sysInitialize() + // sysInitialize(). We do it here because this DataHandle is created inside + // start(), at which point the step of initializing the handles of an + // algorithm has already happened. static_cast( &handle )->init(); } diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index b747314ccfd..1499ab213d3 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -125,6 +125,9 @@ struct FunctorFactory : public extends { full_lib_code += "#pragma clang diagnostic ignored \"-Wreturn-type-c-linkage\"\n"; full_lib_code += "#endif\n"; full_lib_code += "extern \"C\" {\n"; + // m_all is a map indexed with the hash, which guarantees that the + // iteration order is stable and does not depend on the order of + // `register_functor` calls. for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; -- GitLab From 1691fac83f85bec70fd2b028eee320c4187b95be Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 19:08:05 +0100 Subject: [PATCH 53/74] remove exception() indirection --- Phys/FunctorCore/src/Components/Factory.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 1499ab213d3..144096d3251 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -119,8 +119,6 @@ struct FunctorFactory : public extends { if ( msgLevel( MSG::DEBUG ) ) { debug() << m_all.size() << " functors were registered" << endmsg; } - auto exception = []( auto const& name ) { return GaudiException{name, "FunctorFactory", StatusCode::FAILURE}; }; - std::string full_lib_code{"#if defined(__clang__)\n"}; full_lib_code += "#pragma clang diagnostic ignored \"-Wreturn-type-c-linkage\"\n"; full_lib_code += "#endif\n"; @@ -198,7 +196,7 @@ struct FunctorFactory : public extends { auto total_time = std::chrono::duration_cast( std::chrono::high_resolution_clock::now() - start_time ); info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; - if ( return_code != 0 ) { throw exception( "Non zero return code!\n" ); } + if ( return_code != 0 ) { throw GaudiException{"Non zero return code!", "FunctorFactory", StatusCode::FAILURE}; } if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Rename " << lib_tmp_full_path << " to " << lib_full_path << endmsg; @@ -208,7 +206,9 @@ struct FunctorFactory : public extends { m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; } - if ( m_lib_handle == nullptr ) { throw exception( std::string( "dlopen Error:\n" ) + dlerror() ); } + if ( m_lib_handle == nullptr ) { + throw GaudiException{std::string( "dlopen Error:\n" ) + dlerror(), "FunctorFactory", StatusCode::FAILURE}; + } // at this point we have compiled the functors so now it is time to make // sure that we initialize each algorithm's functors @@ -217,7 +217,9 @@ struct FunctorFactory : public extends { auto function_name = Functors::Cache::hashToFuncName( entry.first ); auto factory_function = reinterpret_cast( dlsym( m_lib_handle.get(), function_name.c_str() ) ); - if ( factory_function == nullptr ) { throw exception( std::string( "dlsym Error:\n" ) + dlerror() ); } + if ( factory_function == nullptr ) { + throw GaudiException{std::string( "dlsym Error:\n" ) + dlerror(), "FunctorFactory", StatusCode::FAILURE}; + } for ( auto const& copy_func : entry.second.first ) { auto functor = factory_function(); -- GitLab From f6967e9bb3b39eb34a443e5d752894e1f6c0ca3e Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 21:23:34 +0100 Subject: [PATCH 54/74] replace header hash with hash of header+functor_jitter --- Phys/FunctorCore/src/Components/Factory.cpp | 47 +++++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 144096d3251..979b63f614d 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -98,6 +98,44 @@ struct FunctorFactory : public extends { StatusCode initialize() override { auto sc = Service::initialize(); + // we will fill this buffer with the content of functor_jitter and the + // preprocessed header to then calcuate a hash and write that value into + // m_header_hash which will be used to define a JIT library's filename + std::stringstream buffer; + + // first we need to find the "functor_jitter" script on the runtime_path. + // That env var looks like "path1:path2:path3...." + auto const runtime_path = System::getEnv( "PATH" ); + auto begin = 0U; + auto end = runtime_path.find( ':' ); + auto found_functor_jitter = std::filesystem::path{}; + + while ( end != std::string::npos ) { + auto path = std::filesystem::path{runtime_path.substr( begin, end - begin )}; + path.append( "functor_jitter" ); + if ( std::filesystem::is_regular_file( path ) ) { + found_functor_jitter = path; + break; + } + // move new begin to just after the ":" + begin = end + 1; + end = runtime_path.find( ':', begin ); + } + // if we didn't find anything and break out of the while loop, we need to + // check the last element + if ( end == std::string::npos ) { + auto path = std::filesystem::path{runtime_path.substr( begin, end - begin )}; + path.append( "/functor_jitter" ); + if ( std::filesystem::is_regular_file( path ) ) { found_functor_jitter = path; } + } + + if ( found_functor_jitter.empty() ) { + error() << "Could not find 'functor_jitter' executable on runtime path!" << endmsg; + return StatusCode::FAILURE; + } + debug() << "Calculating hash of: " << found_functor_jitter << endmsg; + buffer << std::ifstream{found_functor_jitter}.rdbuf(); + // runtime environment variable which points to the preprocessed header auto const path_to_header = System::getEnv( "FUNCTORFACTORY_PREPROCESSED_HEADER" ); if ( path_to_header == "UNKNOWN" ) { @@ -106,10 +144,10 @@ struct FunctorFactory : public extends { return StatusCode::FAILURE; } - std::stringstream buffer; buffer << std::ifstream{path_to_header}.rdbuf(); - m_header_hash = Functors::Cache::hashToStr( Functors::Cache::makeHash( buffer.str() ) ); - debug() << "FunctorFactory initialized with header hash: " << m_header_hash << endmsg; + m_dependencies_hash = Functors::Cache::hashToStr( Functors::Cache::makeHash( buffer.str() ) ); + debug() << "Calculating hash of: " << path_to_header << endmsg; + debug() << "FunctorFactory initialized with dependencies hash: " << m_dependencies_hash << endmsg; return sc; } @@ -129,7 +167,7 @@ struct FunctorFactory : public extends { for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; - auto const file_prefix = "FunctorJitLib_" + m_header_hash + "_" + + auto const file_prefix = "FunctorJitLib_" + m_dependencies_hash + "_" + Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); auto const lib_filename = file_prefix + ".so"; auto const lib_full_path = m_jit_lib_dir + lib_filename; @@ -245,6 +283,7 @@ private: Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; std::string m_header_hash{}; + std::string m_dependencies_hash{}; LibHandle m_lib_handle{nullptr, lib_closer}; }; -- GitLab From d5036c04de5e8e3642c3f22b6f3d1e080370f95e Mon Sep 17 00:00:00 2001 From: Rosen Matev Date: Tue, 15 Mar 2022 21:37:33 +0100 Subject: [PATCH 55/74] split statement to improve readability --- Phys/FunctorCore/tests/options/test_vector_functors.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/tests/options/test_vector_functors.py b/Phys/FunctorCore/tests/options/test_vector_functors.py index a19faf06ae3..13b376b9859 100644 --- a/Phys/FunctorCore/tests/options/test_vector_functors.py +++ b/Phys/FunctorCore/tests/options/test_vector_functors.py @@ -53,11 +53,12 @@ def add_algorithm(type_suffix, eval_or_init, functors, producer=None): if not inside_cache_generation: kwargs["OutputLevel"] = DEBUG - algo_instance = algo_type( + configurable_algs, _ = algo_type( Functors=functors, PODFunctors={k: POD(v) for k, v in functors.items()}, - **kwargs).configuration().apply()[0][-1] + **kwargs).configuration().apply() + algo_instance = configurable_algs[-1] ApplicationMgr().TopAlg.append(algo_instance) -- GitLab From 2fc49964e775afe4e23fc33a4ccb05a9efd19414 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Tue, 15 Mar 2022 21:51:48 +0100 Subject: [PATCH 56/74] add check for logic in test_vector_functors --- .../tests/options/test_vector_functors.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Phys/FunctorCore/tests/options/test_vector_functors.py b/Phys/FunctorCore/tests/options/test_vector_functors.py index 13b376b9859..c6fec60d620 100644 --- a/Phys/FunctorCore/tests/options/test_vector_functors.py +++ b/Phys/FunctorCore/tests/options/test_vector_functors.py @@ -44,8 +44,9 @@ if not inside_cache_generation: def add_algorithm(type_suffix, eval_or_init, functors, producer=None): - algo_type = getattr(Algorithms, - 'TestThOrFunctor' + eval_or_init + '__' + type_suffix) + algo_type_str = 'TestThOrFunctor' + eval_or_init + '__' + type_suffix + algo_type = getattr(Algorithms, algo_type_str) + functors = {f.code_repr(): f for f in functors} kwargs = {} if producer is not None: @@ -58,7 +59,14 @@ def add_algorithm(type_suffix, eval_or_init, functors, producer=None): PODFunctors={k: POD(v) for k, v in functors.items()}, **kwargs).configuration().apply() + + # Get the TestThOrFunctor algorithm which we (knowing some PyConf + # externals) should be able to find as the last element in the list algo_instance = configurable_algs[-1] + # check that the trick from above actually returns the algorithm that we + # are looking for. + if algo_type_str != algo_instance.getType(): + raise Exception("test_vector_functors.py: our trick didn't work") ApplicationMgr().TopAlg.append(algo_instance) -- GitLab From b91d7f924d3fa23300a114635aafcece12108aee Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 21 Mar 2022 14:56:18 +0100 Subject: [PATCH 57/74] fix linker errors and cleanup --- Phys/SelTools/include/SelTools/SigmaNet.h | 30 ++++++++++++----------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/Phys/SelTools/include/SelTools/SigmaNet.h b/Phys/SelTools/include/SelTools/SigmaNet.h index b79d87935eb..260d35f35e6 100644 --- a/Phys/SelTools/include/SelTools/SigmaNet.h +++ b/Phys/SelTools/include/SelTools/SigmaNet.h @@ -12,9 +12,12 @@ #include "GaudiKernel/Environment.h" #include "GaudiKernel/GaudiException.h" #include "Kernel/STLExtensions.h" -#include "SelTools/MVAUtils.h" +#include "MVAUtils.h" +#include +#include #include +#include #include #include @@ -30,14 +33,14 @@ namespace Sel { return std::any_cast( i->second ); } - void groupsort2( LHCb::span x ) { + inline void groupsort2( LHCb::span x ) { for ( long unsigned int i = 0; i + 1 < x.size(); i += 2 ) { if ( x[i] > x[i + 1] ) std::swap( x[i], x[i + 1] ); } } - void multiply_and_add( LHCb::span w, LHCb::span b, LHCb::span x, - LHCb::span y ) { + inline void multiply_and_add( LHCb::span w, LHCb::span b, LHCb::span x, + LHCb::span y ) { assert( w.size() == y.size() * x.size() ); assert( b.size() == y.size() ); for ( long unsigned int i = 0; i < y.size(); ++i ) { @@ -45,7 +48,7 @@ namespace Sel { } } - float sigmoid( float x ) { return 1 / ( 1 + expf( -1 * x ) ); } + inline float sigmoid( float x ) { return 1 / ( 1 + expf( -1 * x ) ); } template float evaluate( LHCb::span values, LHCb::span weights, LHCb::span biases, @@ -54,7 +57,7 @@ namespace Sel { assert( values.size() == layer_sizes.front() ); - std::array storage; + std::array storage{}; auto input_for = [&]( long unsigned int layer ) { assert( layer > 0 && layer <= layer_sizes.size() ); assert( layer_sizes[layer - 1] <= N ); @@ -134,13 +137,13 @@ namespace Sel { void setupReader(); }; - auto SigmaNet::operator()( LHCb::span values ) const { + inline auto SigmaNet::operator()( LHCb::span values ) const { if ( values.size() != m_input_size ) { throw exception( "'InputSize' does not agree with provided Input!!!" ); } // Evaluate MVA return evaluate( values, m_weights, m_biases, m_layer_sizes, m_monotone_constraints, m_lambda ); } - void SigmaNet::bind( Gaudi::Algorithm* alg ) { + inline void SigmaNet::bind( Gaudi::Algorithm* alg ) { readConfig( *alg ); // Retrieve the configuration readWeightsFile( *alg ); // Read the .pkl File with the weights if ( alg->msgLevel( MSG::VERBOSE ) ) { @@ -151,7 +154,7 @@ namespace Sel { } // Retrieve the configuration - void SigmaNet::readConfig( Gaudi::Algorithm const& alg ) { + inline void SigmaNet::readConfig( Gaudi::Algorithm const& alg ) { if ( alg.msgLevel( MSG::VERBOSE ) ) { alg.verbose() << "Start reading config..." << endmsg; } @@ -247,10 +250,9 @@ namespace Sel { if ( alg.msgLevel( MSG::VERBOSE ) ) { alg.verbose() << "weightfile: " << unresolved_weightfile << endmsg; } System::resolveEnv( unresolved_weightfile, m_weightfile ).ignore(); // LEON - return; } - void SigmaNet::readWeightsFile( Gaudi::Algorithm const& alg ) { + inline void SigmaNet::readWeightsFile( Gaudi::Algorithm const& alg ) { // Check that the weightfile exists if ( !checkWeightsFile() ) { throw exception( "Couldn't open file: " + m_weightfile ); } @@ -267,10 +269,10 @@ namespace Sel { m_layer_sizes = std::vector( m_n_layers + 1, 0 ); - nlohmann::json jf = nlohmann::json::parse( fin ); + auto jf = nlohmann::json::parse( fin ); for ( auto it = jf.begin(); it != jf.end(); ++it ) { - std::string current_key = it.key(); + const std::string& current_key = it.key(); if ( current_key.find( "sigmanet.sigma" ) != std::string::npos ) { if ( jf.at( current_key ).at( 0 ) != m_lambda ) { @@ -338,7 +340,7 @@ namespace Sel { } } - bool SigmaNet::checkWeightsFile() { + inline bool SigmaNet::checkWeightsFile() { // Check existence of WeightFile: locally return std::ifstream{m_weightfile}.good(); } -- GitLab From 840bac7450ecb2960e7e8f5a6ff265e57997f688 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 21 Mar 2022 15:01:57 +0100 Subject: [PATCH 58/74] enable parallel compilation and retrieve functors via PluginSVC(prep for move to pluginSVC cache) --- Phys/FunctorCore/CMakeLists.txt | 53 +++-- Phys/FunctorCore/include/Functors/Core.h | 6 - Phys/FunctorCore/include/Functors/IFactory.h | 14 +- Phys/FunctorCore/src/Components/Factory.cpp | 225 ++++++++++++------- 4 files changed, 195 insertions(+), 103 deletions(-) diff --git a/Phys/FunctorCore/CMakeLists.txt b/Phys/FunctorCore/CMakeLists.txt index 3bcc71251be..97be5cff94d 100644 --- a/Phys/FunctorCore/CMakeLists.txt +++ b/Phys/FunctorCore/CMakeLists.txt @@ -35,7 +35,6 @@ gaudi_add_library(FunctorCoreLib Rec::DaVinciInterfacesLib Rec::PrKernel Rec::SelKernelLib - Rec::ParticleCombinersLib Rec::SelToolsLib Rec::TrackKernel ROOT::RIO @@ -166,34 +165,58 @@ file(GENERATE import os import subprocess as sp import sys +from multiprocessing import Pool header = os.environ['FUNCTORFACTORY_PREPROCESSED_HEADER'] -# we know all our libs will be on the LD_LIBRARY_PATH so just point the linker there -my_env = os.environ.copy() -my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] +if len(sys.argv) != 4: + raise Exception( + 'expect 4 arguments! e.g. functor_jitter {N_jobs} {source_directory} {output_lib_name}' + ) -if len(sys.argv) != 3: - raise Exception('expect two arguments! e.g. functor_jitter input.cpp output.so') +n_jobs = None if sys.argv[1] == '-1' else int(sys.argv[1]) +source_dir = sys.argv[2] +lib_name = sys.argv[3] +files = os.listdir(source_dir) +os.chdir(source_dir) # debug info is only needed for debugging or the throughput tests. Those jobs # should set this env var otherwise if not set we explicitly force the debug # level to zero to reduce memory and compilation time overhead of JIT by a # factor of >2 -extra_args = os.environ.get('FUNCTOR_JIT_EXTRA_ARGS', '-g0') +extra_args = os.environ.get('THOR_JIT_EXTRA_ARGS', '-g0') cmd = ''' ${CMAKE_CXX_COMPILER_CONTENT}''' -cmd = cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ --D$>, -D> \ -${no_pedantic} \ -${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC -shared \ -{3} -include {0} \ -${JIT_LINK_LIBS} \ --o {1} {2}'.format(header, sys.argv[2], sys.argv[1], extra_args) +my_pool = Pool(n_jobs) +return_codes = [] + +for file in files: + compile_cmd = cmd + ' -std=c++${GAUDI_CXX_STANDARD} \ + -D$>, -D> \ + ${no_pedantic} \ + ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}} -fPIC \ + {2} -include {0} -c {1}'.format(header, file, extra_args) + + res = my_pool.apply_async(sp.call, (compile_cmd, ), {'shell': True}) + + return_codes.append(res) + +my_pool.close() +my_pool.join() + +if not all([r.successful() for r in return_codes]): + print('Nonzero exit codes in compilation jobs') + exit(1) + +# we know all our libs will be on the LD_LIBRARY_PATH so just point the linker there +my_env = os.environ.copy() +my_env['LIBRARY_PATH'] = my_env['LD_LIBRARY_PATH'] + +link_cmd = cmd + ' -g -fPIC -shared {1} ${JIT_LINK_LIBS} -o {0} *.o'.format(lib_name, extra_args) -exit(sp.call(cmd, shell=True, env=my_env)) +exit(sp.call(link_cmd, shell=True, env=my_env)) ") # we don't yet have cmake 3.20 so file(GENERATE) doesn't accept permissions yet diff --git a/Phys/FunctorCore/include/Functors/Core.h b/Phys/FunctorCore/include/Functors/Core.h index 15603a5fd76..b5f2832e941 100644 --- a/Phys/FunctorCore/include/Functors/Core.h +++ b/Phys/FunctorCore/include/Functors/Core.h @@ -495,11 +495,5 @@ namespace Functors { if ( !m_functor ) { throw std::runtime_error( "Empty Functor return type queried" ); } return m_functor->rtype(); } - - /** This is useful for the functor cache. - * - * @todo Give a more useful description... - */ - using Factory = typename ::Gaudi::PluginService::Factory; }; } // namespace Functors diff --git a/Phys/FunctorCore/include/Functors/IFactory.h b/Phys/FunctorCore/include/Functors/IFactory.h index 424cdd9b799..5926be65755 100644 --- a/Phys/FunctorCore/include/Functors/IFactory.h +++ b/Phys/FunctorCore/include/Functors/IFactory.h @@ -58,11 +58,11 @@ namespace Functors { /** Implementation method that registers an input/output-type-agnostic std::unique_ptr * object with the factory */ - virtual void do_register( std::function do_copy, std::string_view functor_type, - ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; + virtual void do_register( std::function )> do_copy, + std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour ) = 0; public: - DeclareInterfaceID( IFactory, 1, 0 ); + DeclareInterfaceID( IFactory, 2, 0 ); /** Factory method to register a C++ functor object JIT compilation. * @@ -78,8 +78,12 @@ namespace Functors { void register_functor( Gaudi::Algorithm* owner, FType& functor, ThOr::FunctorDesc const& desc, CompilationBehaviour compile = DefaultCompilationBehaviour ) { - auto do_copy = [owner, &functor]( Functors::AnyFunctor* b ) { - auto ftype_ptr = dynamic_cast( b ); // cast AnyFunctor* -> FType* (base -> derived) + // we only allow passing unique_ptr as that ensures that it's clear that + // we take ownership of the passed in pointer which is important because we + // are going to move the guts of the passed in object into the registered + // algorithm's functor + auto do_copy = [owner, &functor]( std::unique_ptr b ) { + auto ftype_ptr = dynamic_cast( b.get() ); // cast AnyFunctor* -> FType* (base -> derived) if ( !ftype_ptr ) { // This should only happen if you have a bug (e.g. you used a diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 979b63f614d..b8b87203e76 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -49,7 +49,7 @@ struct FunctorFactory : public extends { // pointer-to-function type we use when JIT compiling using factory_function_ptr = functor_base_t ( * )(); - void do_register( std::function do_copy, std::string_view functor_type, + void do_register( std::function )> do_copy, std::string_view functor_type, ThOr::FunctorDesc const& desc, CompilationBehaviour ) override { // If we are already in RUNNING state we won't go through start() thus a // do_register call doesn't make sense as the library won't be compiled and @@ -59,15 +59,8 @@ struct FunctorFactory : public extends { throw GaudiException{"do_register needs to be called before start()", "FunctorFactory", StatusCode::FAILURE}; } - // FIXME it seems that having a quoted string in the middle of the - // string adds quotes at the start/end...need Gaudi!919 - std::size_t findex{desc.code.front() == '"'}; - std::size_t codelen{desc.code.size() - findex - ( desc.code.back() == '"' )}; - auto trimmed_code = std::string_view{desc.code}.substr( findex, codelen ); - // This is basically std::make_unique>( PTCUT( ... ) ... ) - auto func_body = - "return std::make_unique<" + std::string( functor_type ) + ">(" + std::string( trimmed_code ) + ");"; + auto func_body = "return std::make_unique<" + std::string( functor_type ) + ">(" + std::string( desc.code ) + ");"; // Now we can calculate the hash const auto hash = Functors::Cache::makeHash( func_body ); @@ -84,17 +77,85 @@ struct FunctorFactory : public extends { iter->second.first.push_back( do_copy ); } else { - // Get the name for the factory function. Add a suffix to avoid it - // matching the cache entries. + // fmt::format turns {{ -> { + std::string code_format = R"_( +::Gaudi::PluginService::DeclareFactory> +functor_{0}{{ + std::string{{"{0}"}}, + []() -> std::unique_ptr {{ + {1}; + }} +}}; +)_"; + + auto hash_str = Functors::Cache::hashToStr( hash ); auto function_name = Functors::Cache::hashToFuncName( hash ); - // Declare the factory function - auto cpp_code = std::string{functor_base_t_str} + " " + function_name + "(){ " + func_body + "}\n"; + auto cpp_code = fmt::format( code_format, hash_str, func_body ); m_all.emplace( hash, std::pair{std::vector{do_copy}, cpp_code} ); } } + /** + * @brief split functors in m_all into N cpp files and write those. + * + * Split strategy is controled by the value of m_split. + * m_split < 0 -> split such that each file contains abs(m_split) + * m_split > 0 -> split into m_split files, always creates m_split files even if fewer functors are registered. + * m_split == 0 -> treated as m_split=1 + * + * If splitting leads to uneven division, remaining functors are spread over the existing files. + * Used pattern to name files: "FUNCTORS_FunctorFactory_{:04d}.cpp"" + * + * FIXME what negative value is the sweet spot for JIT? + * + * @param dest_dir Destination directory files are written to. + */ + void write_cpp_files( std::filesystem::path const& dest_dir ) const { + auto const cpp_filename = dest_dir + "FUNCTORS_FunctorFactory_{:04d}.cpp"; + + auto split = m_split > 0 ? m_split.value() : m_all.size() / std::abs( m_split ); + split = split == 0 ? 1 : split; + + std::vector files( split ); + for ( std::size_t i{0}; i < split; ++i ) { files[i] = fmt::format( cpp_filename, i + 1 ); } + + // integer division will give us the amount of functors per file. + auto const functors_per_file = m_all.size() / split; + // the remainder tells us how many files will get 1 extra functor. + auto const remainder = m_all.size() % split; + // NOTE if we have less functors than required by split (m_all.size() < + // split) we still write all N=split files as the functor cache code in + // cmake expects these files to be created + + auto open_and_check = [disabe_jit = m_disable_jit]( std::ofstream& out, std::filesystem::path const& fname ) { + out.open( fname ); + if ( !out.is_open() ) { + throw GaudiException{"Failed to open file " + fname, "FunctorFactory", StatusCode::FAILURE}; + } + // if jit is disabled we assume this is for the functor cache so we + // include the unprocessed header + out << ( disabe_jit ? "#include \"Functors/JIT_includes.h\"\nnamespace {\n" : "namespace {\n" ); + }; + + // NOTE + // m_all is a map indexed with the hash, which guarantees that the + // iteration order is stable and does not depend on the order of + // `register_functor` calls. + auto functors_iter = m_all.begin(); + auto out = std::ofstream{}; + for ( std::size_t file_idx{0}; file_idx < files.size(); ++file_idx ) { + open_and_check( out, files[file_idx] ); + std::size_t written_functors{0}; + while ( functors_iter != m_all.end() && ++written_functors <= ( functors_per_file + ( file_idx < remainder ) ) ) { + out << functors_iter++->second.second; + } + out << "\n}"; + out.close(); + } + } + StatusCode initialize() override { auto sc = Service::initialize(); @@ -157,75 +218,68 @@ struct FunctorFactory : public extends { if ( msgLevel( MSG::DEBUG ) ) { debug() << m_all.size() << " functors were registered" << endmsg; } - std::string full_lib_code{"#if defined(__clang__)\n"}; - full_lib_code += "#pragma clang diagnostic ignored \"-Wreturn-type-c-linkage\"\n"; - full_lib_code += "#endif\n"; - full_lib_code += "extern \"C\" {\n"; + // FIXME Old FunctorFactory had this namespace but do I need it? + std::string full_lib_code{"namespace {\n"}; // m_all is a map indexed with the hash, which guarantees that the // iteration order is stable and does not depend on the order of // `register_functor` calls. for ( auto const& entry : m_all ) { full_lib_code += entry.second.second; } full_lib_code += "}"; - auto const file_prefix = "FunctorJitLib_" + m_dependencies_hash + "_" + - Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); + auto const content_hash = Functors::Cache::hashToStr( Functors::Cache::makeHash( full_lib_code ) ); + auto const file_prefix = "FunctorJitLib_" + m_dependencies_hash + "_" + content_hash; auto const lib_filename = file_prefix + ".so"; auto const lib_full_path = m_jit_lib_dir + lib_filename; + // declare the variable in outer scope to be able to cleanup at the end. + std::filesystem::path tmp_dir; - // we first check in 2 ways if the lib already exists and only create it if - // both check fails. - // 1. we check via plain dlopen(libname.so) which searches the - // LD_LIBRARY_PATH. This is meant to find a cached library which was for - // example put inside Rec/InstallArea/BINARY_TAG/lib which is the - // recommened way to ship precompiled caches. - // 2. We use the full path to the lib inside the FunctorJitLibDir (e.g. - // /tmp/libname.so) to see if we have already compiled this library in a - // previous execution of the same job - // NOTE - // we don't want this lib to be used to resolve any symbols except what - // we manually get out via dlsym -> use RTLD_LOCAL (would be default - // but specified to make it obvious). RTLD_LAZY is to only load what we - // need (the .so has a bunch of other symbols in it) - - if ( m_lib_handle = LibHandle{dlopen( lib_filename.c_str(), RTLD_GLOBAL | RTLD_LAZY ), lib_closer}; + // We first check if we have already compiled this library in a previous + // execution of the same job by looking for full path to the lib inside the + // FunctorJitLibDir (e.g. /tmp/libname.so) + if ( m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; m_lib_handle != nullptr ) { - info() << "Using functor library: " << lib_filename << ", which was found via LD_LIBRARY_PATH" << endmsg; - } else if ( m_lib_handle = LibHandle{dlopen( lib_full_path.c_str(), RTLD_LOCAL | RTLD_LAZY ), lib_closer}; - m_lib_handle != nullptr ) { info() << "Reusing functor library: " << lib_full_path << endmsg; - } else if ( m_disable_jit ) { - // This scenario is only here to avoid that the LoKi FunctorCache - // creation leads to unnecessary creation of this jit library - warning() << "Current configuration requires new functor library but property DisableJIT is enabled! Functors " - "will not be initialized!" - << endmsg; - return sc; - } else { - auto const cpp_filename = m_jit_lib_dir + file_prefix + ".cpp"; - info() << "New functor library will be created." << endmsg; - if ( msgLevel( MSG::DEBUG ) ) { debug() << "Generating C++ file: " << cpp_filename << endmsg; } - { - std::ofstream out{cpp_filename}; - if ( !out.is_open() ) { - throw GaudiException{"Failed to open file " + cpp_filename, "FunctorFactory", StatusCode::FAILURE}; - } - out << full_lib_code; - out.close(); + // In a multi job scenario we want to avoid many jobs directly writing to + // the same file and potentially causing problems. Thus, in JIT mode we + // will create all files in a subdirectory that is unique for each + // process and only at the end move the created library one directory up + // into m_jit_lib_dir. If two jobs create the same lib and try to move at + // the same time we are still save because the rename/move operation is + // atomic in the sense that a dlopen call will either load the already + // existing file or the newly renamed one. But it can't fail due to e.g. + // the file not having been fully overwritten or similar weird things. + // + // But if m_disable_jit is set we are + // running just for the purpose of creating the cpp files so we dump them + // directly in m_jit_lib_dir + tmp_dir = m_jit_lib_dir.value(); + if ( !m_disable_jit ) { + tmp_dir += file_prefix + "_" + std::to_string( getpid() ) + "/"; + std::filesystem::remove_all( tmp_dir ); + std::filesystem::create_directory( tmp_dir ); } - // In a multi job scenario we want to avoid many jobs directly writing to - // the same file and potentially causing problems. Thus we first create a - // uniquely named library and then later rename it. The rename is atomic - // in the sense that a dlopen call will either load the already existing - // file or the newly renamed one. But it can't fail due to e.g. the file - // not having been fully overwritten or similar weird things. - auto const lib_tmp_full_path = m_jit_lib_dir + "FunctorJitLib_tmp_" + std::to_string( getpid() ); + write_cpp_files( tmp_dir ); + + // This branch is used for the FunctorCache creation, where we only want + // to write the files but not JIT them. + if ( m_disable_jit ) { + warning() + << "Current configuration requires new functor library but property DisableJIT is enabled! Some functors " + "will not be initialized!" + << endmsg; + return sc; + } + + info() << "New functor library will be created." << endmsg; + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Based on generated C++ files in folder: " << tmp_dir << endmsg; } + // functor_jitter is a shell script generated by cmake to invoke the // correct compiler with the correct flags see: // Phys/FunctorCore/CMakeLists.txt - auto cmd = "functor_jitter " + cpp_filename + " " + lib_tmp_full_path; + auto cmd = "functor_jitter " + std::to_string( m_jit_n_jobs ) + " " + tmp_dir + " " + lib_filename; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Command that will be executed:\n" << cmd << endmsg; } @@ -236,6 +290,7 @@ struct FunctorFactory : public extends { info() << "Compilation of functor library took " << total_time.count() << " seconds" << endmsg; if ( return_code != 0 ) { throw GaudiException{"Non zero return code!", "FunctorFactory", StatusCode::FAILURE}; } + auto const lib_tmp_full_path = tmp_dir + lib_filename; if ( msgLevel( MSG::VERBOSE ) ) { verbose() << "Rename " << lib_tmp_full_path << " to " << lib_full_path << endmsg; } @@ -251,17 +306,13 @@ struct FunctorFactory : public extends { // at this point we have compiled the functors so now it is time to make // sure that we initialize each algorithm's functors for ( auto const& entry : m_all ) { - - auto function_name = Functors::Cache::hashToFuncName( entry.first ); - auto factory_function = - reinterpret_cast( dlsym( m_lib_handle.get(), function_name.c_str() ) ); - if ( factory_function == nullptr ) { - throw GaudiException{std::string( "dlsym Error:\n" ) + dlerror(), "FunctorFactory", StatusCode::FAILURE}; - } - for ( auto const& copy_func : entry.second.first ) { - auto functor = factory_function(); - copy_func( functor.get() ); + // registration of the functors with the PluginService happens + // automatically when the library is opened, thus we can now simply go + // via the PluginService to get our functors + auto functor = ::Gaudi::PluginService::Factory::create( + Functors::Cache::hashToStr( entry.first ) ); + copy_func( std::move( functor ) ); } } @@ -269,11 +320,13 @@ struct FunctorFactory : public extends { } private: - std::map>, std::string>> + std::map )>>, std::string>> m_all; - Gaudi::Property m_disable_jit{this, "DisableJIT", System::getEnv( "THOR_DISABLE_JIT" ) != "UNKNOWN"}; + Gaudi::Property m_disable_jit{this, "DisableJIT", System::getEnv( "THOR_DISABLE_JIT" ) != "UNKNOWN"}; Gaudi::Property m_jit_lib_dir{this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), + Gaudi::Property m_jit_lib_dir{this, "JitLibDir", System::getEnv( "THOR_JIT_LIBDIR" ), [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( ( m_jit_lib_dir == "UNKNOWN" ) ? std::filesystem::temp_directory_path().string() @@ -282,7 +335,25 @@ private: }, Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; - std::string m_header_hash{}; + // a value of -1 means we will use as all threads available + Gaudi::Property m_jit_n_jobs{ + this, "JITNJobs", -1, + [this]( auto& /*unused*/ ) { + if ( int tmp{}; boost::conversion::try_lexical_convert( System::getEnv( "THOR_JIT_N_JOBS" ), tmp ) ) { + m_jit_n_jobs = tmp; + } + }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; + Gaudi::Property m_split{ + this, "Split", -10, + [this]( auto& /*unused*/ ) { + if ( int tmp{}; boost::conversion::try_lexical_convert( System::getEnv( "LOKI_GENERATE_CPPCODE" ), tmp ) ) { + m_split = tmp; + } + }, + Gaudi::Details::Property::ImmediatelyInvokeHandler{true}}; + + // Gaudi::Property tmp{this, "tmp", std::filesystem::path{"ab/cd"}}; std::string m_dependencies_hash{}; LibHandle m_lib_handle{nullptr, lib_closer}; }; -- GitLab From 596cba511ab715d1da15d456962b693f69d130d2 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 21 Mar 2022 15:02:21 +0100 Subject: [PATCH 59/74] cleanup temporary directoy+files if not in DEBUG --- Phys/FunctorCore/src/Components/Factory.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index b8b87203e76..839656205d2 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -316,6 +316,12 @@ functor_{0}{{ } } + // if we reach this point then everything probably went well. Thus if we + // aren't running in DEBUG or more verbose, let's cleanup our temporary + // files. if tmp_dir is empty we just loaded an exisiting lib and don't + // need to cleanup. m_disable_jit also shouldn't do a cleanup but that one + // does an early retun so we never get to this line here + if ( msgLevel() > MSG::DEBUG && !tmp_dir.empty() ) { std::filesystem::remove_all( tmp_dir ); } return sc; } -- GitLab From 9118f720c052534100fa2cf0a45e291c09e728b7 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 21 Mar 2022 15:04:19 +0100 Subject: [PATCH 60/74] Drop new functor cache and move back to loki style cache Benefit of only generating the cpp files and letting cmake handle the rest is that the cache library is treated as a normal lib and we get some stuff for free: - ccache is uses like for all other cpp files - list components -> easily retrival of functors in cache via PluginSVC --- Phys/FunctorCache/CMakeLists.txt | 31 ++++----- .../FunctorCache/cmake/ThOrFunctorCache.cmake | 63 ------------------- .../options/disable_expandvars.py | 14 ----- Phys/FunctorCore/src/Components/Factory.cpp | 27 +++++++- 4 files changed, 40 insertions(+), 95 deletions(-) delete mode 100644 Phys/FunctorCache/cmake/ThOrFunctorCache.cmake delete mode 100644 Phys/FunctorCache/options/disable_expandvars.py diff --git a/Phys/FunctorCache/CMakeLists.txt b/Phys/FunctorCache/CMakeLists.txt index 715df54d34d..bb22398d2cd 100644 --- a/Phys/FunctorCache/CMakeLists.txt +++ b/Phys/FunctorCache/CMakeLists.txt @@ -13,15 +13,13 @@ Phys/FunctorCache ----------------- #]=======================================================================] -gaudi_install(CMAKE - cmake/ThOrFunctorCache.cmake -) - option(THOR_BUILD_TEST_FUNCTOR_CACHE "Build functor cache for THOR functors" ON) if(THOR_BUILD_TEST_FUNCTOR_CACHE) - # need to make sure the FunctorFactory is built + include(LoKiFunctorsCache) + + # need to make sure the FunctorFactory is built set(cache_deps FunctorCore) # make sure GaudiConfig2 database has been correctly completed @@ -54,16 +52,19 @@ if(THOR_BUILD_TEST_FUNCTOR_CACHE) list(APPEND cache_deps SelAlgorithms) endif() - include(cmake/ThOrFunctorCache.cmake) + # Disable LoKi-specific hacks in LoKiFunctorsCachePostActionOpts.py + # For now there is no need for a ThOr-specific alternative. + set(LOKI_FUNCTORS_CACHE_POST_ACTION_OPTS) - thor_functor_cache(functor_datahandle_test - OPTIONS - ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/functor_datahandle_test.py - ${CMAKE_CURRENT_SOURCE_DIR}/options/SilenceErrors.py - ${CMAKE_CURRENT_SOURCE_DIR}/options/SuppressLogMessages.py - ${CMAKE_CURRENT_SOURCE_DIR}/options/disable_expandvars.py - DEPENDS - ${cache_deps} - ) + loki_functors_cache(FunctorDatahandleTest + # options/SilenceErrors.py + # options/SuppressLogMessages.py + options/ThOr_create_cache_opts.py + ${PROJECT_SOURCE_DIR}/Phys/FunctorCore/tests/options/functor_datahandle_test.py + FACTORIES FunctorFactory + LINK_LIBRARIES Rec::FunctorCoreLib + DEPENDS ${cache_deps} + SPLIT 2 + ) endif(THOR_BUILD_TEST_FUNCTOR_CACHE) diff --git a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake b/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake deleted file mode 100644 index 1e19a9992df..00000000000 --- a/Phys/FunctorCache/cmake/ThOrFunctorCache.cmake +++ /dev/null @@ -1,63 +0,0 @@ -############################################################################### -# (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration # -# # -# This software is distributed under the terms of the GNU General Public # -# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # -# # -# In applying this licence, CERN does not waive the privileges and immunities # -# granted to it by virtue of its status as an Intergovernmental Organization # -# or submit itself to any jurisdiction. # -############################################################################### - - -#[========================================================================[.rst: -.. command:: thor_functor_cache - - .. code-block:: cmake - - thor_functor_cache(cache_name - OPTIONS optionfile1 [optionfile2...] - [DEPENDS target1 target2 ....]) - - This function builds a functor cache using the passed in options files. A - target with the name "FUNCTOR_CACHE_${cache_name} is created. Optional - dependencies can be specified in the DEPENDS filed and will be added to the - created target. - -#]========================================================================] -function(thor_functor_cache cache_name) - - set(CACHE_LIB_DIR "${CMAKE_CURRENT_BINARY_DIR}/${cache_name}") - - CMAKE_PARSE_ARGUMENTS(ARG "" "" "OPTIONS;DEPENDS" ${ARGN}) - - # validate all options files exist - foreach(arg IN LISTS ARG_OPTIONS ) - if(NOT EXISTS ${arg}) - message(FATAL_ERROR "thor_functor_cache() options file \"" ${arg} "\" could not be found!") - endif() - endforeach() - - message(STATUS "Generating Functorcache: " ${cache_name}) - - add_custom_command(OUTPUT ${cache_name}.stamp - DEPENDS ${ARG_OPTIONS} ${ARG_DEPENDS} - # make sure that the directory already exists otherwise the compilation job - # will fail! - COMMAND mkdir -p ${CACHE_LIB_DIR} - # set FUNCTOR_JIT_EXTRA_ARGS to overwrite the default value of "-g0" - COMMAND ${CMAKE_BINARY_DIR}/run env FUNCTOR_JIT_EXTRA_ARGS="" FUNCTOR_JIT_LIBDIR=${CACHE_LIB_DIR} gaudirun.py ${ARG_OPTIONS} - # This file is only created as dependency proxy as we don't know the output - # file name the JIT library - COMMAND touch ${cache_name}.stamp - ) - -install( - DIRECTORY "${CACHE_LIB_DIR}/" - TYPE LIB - # we don't need to install the *.cpp files - PATTERN "*.cpp" EXCLUDE - PATTERN "*.so" PERMISSIONS OWNER_EXECUTE OWNER_WRITE OWNER_READ GROUP_EXECUTE GROUP_READ WORLD_EXECUTE WORLD_READ -) - add_custom_target(FUNCTOR_CACHE_${cache_name} ALL DEPENDS ${cache_name}.stamp) -endfunction() diff --git a/Phys/FunctorCache/options/disable_expandvars.py b/Phys/FunctorCache/options/disable_expandvars.py deleted file mode 100644 index 1affc6e3ecb..00000000000 --- a/Phys/FunctorCache/options/disable_expandvars.py +++ /dev/null @@ -1,14 +0,0 @@ -############################################################################### -# (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration # -# # -# This software is distributed under the terms of the GNU General Public # -# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". # -# # -# In applying this licence, CERN does not waive the privileges and immunities # -# granted to it by virtue of its status as an Intergovernmental Organization # -# or submit itself to any jurisdiction. # -############################################################################### -import GaudiKernel.Configurable - -# Workaround for gaudi/Gaudi#117 that monkey patches gaudirun.py -GaudiKernel.Configurable.expandvars = lambda x: x diff --git a/Phys/FunctorCore/src/Components/Factory.cpp b/Phys/FunctorCore/src/Components/Factory.cpp index 839656205d2..bd2f72d7300 100644 --- a/Phys/FunctorCore/src/Components/Factory.cpp +++ b/Phys/FunctorCore/src/Components/Factory.cpp @@ -12,7 +12,9 @@ #include "Functors/FunctorDesc.h" #include "Functors/IFactory.h" #include "GaudiKernel/Service.h" +#include "fmt/format.h" #include +#include #include #include #include @@ -64,9 +66,28 @@ struct FunctorFactory : public extends { // Now we can calculate the hash const auto hash = Functors::Cache::makeHash( func_body ); + // Check via the PluginService if the functor is already present in a + // prebuilt functor cache, if not disabled. + + if ( !m_disable_cache ) { + auto cached_functor = + ::Gaudi::PluginService::Factory::create( Functors::Cache::hashToStr( hash ) ); + if ( cached_functor ) { + do_copy( std::move( cached_functor ) ); + if ( msgLevel( MSG::VERBOSE ) ) { + verbose() << "Functor: " << desc.repr << " with hash: " << Functors::Cache::hashToStr( hash ) + << " found in cache." << endmsg; + } + // becasue we don't JIT this functor, the FunctorFactory doesn't need to + // keep track of any info for this functor. We have already resolved it + // completely so we can return early + return; + } + } + if ( msgLevel( MSG::VERBOSE ) ) { - verbose() << "Registering functor: " << desc.repr << " with hash: " << Functors::Cache::hashToStr( hash ) - << endmsg; + verbose() << "Functor: " << desc.repr << " with hash: " << Functors::Cache::hashToStr( hash ) + << " registered for JIT." << endmsg; } // See if we already JIT compiled this functor and can therefore reuse @@ -331,7 +352,7 @@ private: m_all; Gaudi::Property m_disable_jit{this, "DisableJIT", System::getEnv( "THOR_DISABLE_JIT" ) != "UNKNOWN"}; - Gaudi::Property m_jit_lib_dir{this, "FunctorJitLibDir", System::getEnv( "FUNCTOR_JIT_LIBDIR" ), + Gaudi::Property m_disable_cache{this, "DisableCache", System::getEnv( "THOR_DISABLE_CACHE" ) != "UNKNOWN"}; Gaudi::Property m_jit_lib_dir{this, "JitLibDir", System::getEnv( "THOR_JIT_LIBDIR" ), [this]( auto& /*unused*/ ) { m_jit_lib_dir = ( ( m_jit_lib_dir == "UNKNOWN" ) -- GitLab From 773d19b97c34cac8e3df70931b44cd5cb0852700 Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Mon, 21 Mar 2022 16:07:48 +0100 Subject: [PATCH 61/74] disable jit in functor_datahandle to test usage of cache --- Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt b/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt index 8c10694947f..0ee107a4f64 100644 --- a/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt +++ b/Phys/FunctorCore/tests/qmtest/test_functor_datahandle.qmt @@ -17,6 +17,9 @@ Checks that the registration of DataHandles of Functors works correctly ../options/functor_datahandle_test.py + + THOR_DISABLE_JIT=1 +