root icon indicating copy to clipboard operation
root copied to clipboard

[ntuple] Move RColumnElement concrete definitions out of the header file

Open silverweed opened this issue 1 year ago • 1 comments

This Pull request:

by @hahnjo suggestion, since we are exposing a type-erased virtual interface for RColumnElement through RColumnElementBase, we can hide all the concrete definitions into a cxx file. This has the benefit of reducing the compile time of any translation unit that includes RColumnElementBase.hxx significantly, as the compiler won't have to instantiate all the combination matrix of RColumnElement<CppT, ColumnType> every time. It also let us have better control on exactly which types of RColumnElement we allow to instantiate by explicitly listing them into a proxy enum.

Changes or fixes:

  • renamed RColumnElement.hxx to RColumnElementBase.hxx
  • moved all concrete definitions from RColumnElementBase.hxx to src/RColumnElement.hxx. This is a private header file that is included by RColumnElement.cxx and by ntuple_endian.cxx. The reason to separate this from RColumnElement.cxx is that ntuple_endian.cxx needs to simulate a big-endian machine by defining R__LITTLE_ENDIAN 0, which changes the definitions of some RColumnElements. To avoid including the whole RColumnElement.cxx in the test, we decided to split the definitions into a file that can be included independently by the test. It's not a perfect solution (the test executable ends up with mismatching instantiations of RColumnElement since it links to libROOTNTuple.so) but it's technically not worse than before. We still might want to think of alternative solutions.
  • introduces an enum EColumnCppType that lists all the allowed c++ in-memory types for RColumnElement. This is used internally to map the templated RColumnElementBase::Generate to a non-templated function that can be implemented in the cxx file.

Checklist:

  • [x] tested changes locally
  • [ ] updated the docs (if necessary)

silverweed avatar Aug 23 '24 09:08 silverweed

Test Results

    13 files      13 suites   2d 19h 23m 54s :stopwatch:  3 027 tests  3 027 :white_check_mark: 0 :zzz: 0 :x: 33 832 runs  33 832 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 28278683.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Aug 23 '24 11:08 github-actions[bot]