[cling] Reuse existing weak symbols, also from JIT: #12018

Axel-Naumann · 2023-01-12T12:55:09Z

With the upgrade to llvm-13, the JIT lost the ability to re-use existing weak symbols that the JIT had already emitted, instead only looking at dlsym. This causes a significant increase in JITted symbols, and thus a significant slow-down of cling / its JIT.

This restores the old behavior, with an identical set of symbols that jet jitted.

This Pull request:

Changes or fixes:

Checklist:

tested changes locally
updated the docs (if necessary)

This PR fixes #

phsft-bot · 2023-01-12T12:55:17Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T13:29:56Z

Build failed on ROOT-debian10-i386/soversion.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-01-12T13:36:33Z

Build failed on mac12/noimt.
Running on macphsft18.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See console output.

Failing tests:

And 401 more

phsft-bot · 2023-01-12T13:38:57Z

Build failed on ROOT-performance-centos8-multicore/cxx17.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T13:15:32.725Z] 841/2422 Test Fix for ROOT-8732 #517: tutorial-dataframe-df101_h1Analysis ...............................................................***Failed Error regular expression found in output. Regex=[: error:] 24.31 sec

Failing tests:

And 117 more

phsft-bot · 2023-01-12T14:00:00Z

Build failed on ROOT-ubuntu18.04/nortcxxmod.
Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 11 more

phsft-bot · 2023-01-12T14:39:51Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 22 more

phsft-bot · 2023-01-12T15:21:00Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T15:24:40Z

Build failed on ROOT-ubuntu18.04/nortcxxmod.
Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:24:25.554Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T15:24:25.810Z] /mnt/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: ‘jit’ was not declared in this scope

phsft-bot · 2023-01-12T15:26:30Z

Build failed on mac12/noimt.
Running on macphsft18.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:24:38.584Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T15:24:38.842Z] /Users/sftnight/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: use of undeclared identifier 'jit'

phsft-bot · 2023-01-12T15:26:45Z

Build failed on ROOT-performance-centos8-multicore/cxx17.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:26:20.342Z] /data/sftnight/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: ‘jit’ was not declared in this scope

phsft-bot · 2023-01-12T15:29:24Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T15:32:13Z

Build failed on ROOT-ubuntu18.04/nortcxxmod.
Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:32:00.639Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T15:32:00.639Z] /mnt/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:55: error: no matching function for call to ‘llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol>::count(std::__cxx11::string)’

phsft-bot · 2023-01-12T15:33:21Z

Build failed on mac12/noimt.
Running on macphsft18.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:31:38.481Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T15:31:38.737Z] /Users/sftnight/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: no viable conversion from 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char>>') to 'const llvm::orc::SymbolStringPtr'

phsft-bot · 2023-01-12T15:33:57Z

Build failed on ROOT-debian10-i386/soversion.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:33:50.687Z] /home/sftnight/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:55: error: no matching function for call to ‘llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol>::count(std::__cxx11::string)’

Axel-Naumann · 2023-01-12T15:36:06Z

With this we go from 141 Billion instructions (pre-llvm-13, i.e. llvm-9) to 134B instr with llvm13, for valgrind --tool=callgrind --fn-skip=_dl_runtime_resolve_avx ./root/tree/dataframe/RDFBenchmarks --benchmark_filter=BM_RDataFrame_JitLargeGraphs/10000

phsft-bot · 2023-01-12T15:36:42Z

Build failed on ROOT-performance-centos8-multicore/cxx17.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:36:19.075Z] /data/sftnight/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:55: error: no matching function for call to ‘llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol>::count(std::__cxx11::string)’

phsft-bot · 2023-01-12T15:37:29Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2023-01-12T15:36:46.052Z] C:\build\workspace\root-pullrequests-build\root\interpreter\cling\lib\Interpreter\IncrementalJIT.cpp(589,5): error C2664: 'llvm::DenseMapBase<llvm::DenseMap<llvm::orc::SymbolStringPtr,llvm::JITEvaluatedSymbol,llvm::DenseMapInfo<llvm::orc::SymbolStringPtr>,llvm::detail::DenseMapPair<KeyT,ValueT>>,KeyT,ValueT,KeyInfoT,BucketT>::size_type llvm::DenseMapBase<llvm::DenseMap<KeyT,ValueT,KeyInfoT,BucketT>,KeyT,ValueT,KeyInfoT,BucketT>::count(const T &) const': cannot convert argument 1 from 'std::string' to 'const T &' [C:\build\workspace\root-pullrequests-build\build\interpreter\cling\lib\Interpreter\obj.clingInterpreter.vcxproj]

phsft-bot · 2023-01-12T16:24:47Z

Build failed on mac11/cxx14.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T16:24:33.039Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T16:24:33.296Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: no viable conversion from 'std::string' (aka 'basic_string<char>') to 'const llvm::orc::SymbolStringPtr'

phsft-bot · 2023-01-12T16:25:22Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T16:24:28.528Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T16:24:28.529Z] /home/sftnight/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:40: error: cannot convert ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’} to ‘llvm::DenseMapBase<llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol>, llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol, llvm::DenseMapInfo<llvm::orc::SymbolStringPtr>, llvm::detail::DenseMapPair<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol> >::const_arg_type_t<llvm::orc::SymbolStringPtr>’ {aka ‘const llvm::orc::SymbolStringPtr&’}

phsft-bot · 2023-01-12T16:41:02Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T16:43:34Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T16:46:46Z

Build failed on ROOT-ubuntu18.04/nortcxxmod.
Running on sft-ubuntu-1804-3.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T16:46:31.971Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T16:46:32.226Z] /mnt/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:45: error: expected primary-expression before ‘.’ token

phsft-bot · 2023-01-12T18:11:42Z

Build failed on ROOT-debian10-i386/soversion.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T18:11:41.259Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T18:11:41.259Z] /home/sftnight/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:43: error: no matching function for call to ‘llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::JITEvaluatedSymbol>::count(std::pair<llvm::orc::SymbolStringPtr, llvm::orc::SymbolLookupFlags>&)’

phsft-bot · 2023-01-12T18:17:22Z

Build failed on mac11/cxx14.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T18:17:06.038Z] FAILED: interpreter/cling/lib/Interpreter/CMakeFiles/obj.clingInterpreter.dir/IncrementalJIT.cpp.o
[2023-01-12T18:17:06.641Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp:589:29: error: no viable conversion from 'std::__1::pair<llvm::orc::SymbolStringPtr, llvm::orc::SymbolLookupFlags>' to 'const llvm::orc::SymbolStringPtr'

phsft-bot · 2023-01-12T18:27:01Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

phsft-bot · 2023-01-12T18:33:56Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2023-01-12T18:33:54.758Z] LINK : fatal error LNK1104: cannot open file 'C:\build\workspace\root-pullrequests-build\build\bin\libCore.dll' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]

phsft-bot · 2023-01-12T19:03:57Z

Build failed on ROOT-ubuntu18.04/nortcxxmod.
Running on sft-ubuntu-1804-3.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-01-12T19:07:24Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 7 more

phsft-bot · 2023-01-12T19:07:28Z

Build failed on mac12/noimt.
Running on macphsft17.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See console output.

Warnings:

[2023-01-12T18:34:18.641Z] /Users/sftnight/build/jenkins/workspace/root-pullrequests-build/root/core/base/src/TDirectory.cxx:1245:7: warning: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Wdeprecated-declarations]

Failing tests:

And 331 more

phsft-bot · 2023-01-12T19:10:17Z

Build failed on ROOT-performance-centos8-multicore/cxx17.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

[2023-01-12T18:45:50.940Z] 846/2422 Test Fix for ROOT-8732 #517: tutorial-dataframe-df101_h1Analysis ...............................................................***Failed Error regular expression found in output. Regex=[: error:] 7.44 sec

Failing tests:

And 59 more

phsft-bot · 2023-01-12T19:38:52Z

Build failed on ROOT-debian10-i386/soversion.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-01-12T20:15:07Z

Build failed on mac11/cxx14.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 623 more

With the upgrade to llvm-13, the JIT lost the ability to re-use existing weak symbols that the JIT had already emitted, instead only looking at dlsym. This causes a significant increase in JITted symbols, and thus a significant slow-down of cling / its JIT. This restores the old behavior, with an identical set of symbols that jet jitted.

Small functions might get inlined, and hiding their definition prevents the inliner from doing its job.

m_JIT.getSymbolAddress() invokes the symbol materializers, which compile (which is sort of okay) but also try autoloading (which totally is not okay). Instead, implement a function to search existing JIT symbols. This can be accelerated by looking up the whole set of symbols, instead of doing it symbol by symbol. I leave that refactoring for later...

phsft-bot · 2023-01-13T16:22:47Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

Axel-Naumann · 2023-01-13T16:25:41Z

Update on #12018 (comment) : currently 141B instr => 130B instr

phsft-bot · 2023-01-13T16:54:27Z

Build failed on mac12/noimt.
Running on macphsft17.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See console output.

Warnings:

[2023-01-13T16:30:21.184Z] /Users/sftnight/build/jenkins/workspace/root-pullrequests-build/root/core/base/src/TDirectory.cxx:1245:7: warning: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Wdeprecated-declarations]

Failing tests:

And 63 more

phsft-bot · 2023-01-13T18:00:20Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

Axel-Naumann · 2023-01-16T21:41:07Z

@phsft-bot build just on windows10/cxx14, mac12/noimt

phsft-bot · 2023-01-16T21:41:13Z

Starting build on windows10/cxx14, mac12/noimt
How to customize builds

phsft-bot · 2023-01-16T23:04:14Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

Without, the JIT fails to compile roottest/root/aclic/misc/assertROOT7027.C correctly.

phsft-bot · 2023-01-17T11:56:51Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

hahnjo

Looks good and seems to pass tests on Linux and macOS. IMO it would be fantastic to reorder / squash the commits to make all intermediate states work fine (especially the last commit is fixing failures and also "find existing weak symbol without materializer:" seems important), but up to you.

Axel-Naumann · 2023-01-17T15:33:21Z

IMO it would be fantastic to reorder / squash the commits to make all intermediate states work fine

The main reason why I like the current small-ish steps (even if it breaks intermediate commits at the cost of more fragile bijection) is that I will certainly be asking myself, "why did I not do this differently" in some months, and having the commit that explains why e.g. I switched from "just ask the JIT for symbols" to "look for symbol names" is extra information. Same for the final Windows "fix", for instance - it's a very distinct / separate step.

Does that make sense?

Axel-Naumann · 2023-01-17T15:42:01Z

Merged by hand, leaving a --no-ff merge commit / branch to signal that this is a set of commits.

Axel-Naumann · 2023-01-30T12:49:14Z

v6-28: #12163

Axel-Naumann requested review from hahnjo and vgvassilev January 12, 2023 12:55

bellenot assigned Axel-Naumann Jan 12, 2023

Axel-Naumann force-pushed the cling-JIT-reuse-weak-JIT-syms branch from 5c7cf42 to fc5fe41 Compare January 12, 2023 15:29

Axel-Naumann marked this pull request as draft January 12, 2023 15:29

Axel-Naumann marked this pull request as ready for review January 12, 2023 15:36

Axel-Naumann force-pushed the cling-JIT-reuse-weak-JIT-syms branch from fc5fe41 to c313072 Compare January 12, 2023 16:40

Axel-Naumann force-pushed the cling-JIT-reuse-weak-JIT-syms branch from c313072 to a465470 Compare January 12, 2023 16:43

Axel-Naumann force-pushed the cling-JIT-reuse-weak-JIT-syms branch from 259d703 to f47e127 Compare January 12, 2023 18:26

Axel-Naumann added 4 commits January 13, 2023 16:37

[cling] JIT: only re-use *large* existing weak functions:

b6d2061

Small functions might get inlined, and hiding their definition prevents the inliner from doing its job.

[cling] JIT: separate re-use of weak sym vars, funcs (NFCI).

7fae1ac

Axel-Naumann force-pushed the cling-JIT-reuse-weak-JIT-syms branch from f47e127 to ba2f10e Compare January 13, 2023 16:22

[cling] For MSVC weak symbols, re-use even small funcs:

42e9bdf

Without, the JIT fails to compile roottest/root/aclic/misc/assertROOT7027.C correctly.

hahnjo approved these changes Jan 17, 2023

View reviewed changes

Axel-Naumann closed this Jan 17, 2023

[cling] Reuse existing weak symbols, also from JIT: #12018

[cling] Reuse existing weak symbols, also from JIT: #12018

Uh oh!

Conversation

Axel-Naumann commented Jan 12, 2023

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Failing tests:

Uh oh!

phsft-bot commented Jan 12, 2023

Failing tests:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Failing tests:

Uh oh!

phsft-bot commented Jan 12, 2023

Failing tests:

Uh oh!

phsft-bot commented Jan 12, 2023

Failing tests:

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

Axel-Naumann commented Jan 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Errors:

Uh oh!

phsft-bot commented Jan 12, 2023

Uh oh!

phsft-bot commented Jan 12, 2023

Axel-Naumann commented Jan 12, 2023 •

edited

Loading

Axel-Naumann commented Jan 17, 2023 •

edited

Loading