Skip to content

[ntuple] Some cleanups in the page source#22391

Merged
jblomer merged 5 commits into
root-project:masterfrom
jblomer:ntuple-cleanup-loadpage
May 28, 2026
Merged

[ntuple] Some cleanups in the page source#22391
jblomer merged 5 commits into
root-project:masterfrom
jblomer:ntuple-cleanup-loadpage

Conversation

@jblomer

@jblomer jblomer commented May 22, 2026

Copy link
Copy Markdown
Contributor

In preparation for huge-file support, some cleanup in the RPageSource family of classes.

  • Don't deal with zero pages or with the descriptor when loading concrete pages
  • DAOS: remove object-per-cluster mapping

There is more to improve, left for follow-up PRs.

@jblomer jblomer self-assigned this May 22, 2026
@jblomer jblomer force-pushed the ntuple-cleanup-loadpage branch from 5422338 to 26fff4b Compare May 22, 2026 15:26
Comment thread tree/ntuple/inc/ROOT/RPageStorage.hxx Outdated
Comment thread tree/ntuple/inc/ROOT/RPageStorage.hxx Outdated
@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 12h 35m 8s ⏱️
 3 859 tests  3 814 ✅   0 💤 45 ❌
77 063 runs  76 861 ✅ 157 💤 45 ❌

For more details on these failures, see this check.

Results for commit a8107c2.

♻️ This comment has been updated with latest results.

Comment thread tree/ntuple/src/RPageStorageFile.cxx
Comment thread tree/ntuple/src/RPageStorageDaos.cxx Outdated
@jblomer jblomer force-pushed the ntuple-cleanup-loadpage branch from 26fff4b to de3fb29 Compare May 26, 2026 12:43
@jblomer jblomer marked this pull request as draft May 26, 2026 13:42
jblomer added 4 commits May 26, 2026 16:52
The per-cluster/per-page OID mapping option was already unused
(hard-coded). Going forward, the DAOS backend should only
use the one-object-per-page option.
@jblomer jblomer force-pushed the ntuple-cleanup-loadpage branch from de3fb29 to a8107c2 Compare May 26, 2026 14:54
@jblomer jblomer marked this pull request as ready for review May 26, 2026 14:54
@jblomer jblomer requested review from pcanal and silverweed May 27, 2026 07:50
@jblomer jblomer merged commit a9cc13f into root-project:master May 28, 2026
31 of 33 checks passed
@jblomer jblomer deleted the ntuple-cleanup-loadpage branch May 28, 2026 10:55
Comment on lines +414 to +419
if (pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) {
assert(!pageInfo.HasChecksum());
memcpy(const_cast<void *>(sealedPage.GetBuffer()), ROOT::Internal::RPage::GetPageZeroBuffer(),
sealedPage.GetBufferSize());
return;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would we ever need to load a sealed zero page? According to the coverage information, this code is never executed...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adressed in 3f630db

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.

4 participants