[Core] New TSystem::CompileMacro flag, "h" #14067
Conversation
When TSystem::CompileMacro is invoked with new option "h", a hash of the macro name and content is produced and embedded as a symbol in the library. Then, even if the timestamp of the file is changed, if the hash is the same, the macro is not recompiled and the interpreter is not accessed. This fixes memory hoarding of workers of distributed systems when a. A C macro is used to accelerate some python functions b. A setup of the worker is not possible (for example only a setup per task is allowed)
dpiparo
requested review from
bellenot,
Axel-Naumann and
pcanal
as code owners
|
Starting build on |
|
Thanks Danilo! Now that you are modifying this file, maybe could you also address the two first points of this issue wrt the doxygen documentation? #10395 Thks |
|
Hi @ferdymercury , thanks for the reminder - it's a good proposal. I propose to let the tests run and add one additional commit when some additional refinement is needed! |
|
Build failed on windows10/default. Errors:
|
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
|
Build failed on ROOT-performance-centos8-multicore/soversion. |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
| @@ -46,6 +46,7 @@ allows a simple partial implementation for new OS'es. | |||
| #include "THashList.h" | |||
| #include "ThreadLocalStorage.h" | |||
|
|
|||
| #include <dlfcn.h> | |||
|
Can this new mode be the default? And we add option h for the old, non hash approach? |
| const std::string dlsym_jit_name("__ROOT_Internal_CompileMacro_lsym_jit"); | ||
| std::string dlsym_jit_code = "void *"; | ||
| dlsym_jit_code += | ||
| dlsym_jit_name + "(const char* hash_symbol_name) {return dlsym(RTLD_DEFAULT, hash_symbol_name);} "; |
There was a problem hiding this comment.
dlsym is (was?) not portable to Windows. See TROOT.cxx TCling.cxx TProtoClass.cxx and TClass.cxx (where TROOT.cxx seems to be the more recent one. (and yes we could use a factorization of those 4 instances).
| @@ -2827,6 +2829,7 @@ int TSystem::CompileMacro(const char *filename, Option_t *opt, | |||
| Bool_t withInfo = kTRUE; | |||
| Bool_t verbose = kFALSE; | |||
| Bool_t internalDebug = kFALSE; | |||
| Bool_t compileIfDifferent = kFALSE; | |||
There was a problem hiding this comment.
| Bool_t compileIfDifferent = kFALSE; | |
| Bool_t compileOnlyIfDifferent = kFALSE; |
|
|
||
| std::string hash_symbol_name = "__ROOT_Internal_CompileMacro_"; | ||
| if (compileIfDifferent) { | ||
| std::ifstream inputFile(expFileName); |
There was a problem hiding this comment.
We should check both the source and the optional corresponding header file.
| // Add to the dictionary the symbol which will be checked | ||
| // to avoid to rebuild, unload and reload the library if requested by the user | ||
| // with the "h" option | ||
| std::ofstream dictfile(dict, std::ios::app); |
There was a problem hiding this comment.
On windows, this means reopening a file that was just written, it may induced race condition with the file system locks (or maybe not). An alternative is to produce yet another file and have it included in the dictionary source file by rootcling by using, in the LinkDef.h the #pragma extra_include ... line.
| ROOT_EXPECT_INFO(gSystem->CompileMacro(srcFileName, "h"), "ACLiC", | ||
| "The macro has been already built and loaded according to its checksum."); |
There was a problem hiding this comment.
We should also test that it is NOT being rebuild upon the timestamp being updated and that it IS being rebuild upon the source (or header files) being changed.
|
|
||
| std::ofstream srcFile; | ||
| srcFile.open(srcFileName); | ||
| srcFile << "int testmacro(){return 42;}"; |
There was a problem hiding this comment.
| srcFile << "int testmacro(){return 42;}"; | |
| srcFile << "int testmacro(){return 42;}\n"; |
This Pull request:
Adds a new flag to CompileMacro, "h".
Changes or fixes:
This new functionality allows not to rebuild the macro even if the file is newer than the library based on a hash built for the macro and its name. This helps distributed execution, providing a clean way to augment python analyses with accelerated functions in C++ and compiled when the worker cannot be setup but just individual tasks.
Checklist: