jsedding commented on PR #3151: URL: https://github.com/apache/jackrabbit-oak/pull/3151#issuecomment-5830273781
Thanks for the optimization, the `UUID` allocation for the lookup had been bothering me for some time. Having an allocation-free lookup makes a lot of sense. OAK-12417 describes removing `RemoteSegmentArchiveEntry#getUuid()` and building UUIDs lazily for `getSegmentUUIDs()`, however the current implementation still retains the UUID field and uses `getUuid()` in `getSegmentUUIDs()`. I think it would be good to adjust the JIRA description. Regarding the memory footprint, I expect this change to provide a modest improvement, but less than stated in the PR description. The previous UUID-based index was already re-using the UUID instances from `RemoteSegmentArchiveEntry#uuid` and the map implementation holding the index was a `java.util.ImmutableCollections.MapN` that uses an `Object[]` internally to hold both the keys and values. The array used in the `SegmentIndex` will be smaller, because the UUIDs are reduced to an `int` and used as the array index. If we stop caching the UUIDs in `TarReader#segmentUUIDs`, the only current caller of `SegmentArchiveReader#getSegmentUUIDs`, then we could replace `RemoteSegmentArchiveEntry#uuid` with two long fields (msb, lsb) again, removing the UUIDs from memory. I think there are no production/hot code-paths using `TarReader#getUUIDs()`, so maybe that would make sense. But that should be a separate ticket. Minor nit: the new Java files use a differently formatted/cased Apache license header than the exact form documented in AGENTS.md. It will likely be accepted by RAT, but aligning it with the repository’s standard header would keep the new files consistent with the project convention. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
