Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RDF] Add option to not clone histograms in MT-RDF #17503

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

[RDF] Add option to not clone histograms in MT-RDF #17503

wants to merge 4 commits into from

Conversation

hageboeck
Copy link
Member

@hageboeck hageboeck commented Jan 23, 2025 •
edited
Loading

  • Add a wrapper for objects which are not thread safe
  • Write an RDF action that uses TH3D without cloning them

TODOs:

  • Document the feature
  • Converge on how to enable the interface
    • Extend to 1D, 2D, ND?
    • Provide clone = Auto option?

@hageboeck hageboeck self-assigned this Jan 23, 2025
@hageboeck
[RDF] Add a wrapper to make histograms threadsafe.
e436864 
When histogram objects without thread safety are used in RDF, they have
to be cloned. This can consume a prohibitive amount of memory, so
wrapping them in a thread-safe object can be a way out.
Here, a wrapper is added that will fill a single histogram under lock,
but it will not block the work of other threads by providing a lock-free
fill queue.
@hageboeck
[RDF] Move iterator functions of FillHelper out of the class.
098211b 
These functions are useful also for other action helpers, and they could
be simplified using "if constexpr". To reuse them, they are now outside
of the class in the namespace ROOT::Internal::RDF.
@hageboeck
[RDF] Add an ActionHelper for filling histograms without cloning.
e53d46a 
This new action helper makes use of the class BufferedFillWrapper,
which manages a single histogram for all threads. For the moment,
this was only implemented for TH3D, but it can be extended easily.
@hageboeck
[RDF][NFC] Formatting of comments.
7863144
@github-actions GitHub Actions
Copy link

Test Results

    16 files      16 suites   4d 3h 37m 35s ⏱️
 2 653 tests  2 629 ✅ 23 💤 1 ❌
41 815 runs  41 813 ✅  0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 7863144.

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

@bellenot bellenot Awaiting requested review from bellenot bellenot will be requested when the pull request is marked ready for review bellenot is a code owner

@martamaja10 martamaja10 Awaiting requested review from martamaja10 martamaja10 will be requested when the pull request is marked ready for review martamaja10 is a code owner

@vepadulano vepadulano Awaiting requested review from vepadulano vepadulano will be requested when the pull request is marked ready for review vepadulano is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@hageboeck hageboeck

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@hageboeck