Skip to content

DPL: decouple DataAllocator from TableBuilder#2983

Merged
ktf merged 1 commit intoAliceO2Group:devfrom
ktf:decouple-table-builder
Feb 21, 2020
Merged

DPL: decouple DataAllocator from TableBuilder#2983
ktf merged 1 commit intoAliceO2Group:devfrom
ktf:decouple-table-builder

Conversation

@ktf
Copy link
Copy Markdown
Member

@ktf ktf commented Feb 20, 2020

If one needs DataAllocator to create arrow tables, the
TableBuilder.h header should be included before DataAllocator.h.

This way we avoid rebuilding everything when ASoA.h changes.

We should probably do the same for boost and ROOT.

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Feb 20, 2020

@matthiasrichter this is our Holy Grail. It allows to expose arrow only to the tasks / workflows which use it. We could do something similar for ROOT and for boost, actually.

@ktf ktf force-pushed the decouple-table-builder branch from b5bc5d6 to ca8aa83 Compare February 20, 2020 20:50
If one needs DataAllocator to create arrow tables, the
TableBuilder.h header should be included before DataAllocator.h.

This way we avoid rebuilding everything when ASoA.h changes.

We should probably do the same for boost and ROOT.
@ktf ktf force-pushed the decouple-table-builder branch from ca8aa83 to c179106 Compare February 20, 2020 20:51
@ktf ktf merged commit 78b4c54 into AliceO2Group:dev Feb 21, 2020
@ktf ktf deleted the decouple-table-builder branch February 21, 2020 06:25
@matthiasrichter
Copy link
Copy Markdown
Collaborator

That looks great! I think that is good enough for the moment and allows to disentangle compilation. Actually, the sequence of including the headers should not matter, right? Only before the actual function call, the TableBuilder.h needs to be included.

Lets clean up ROOT and BOOST support in a similar way (when we have time).

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Feb 21, 2020

Not sure if only before the actual function call or before the DataAllocator.h is included. Template resolution is tricky. We need need to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants