Skip to content

[ntuple] Add RNTupleLocatorMulti class for kTypeMulti locator#22577

Open
JasMehta08 wants to merge 1 commit into
root-project:masterfrom
JasMehta08:ntuple-locator-multi-class
Open

[ntuple] Add RNTupleLocatorMulti class for kTypeMulti locator#22577
JasMehta08 wants to merge 1 commit into
root-project:masterfrom
JasMehta08:ntuple-locator-multi-class

Conversation

@JasMehta08

Copy link
Copy Markdown
Contributor

This Pull request:

(Is a part of the GSoC 2026 project S3 Backend for RNTuple.)

Changes or fixes:

Follow-up to #22434. Introduces a dedicated RNTupleLocatorMulti class for the kTypeMulti locator, replacing the previous reuse of RNTupleLocatorObject64.

The new class stores the object identifier and byte offset as two explicit 32-bit fields plus a separate 4-bit reserved field. GetLocation() returns the canonical packed 64-bit value (parallel to RNTupleLocatorObject64::GetLocation()), with the layout:

  • bits 63..60: 4 reserved bits
  • bits 59..30: 30-bit object identifier (max ~1 billion objects)
  • bits 29..0: 30-bit byte offset (max 1 GiB per object)

The (uint32_t, uint32_t) constructor and SetReserved() validate their inputs against the 30-bit and 4-bit ranges respectively. The 0x03 payload remains a 4/8-byte size followed by an 8-byte packed value, so the wire format stays identical.

RNTupleLocator::SetPosition and GetPosition<> are split accordingly so RNTupleLocatorObject64 is now reserved for kTypeObject64 only, and the new RNTupleLocatorMulti overload/specialization handle kTypeMulti.

Checklist:

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

This PR is a follow-up #22434

@JasMehta08 JasMehta08 requested a review from jblomer as a code owner June 11, 2026 09:18
@jblomer jblomer self-assigned this Jun 11, 2026
@jblomer jblomer assigned JasMehta08 and unassigned jblomer Jun 11, 2026

@jblomer jblomer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Some simplification is possible, I think, by not storing reserved bits altogether.


std::uint32_t fObjectId = 0;
std::uint32_t fOffset = 0;
std::uint8_t fReserved = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thinking about it, we probably don't need to store the reserved bits because we anyway don't know what to do with them. That should also avoid the need for masks.

} else {
RNTupleSerializer::SerializeUInt64(locator.GetNBytesOnStorage(), buffer);
}
RNTupleSerializer::SerializeUInt64(data.GetLocation(), buffer + sizeofNBytesOnStorage);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here, I think we should write the two integers (object id, offset) as 32bit integers to disk. This avoids any potential issues of platform-independence when we combine the two integers in a single 64 bit int.

@github-actions

Copy link
Copy Markdown

Test Results

    19 files      19 suites   3d 2h 44m 25s ⏱️
 3 863 tests  3 810 ✅ 0 💤 53 ❌
66 233 runs  66 168 ✅ 1 💤 64 ❌

For more details on these failures, see this check.

Results for commit 0d8ca56.

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.

2 participants