Reduce DICOM write time #4164
Reduce DICOM write time #4164
Conversation
This reduces the number of writes during initialization, which reduces overall conversion time.
There was a problem hiding this comment.
Tested using the set of sample files previously mentioned in #4190 and previous efforts to add pre-compressed support as well as the QPTIFF file mentioned in the PR description.
The conversion was executed with different number of options using both Bio-Formats 7.3.0 as the ground truth and this PR.
| Source | Target | precompressed | Compression | Tile size | Bio-Formats 7.3.0 | Bio-Formats 8.0.0-SNAPSHOT |
|---|---|---|---|---|---|---|
| CMU-1.svs | CMU-1.dcm | no | JPEG | 227.903s | 227.787s | |
| CMU-1.svs | CMU-1.dcm | no | JPEG-2000 | 679.377s | 683.509s | |
| CMU-1.svs | CMU-1.dcm | yes | 78.793s | 79.611s | ||
| CMU-1.svs | CMU-1.dcm | no | JPEG | 512x512 | 180.712s | 183.06s |
| CMU-1.ndpi | CMU-1.dcm | no | JPEG | 378.301s | 111.163s | |
| CMU-1.ndpi | CMU-1.dcm | no | JPEG-2000 | 939.062s | 645.053s | |
| CMU-1.ndpi | CMU-1.dcm | yes | N/A | 15.825s | ||
| CMU-1_0_0.dcm | CMU-1.ome.tiff | no | JPEG | 239.903s | 132.872s | |
| CMU-1_0_0.dcm | CMU-1.ome.tiff | no | JPEG-2000 | 1288.689s | 1230.261s | |
| HandEcompressed_Scan1.qptiff | HandEcompressed_Scan1.dcm | no | JPEG | 256x256 | 262.394s | 266.762s |
| HandEcompressed_Scan1.qptiff | HandEcompressed_Scan1.dcm | no | JPEG | 512x512 | 132.6s | 135.336s |
As can be seen in this table, this PR does not cause any regression but in most scenarios there does not seem to be any significant improvement in terms of writing performance. Conversion times are largely identical to 7.3.0 with the notable exception of the NDPI -> DICOM conversion which seems to be improved.
Especially when repacking tiles, many duplicate calls to this method can get expensive.
|
Thanks, @sbesson. The NDPI difference may be due to picking more suitable default tile sizes, i.e. https://github.com/ome/bioformats/pull/4181/files#diff-32978063b81116de867b85238d17dd0f0a9fcc06c1e1689cb64bae18b7e3ee02L317. 9946211 is a better try at reducing tile processing times. Trying and with current state of which appears to be a ~15% improvement in both cases. |
There was a problem hiding this comment.
Retested the set of sample files and conditions using the latest release of Bio-Formats 7.3.0, Bio-Formats 8.0.0-SNAPSHOT built from the current HEAD of develop 54cf1e8 and Bio-Formats 8.0.0-SNAPSHOT built from the HEAD of this Pull Request 9946211. Also validated that the output of these conversions read as expected.
Below is the table of the conversion times for the different samples and conditions:
| Source | Target | precompressed | Compression | Tile size | 7.3.0 | 54cf1e8 | 9946211 |
|---|---|---|---|---|---|---|---|
| CMU-1.svs | CMU-1.dcm | no | JPEG | 226.751s | 225.782s | 202.474s | |
| CMU-1.svs | CMU-1.dcm | no | JPEG-2000 | 690.253s | 684.681s | 645.753s | |
| CMU-1.svs | CMU-1.dcm | yes | 78.024s | 78.972s | 82.928s | ||
| CMU-1.svs | CMU-1.dcm | no | JPEG | 512x512 | 181.503s | 181.117s | 157.338s |
| CMU-1.ndpi | CMU-1.dcm | no | JPEG | 379.319s | 113.521s | 113.802s | |
| CMU-1.ndpi | CMU-1.dcm | no | JPEG-2000 | 932.658s | 647.821s | 638.946s | |
| CMU-1.ndpi | CMU-1.dcm | yes | N/A | 15.239s | 16.066s | ||
| CMU-1_0_0.dcm | CMU-1.ome.tiff | no | JPEG | 238.852s | 139.076s | 135.982s | |
| CMU-1_0_0.dcm | CMU-1.ome.tiff | no | JPEG-2000 | 1315.766s | 1180.745s | 1132.103s | |
| CMU-1_0_0.dcm | CMU-1.ome.tiff | yes | JPEG | 6.366s | 6.747s | ||
| CMU-1_0_0.dcm | CMU-1.ome.tiff | yes | JPEG-2000 | 15.124s | 19.161s | ||
| HandEcompressed_Scan1.qptiff | HandEcompressed_Scan1.dcm | no | JPEG | 256x256 | 263.58s | 266.288s | 251.463s |
| HandEcompressed_Scan1.qptiff | HandEcompressed_Scan1.dcm | no | JPEG | 512x512 | 134.43s | 137.138s | 117.57s |
This confirms that some of the performance improvement in the NDPI -> DICOM workflow noted in the previous review has been introduced prior to this PR in the development line.
Overall, the conversion times with this PR included are found to be either comparable or smaller than those without this PR included. Based on these numbers, I think it makes sense to include these changes in the upcoming release of Bio-Formats
|
A quick thought: is there any possibility that the DEBUG-level logging of StopWatch could slow things down in a GUI application, e.g. ImageJ or QuPath? |
|
In ImageJ, I see no issue - since the StopWatch logging is at DEBUG and the plugin uses INFO in a way that is not easy for a user to change (https://github.com/ome/bioformats/blob/develop/components/bio-formats-plugins/src/loci/plugins/LociImporter.java#L69), there shouldn't be any impact there. In QuPath, I am having a hard time getting a test setup that includes this PR's changes, so haven't evaluated directly. As with ImageJ, the default log level is INFO, but this is easy to change. I think worst case would be someone has DEBUG/TRACE turned on and sees a problem, in which case we suggest turning it back to INFO. If setting the StopWatches to TRACE instead of DEBUG would be preferable, I am fine with doing that. |
|
@joshmoore any objection to merging this in the current state based on #4164 (comment) (before you going on the road for a few weeks)? |
|
No objections. Sorry. I didn't mean to hold this up. |
Opening as a draft since this is a work in progress, with additional changes to both reading and writing planned.
4e81795 should reduce the total time to run
bfconvertwith DICOM output. This is just buffering the header writes, and does not change the tile writing. A simple test conversion using https://downloads.openmicroscopy.org/images/Vectra-QPTIFF/perkinelmer/PKI_scans/HandEcompressed_Scan1.qptiff:is noticeably faster with 4e81795 when testing locally.