[ntuple] Rename kTypeDAOS to kTypeObject64 and kTypeS3 to kTypeMulti#22434
[ntuple] Rename kTypeDAOS to kTypeObject64 and kTypeS3 to kTypeMulti#22434JasMehta08 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Let's not update the binary specification just yet. For the moment, we just implement them as experimental extensions.
| static constexpr std::uint64_t kMaskReservedBit = 1ull << 60; | ||
|
|
||
| /// To save memory, we use the most significant bits to store the locator type (file, DAOS, zero page, | ||
| /// To save memory, we use the most significant bits to store the locator type (file, Object64, zero page, |
There was a problem hiding this comment.
Actually, that may become a problem for the multi locator that gets one of its two 32bit integers cut by 4 bits. I guess we should cut the the offset, effectively restricting every multi object to 256MiB. That would be ok if backends always also can use the Object64 locator to break out that limitation.
Alternatively, we can restrict the number of multi objects (pages, in the worst case) to 256M. That seems a lot but the limitation is more difficult to break out from. To be discussed.
Perhaps for this PR: let's just make sure that we know into which of the two 32bit integers the four flag bits cut.
There was a problem hiding this comment.
I was thinking either a 32/28 bit split (256 MiB/object, 4G object limit) or a 30/30 split (1 GiB/object, 1G object limit).
Multi range requests have a limit on how many ranges you can pack into one request.
If that cap is low (100-500), either split works fine in practice.
If it's higher (~1000), 30/30 gives the headroom to pack large (~1 MiB) pages into one object, while 32/28 would start capping performance.
There was a problem hiding this comment.
Interesting, the 30/30 split is clever!
| case RNTupleLocator::kTypeMulti: | ||
| size += SerializeLocatorPayloadObject64(locator, payloadp); |
There was a problem hiding this comment.
We probably want to make the serialization of the 2 32bit integers explicit. For a follow-up PR.
Test Results 21 files 21 suites 3d 5h 32m 55s ⏱️ Results for commit d796341. ♻️ This comment has been updated with latest results. |
JasMehta08
force-pushed
the
locator-type-rename
branch
from
38138a6 to
d796341
Compare
2 participants
This Pull request:
Changes or fixes:
Consolidate locator type naming as suggested in the review of PR #22351.
kTypeDAOSis renamed tokTypeObject64since it is used by both DAOS and S3 Mode B backends.kTypeS3is renamed tokTypeMultisince it represents a multi-field packed locator. Wire format values (0x02 and 0x03) are unchanged.Updated
BinaryFormatSpecification.mdto register locator types 0x02 (Object64) and 0x03 (Multi) in the type table, and added an Object64 payload format diagram to the well-known payload formats section describing the shared layout used by both types.Checklist:
This PR is a follow-up to #22351