[TDF] optimizations to the event loop #442
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Values of temporary columns are now stored in unique_ptrs instead of shared_ptrs, as we do not need to share ownership. The update of these values are now handled by a separate Update function. The content of the unique_ptr is updated, instead of allocating a new object for each new evaluation as it was before. This last change forces the types of the values of temporary columns to be default-constructible and assignable, rather than copy-constructible as before. The change is necessary to limit usage of shared_ptrs and heap allocation inside the event-loop.
This is a long due optimization that required several changes in the internal
behaviour of TDataFrame{Impl,Action,Branch,Filter}. Unfortunately all changes
are entangled, and I don't see a way to split them in multiple commits that compile.
- Introduced class TDataFrameValue, that abstracts the notion of access to column values.
At compile-time, TDataFrameValue chooses between TTreeReaderArray and TTreeReaderValue.
At runtime, it resolves the column value to either a real TTree branch or a temporary column.
During the event-loop, TDataFrameValue makes sure that values are updated at each entry,
offering a single interface to handle TTree branches and temporary columns.
- TDataFrame{Action,Branch,Filter} now store tuples of TDataFrameValues instead of shared_ptrs
to TTreeReaderValueBase.
- TDataFrameValue also checks that the return type of temporary column expressions
corresponds to the inferred type of the branch that makes use of it.
- TDataFrameImpl was moved at the beginning of the file because the other classes
now need to call methods on it, e.g. to invoke InitTDFValues.
- the GetBranchValue methods have been eliminated in favour of TDataFrameValue
- the BuildReaderValue methods now call InitTDFValue to initialize their TDataFrameValues;
the free function BuildReaderValues has been eliminated
Member
|
Merged after some manual testing. Thanks a lot for this great development! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In particular, all creations and deletions of
shared_ptrs have been removed from the event loop.This is a long due optimization that required several changes in the internal behaviour of TDataFrame{Impl,Action,Branch,Filter}. Unfortunately all changes are entangled, so the third commit is quite fat.
The main change to the internal logic is that
TDataFrame{Action,Branch,Filter}now store a tuple ofTDataFrameValues rather than (possibly null) shared pointers toTTreeReaderValueBase.TDataFrameValueoffers a transparent, unified interface to the different kinds of values that the nodes must handle: temporary columns, to be evaluated on-the-fly,TTreeReaderArrays that must be converted toarray_views andTTreeReaderValues.TDataFrameValuealso incorporates validity checks on the value types, e.g. that arrays read viaTTreeReaderArrayare actually contiguous in memory and that the type of a temporary column is the same as the type expected by the node that makes use of it.