Make DicomTile objects more accessible - #4451
Conversation
sbesson
left a comment
There was a problem hiding this comment.
This PR introduces a couple of public APIs allowing to access the internal tiles associated with each series and precomputes the index of plane associated to every single tile.
Looking at the timing of the integration tests
for i in $(seq 1 30); do curl -s -o log https://bf-testing-results.s3.amazonaws.com/2026/2026-06-$(printf %02d i)/dicom.log && grep "Total time:" log; done
for i in $(seq 1 30); do curl -s -o log https://bf-testing-results.s3.amazonaws.com/2026/2026-07-$(printf %02d i)/dicom.log && grep "Total time:" log; done
the average completion time is 260 min +/- 14 min between 2026-06-01 and 2026-07-03 (before this PR was introduced) vs 249 min +/- 18 min between 2026-07-04 and 2026-07-30.
Is it correct that the major performance improvements will be seen when calling openBytes or openCompressedBytes as the tile lookup should benefit from the preliminary computation? In that case, is it pertinent to also measure the timing of a conversion from DICOM to OME-TIFF for some sample WSI DICOM?
| } | ||
|
|
||
| private void calculateTilePlaneIndexes() { | ||
| List<Double> zs = zOffsets.get(getCoreIndex()); |
There was a problem hiding this comment.
Should this be getZOffsets(getCoreIndex()) to use the new API?
Yes, I'd expect |
sbesson
left a comment
There was a problem hiding this comment.
I ran bfconvert -noflat and bfconvert -noflat -precompressed with and without this PR using https://downloads.openmicroscopy.org/images/DICOM/wsi/2023-04-28/PARNED-0BNNF4_B2_Q30/ as the input using 3 repeats.
Average conversion times are the following:
| Bio-Formats 8.5.0 | PR 4451 | |
|---|---|---|
| bfconvert -noflat | 559s +/- 12s | 548s +/- 9s |
| bfconvert -noflat -precompressed | 143s +/- 3s | 139s +/- 2s |
In summary, this PR is improving access to the internal DICOM tiles via a new public API and refactors a few places in the reader. The nightly repository tests confirm the reading is unaffected and the conversion tests confirm no regression and a possible slight performance improvement. Were you expecting a more substantial writing change and would you like me to perform additional conversion testing on another sample?
Otherwise, no objections to merging from my perspective
I didn't have a specific performance change in mind, other than "not worse, maybe slightly better". I think the test you have done is sufficient to confirm that. |
Adds a bit of API to
DicomReaderto make it easier to getDicomTileobjects corresponding to a particular resolution and/or plane. This also improves indexing of tiles, which should hopefully make image reading faster overall.Local tests pass, but will exclude this temporarily if tests fail with it included. Since this is expected to improve performance, will need to compare total runtime of passing tests with and without this change, for several days each.