[core] Materialize symbols for TError variables

[vepadulano]

The variables present in TError.h are used throughout the ROOT libraries. Anytime one of these variables is requested, e.g. with a simple test such as python -c "import ROOT; ROOT.kError", the interpreter will lookup the corresponding symbol. Previously, the variables were declared and defined in the header directly but were not generating any symbol since the linkage was internal. With this commit, provide external linkage to these variables so that the corresponding symbols are materialized in libCore.so. As a consequence, a huge number of extra lookups by cling is avoided.

This is in direct reference to #14261 (comment) , a followup issue to the original issue at cms-sw/cmssw#43077

These are the results of the patch:

Before:

$: strace -z -f -o log.txt -e trace=openat python3 -c 'import ROOT;print(ROOT.kError);'
$: grep openat log.txt | wc -l
$: 16279

After:

$: strace -z -f -o log.txt -e trace=openat python3 -c 'import ROOT;print(ROOT.kError);'
$: grep openat log.txt | wc -l
$: 417  

FYI @smuzaffar

[phsft-bot]

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

[smuzaffar]

thanks @vepadulano , I have already started the cmssw tests cms-sw#191

[vgvassilev]

Let’s hold off this fix since we probably need to understand why we constexpr does not work.

[vepadulano]

Hi @vgvassilev , I agree this is not a general fix for the root of the issue, rather it is a way to eliminate the problem highlighted in the CMSSW test. And indeed, marking those variables as constexpr is not enough. Also, marking them as inline constexpr doesn't solve the problem (tested on my machine), I was hoping that it would have had the same practical result.

My understanding is that this is strictly required for CMS to adopt ROOT 6.30 in their next software release.

[pcanal]

This needs to be reverted as soon as the underlying issue (#14277) is solved.

[dpiparo]

Let’s hold off this fix since we probably need to understand why we constexpr does not work.

We all agree that a proper fix in Cling is what is needed. However, the last open pre-release of CMSSW 14 closes on January 23rd: if this problem is not fixed, CMS will not be able to pick ROOT 6.30. Merging the changes proposed by this PR fixes known CMS problems and does not exclude a proper fix of Cling. If we manage to have a Cling trust in the next few days and CMS gives its green light, we can always revert the changes in this PR, right?
(Edit: Philippe commented while I was writing. I agree with him.)

[vgvassilev]

We can revert this commit: 3184f75 Do we need to make the constants out-of-line? That'd break performance.

[vepadulano]

Let me try with your suggestion, I will report the results from strace

[vepadulano]

Unfortunately that is not enough. Here are the numbers:

Patch of this PR:

$: strace -z -f -o log.txt -e trace=openat python3 -c 'import ROOT;print(ROOT.kError);'
$: 3000
$: grep openat log.txt | wc -l
$: 323

Revert of 3184f75

$: strace -z -f -o log.txt -e trace=openat python3 -c 'import ROOT;print(ROOT.kError);'
$: 3000
$: grep openat log.txt | wc -l
$: 13297

[vepadulano]

It really seems the discriminating factor is whether kError and other k* constants are present with symbols in libCore.so or not.