[hist] Annotate RHist with R__CLING_PTRCHECK(off).#22219
Conversation
Test Results 22 files 22 suites 3d 14h 32m 32s ⏱️ Results for commit 7d64f80. ♻️ This comment has been updated with latest results. |
In JITted RDF nodes, cling added an unnecessary pointer check, which significantly slows down execution. To allow for compiling the package standalone, the macro is defined in RHistUtils instead of including ROOT's RTypes.h
hahnjo
left a comment
hahnjo
left a comment
There was a problem hiding this comment.
Still trying to figure out why we only need to annotate the two classes. I would expect to also need it in RHistEngine, RHistStats, RHist
|
|
||
| #ifdef __CLING__ | ||
| // This attribute can significantly speed up JIT-compiled code in CLING | ||
| // It is also defined in ROOT, so keep the definitions the same. |
There was a problem hiding this comment.
Wondering if we should give it a different name...
There was a problem hiding this comment.
I found it in the C standard, and it's covering the exact case where two (sometimes related, sometimes unrelated) include paths lead to the macro being redefined.
6.10.3 p2:
An identifier currently defined as an object-like macro shall not be redefined by another
#define preprocessing directive unless the second definition is an object-like macro
definition and the two replacement lists are identical. Likewise, an identifier currently
defined as a function-like macro shall not be redefined by another #define
preprocessing directive unless the second definition is a function-like macro definition
that has the same number and spelling of parameters, and the two replacement lists are
identical.
By the above, we are doing it standard-compliant as it's now.
There was a problem hiding this comment.
But indeed, we could define also an RHist-specific macro that expands to the same attribute. I don't see a downside of that, so I guess that should be the way forward.
In general, that might be correct, but the code path in RDF is such that it doesn't trigger the pointer check. |
|
Is this superseded by #22288 ? |
Indeed, this will achieve the same speedup, and cover even more cases. |
3 participants
In JITted RDF nodes, cling added an unnecessary pointer check, which significantly slows down execution.