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.

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

[ntuple] Refactor RNTupleWriter into RNTupleFillContext #14391

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

hahnjo
Copy link
Member

@hahnjo hahnjo commented Jan 19, 2024

A context is responsible for preparing a cluster, and will be the unit of parallelisation for concurrent writing. Also has a commit to return const reference from RNTupleWriter::GetModel().

@hahnjo hahnjo self-assigned this Jan 19, 2024
@hahnjo hahnjo requested review from jblomer and couet as code owners January 19, 2024 10:39
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

tree/ntuple/v7/inc/ROOT/RNTuple.hxx Outdated Show resolved Hide resolved
tree/ntuple/v7/inc/ROOT/RNTuple.hxx Show resolved Hide resolved
Copy link

github-actions bot commented Jan 19, 2024

Test Results

     9 files       9 suites   1d 19h 5m 5s ⏱️
 2 491 tests  2 488 ✅ 0 💤 3 ❌
21 387 runs  21 384 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit c415866.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I guess as we move forward and support multiple contexts at the same time, we have to revisit some of the methods, e.g. who owns the sink and what GetNEntries() should return.

tree/ntuple/v7/inc/ROOT/RNTuple.hxx Outdated Show resolved Hide resolved
A context is responsible for preparing a cluster, and will be the
unit of parallelisation for concurrent writing.
This is not used yet (RNTupleWriter directly observes the sink's
metrics to avoid an extra prefix), but will be useful for parallel
writing with multiple contexts.
@hahnjo hahnjo force-pushed the ntuple-FillContext branch from a080c9d to c415866 Compare January 22, 2024 08:27
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@hahnjo hahnjo merged commit 072400d into root-project:master Jan 23, 2024
13 of 15 checks passed
@hahnjo hahnjo deleted the ntuple-FillContext branch January 23, 2024 08:43
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.

3 participants