Fix and expand range checks when performing tile retrieval #11
Conversation
|
|
||
| public void checkReadSize(int[] shape) { | ||
| long length = length(shape); | ||
| long maxLength = maxPlaneWidth * maxPlaneHeight; |
There was a problem hiding this comment.
Would it be worth doing long multiplication here too? Or do we know for sure that maxPlaneWidth * maxPlaneHeight will always be less than Integer.MAX_VALUE?
There was a problem hiding this comment.
We don't know for sure but if you were to configure your server like that all sorts of other stuff would break spectacularly.
src/test/java/com/glencoesoftware/omero/zarr/ZarrPixelBufferTest.java
Outdated
Show resolved
Hide resolved
| resolutions); | ||
|
|
||
| // Hack the .zarray so we can appear as though we have more data than | ||
| // we actually have written above. |
There was a problem hiding this comment.
I assume the rationale if that is we want to generate an OME-Zarr of a given dimensions without actually writing the underlying test data (which increases the test time for no value).
@melissalinkert this makes me wonder whether we could introduce an option in bioformats2raw to generate Zarr datasets with sparse data
There was a problem hiding this comment.
I assume the rationale if that is we want to generate an OME-Zarr of a given dimensions without actually writing the underlying test data (which increases the test time for no value).
Correct. Creating an array large enough to not end up with earlier out of bounds checks can take minutes.
src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java
Outdated
Show resolved
Hide resolved
chris-allan
force-pushed
the
fix-range-checks
branch
from
8083a7c to
8a695fa
Compare
|
@melissalinkert: Anything else you'd like to see here before we merge this. It won't go into a release for a while anyway so it also won't be a problem to adjust if you think of something later. |
|
Thought about cutting a patch release with these additional checks ? Or are there other quick fixes/features we'd like to get in as well? |
Much more thorough range changes for tile retrieval. In particular this will prevent the allocation of intermediate large arrays during the tile caching phase of tile retrieval only for the resulting read to fail.
/cc @mabruce