Skip to content

[ntuple] Preparations for introducing slow merging in the RNTupleMerger - #23576

Open
silverweed wants to merge 1 commit into
root-project:masterfrom
silverweed:ntuple_merge_slow_3_3
Open

silverweed wants to merge 1 commit into
root-project:masterfrom
silverweed:ntuple_merge_slow_3_3

Conversation

@silverweed

Copy link
Copy Markdown
Contributor

This Pull request:

refactors the RNTupleMerger to modularize a bit better the parts that depend on the merging strategy (slow or non-slow, aka L4 or L1/2/3).

A new interface, RNTupleMergeStrategy, is introduced which provide the common functions that both strategy need to implement (currently only the non-slow strategy exists, called RNTupleMergeStrategyDefault).
Most of the code currently belonging to the RNTupleMerger class is moved into this strategy class, with no functional changes. The only actual changes in these functions are:

  • the RNTupleMergeData struct is removed as it's being replaced by the Strategy class itself (all its fields now belong to the Strategy);
  • the fNumDstEntries field of the mergeData is removed altogether because I realized it's not needed (replaced by destination.GetNEntries().

A later commit will introduce the slow merging strategy, with minimal impact on the existing code thanks to this change.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@silverweed silverweed self-assigned this Oct 1, 2026
@silverweed
silverweed requested a review from jblomer as a code owner October 1, 2026 15:32
Will help introducing the Slow merging path later
@silverweed
silverweed force-pushed the ntuple_merge_slow_3_3 branch from 8362a5c to 3612562 Compare October 2, 2026 07:40
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 19h 17m 34s ⏱️
 3 880 tests  3 879 ✅ 0 💤 1 ❌
80 409 runs  80 408 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 3612562.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant