Feat/read write direct - #88
Conversation
| * domain, or if the codec pipeline does not allow addressing inner chunks | ||
| */ | ||
| @Nullable | ||
| public ByteBuffer readInnerChunkDirect(long[] innerChunkCoords) throws ZarrException { |
There was a problem hiding this comment.
Maybe the arg could be a long[2][] addressing first the shard and then the inner chunk?
There was a problem hiding this comment.
I would disagree here. The recursive top to bottom descent already happens at other points in the program, and adressing each nested shard, would be a complete overkill here!
Array.readInnerChunkDirect -> Gives us back the shard that it is on
ShardingIndexedCodec -> gives us the mid-shards
while nestedcodec L2 -> gives us the leaf where it is, which gves us the complete recursive top to bottom descent, already.
Therefore no need for another parameter!
There was a problem hiding this comment.
I think we will need long[][] here. First, to address the shard, then the chunk within the shard (and potentially further nested). My main reason is that in the future there will be variable-length chunk grids where different shards may have different number of chunks. Then, you cannot map the inner chunks to a flat coordinate system anymore.
Or see my other comment: https://github.com/zarr-developers/zarr-java/pull/88/changes#r3933950035
| * domain, or if the codec pipeline does not allow addressing inner chunks | ||
| */ | ||
| @Nullable | ||
| public ByteBuffer readInnerChunkDirect(long[] innerChunkCoords) throws ZarrException { |
There was a problem hiding this comment.
I think a writeInnerChunkDirect could also be interesting. It would read and parse (no decode) an existing shard, swap out the bytes for one provided inner chunk, and write out the shard again.
However, users would likely want to write multiple inner chunks for which this would be inefficient. Instead, this could also be a builder pattern, where you start a session for a shard (read and parse existing), then write multiple inner chunks to a buffer, and finally write (commit) the whole shard. Similar to what ShardingIndexedCodec.encode does internally.
There was a problem hiding this comment.
completly agree, editing multiple chunks directly was done pretty inefficient. Added functionality to directly write to multiple chunks at the same time!
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
|
adjusted write to be more efficient. ÌnnerChunkWriter.java is now doing any sort of inner chunk writing, collecting all needed chunks and reconstructing the shard + metadata by going through all of it iteratively. |
| * <p> | ||
| * Instances are <b>not thread-safe</b>. | ||
| */ | ||
| public final class InnerChunkWriter { |
There was a problem hiding this comment.
Maybe this could implement AutoClouseable?
| * @return the encoded inner chunk bytes, or {@code null} if the inner chunk is not present | ||
| */ | ||
| @Nullable | ||
| protected abstract ByteBuffer readInnerChunkEncoded( |
There was a problem hiding this comment.
| protected abstract ByteBuffer readInnerChunkEncoded( | |
| protected abstract ByteBuffer readInnerChunkDirect( |
| * domain, or if the codec pipeline does not allow addressing inner chunks | ||
| */ | ||
| @Nullable | ||
| public ByteBuffer readInnerChunkDirect(long[] innerChunkCoords) throws ZarrException { |
There was a problem hiding this comment.
I think we will need long[][] here. First, to address the shard, then the chunk within the shard (and potentially further nested). My main reason is that in the future there will be variable-length chunk grids where different shards may have different number of chunks. Then, you cannot map the inner chunks to a flat coordinate system anymore.
Or see my other comment: https://github.com/zarr-developers/zarr-java/pull/88/changes#r3933950035
| * For a sharded array this is the inner chunk shape of the sharding codec (the innermost one, if | ||
| * shards are nested); for any other array it is simply {@link ArrayMetadata#chunkShape()}. | ||
| */ | ||
| public int[] innerChunkShape() { |
There was a problem hiding this comment.
This feels out-of-place. Array doesn't even have a "chunkShape" getter, so why would it have an "innerChunkShape" getter. I wonder if, instead, we should have a few new classes that represent the "chunking" of an array, e.g. "RegularChunking", "ShardedRegularChunking", "RectilinearChunking" (future). Depending on the type the user code can then work with the chunking. As a companion there could also be classes that address chunks, which would replace the long[] chunkCoords (and long[][] innerChunkCoords).
| * This is the encoded-bytes counterpart of {@link #readInnerChunkDirect(long[])}. See | ||
| * {@link InnerChunkWriter} for the safety contract. | ||
| */ | ||
| public InnerChunkWriter innerChunkWriter() { |
There was a problem hiding this comment.
I would rename this and remove other writeInnerChunkDirect method.
| public InnerChunkWriter innerChunkWriter() { | |
| public InnerChunkWriter writeInnerChunkDirect() { |
| * {@link InnerChunkWriter} for the safety contract. | ||
| */ | ||
| public InnerChunkWriter innerChunkWriter() { | ||
| return new InnerChunkWriter(this); |
There was a problem hiding this comment.
Have you thought about scoping the InnerChunkWriter to a shard, i.e. new InnerChunkWriter(this, new long []{1,2,3})?
2 participants
implements #85 now, direct reads and writes for arrays.
These methods skip the codec pipeline for neccesary methods, avoiding the decode, encode round trip, which would imply a quality loss for lossy codecs.