Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new in-memory DataAccess implementation optimized for faster integer reads by using a single contiguous int[] instead of segmented int[][], and wires it into the directory/type system.
Changes:
- Added
RAMInt1SegmentDataAccess(singleint[]-backed, int-only, optional storing via flush/loadExisting). - Extended
DATypewith_1SEGvariants and asingleSegmentflag, and updatedGHDirectoryto instantiate the new implementation. - Added
RAMInt1SegmentDataAccessTestto integrate with the existingDataAccessTesthierarchy.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| core/src/main/java/com/graphhopper/storage/RAMInt1SegmentDataAccess.java | New int[]-backed DataAccess implementation incl. load/flush and capacity management. |
| core/src/main/java/com/graphhopper/storage/GHDirectory.java | Creates RAMInt1SegmentDataAccess when DAType is int-based and single-segment. |
| core/src/main/java/com/graphhopper/storage/DAType.java | Adds _1SEG DA types and singleSegment flag influencing equality/toString. |
| core/src/test/java/com/graphhopper/storage/RAMInt1SegmentDataAccessTest.java | Adds test class integration (currently overrides some base tests). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Like RAM_INT, but backed by a single contiguous int[] for maximum read speed. | ||
| * Not a good fit if the array needs to be resized frequently. Limited to Integer.MAX_VALUE ints | ||
| * No support for short,byte and bytes. | ||
| */ | ||
| public static final DAType RAM_INT_1SEG = new DAType(MemRef.HEAP, false, true, true, true); | ||
| /** | ||
| * See RAM_INT_1SEG | ||
| */ | ||
| public static final DAType RAM_INT_1SEG_STORE = new DAType(MemRef.HEAP, true, true, true, true); |
There was a problem hiding this comment.
will have to refactor this into an enum. but will do this after we merge this here.
| @Override | ||
| public int getSegments() { | ||
| return data.length / (segmentSizeInBytes / 4); | ||
| } |
There was a problem hiding this comment.
this is a bit inconsistent as I would have expected 1 here (hence the 1Segment name). But we can keep it as it is to be more compatible with the other DataAccess classes
It's like RAMIntDataAccess, but it is backed by int[], rather than int[][]. So it does not offer cheap resizing and is limited to 2B ints, but the reading speed can be slightly faster, since we do not need the outer index access.
Even though there are no real segments the implementation still uses the segment size: