-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance increase in writeIFD #3680
base: develop
Are you sure you want to change the base?
Conversation
out as “random access stream around the given file” exchange by temporary random access stream around a ByteArrayHandle.
From imagesc message: In TiffSaver.java the function writeIFD() loops over IFD keys in
In writeIFDValue various write operations are executed. This operations write values into the RandomAccessOutputStream out and thereby force several bounds checks. This bounds checks seems to be slow and take a significant amount of time because out is a “random access stream around the given file”. If a temporary random access stream around a ByteArrayHandle is used to collect all data before writing it to the file in a single write operation, about 33% of the operation time can be saved. I got the following times for the Run 1: Write OME-TIFF in : 20332 ms Run 1: Write OME-TIFF in : 14764 ms |
Tested by converting the below public datasets, each run 5 times and averaged (time in seconds):
Those runs were proving inconclusive with mixed results, as a follow up I ran the Java performance tests that were originally designed for the profiling of ome-files (https://github.com/ome/ome-files-performance) and the results again was not as expected. NIRHTa-001.ome.tiff - 6.6.1:
NIRHTa-001.ome.tiff - with this PR:
MitoCheck - 6.6.1:
MitoCheck - with this PR:
tubhiswt-4D - 6.6.1:
tubhiswt-4D - with this PR:
|
Pushing this to 6.8.0 to continue testing in order to better understand the impact of the changes across a variety of datasets |
I reran a number of the performance tests again, this time against 6.7.0. The results are again somewhat inconclusive, the PR performs very well against some datasets such as BBBC but is comparable or slightly slower for others. I would still like to dig into this further to understand exactly what situations this PR performs best in. Pushing to 6.9.0 to re investigate in the new year. public/2016-06/tubhiswt-4D With 6.7.0:
With this PR:
public/2016-06/BBBC/NIRHTa-001.ome.tiff With 6.7.0:
With this PR:
public/2016-06/sub-resolutions/Fluorescence/LuCa-7color_Scan1.ome.tiff With 6.7.0:
With this PR:
public/2016-06/MitoCheck/00001_01.ome.tiff With 6.7.0:
With this PR:
public/2016-06/sub-resolutions/Z-stack/retina_large.ome.tiff With 6.7.0:
With this PR:
|
Conflicting PR. Removed from build BIOFORMATS-push#1103. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1104. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#7. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#8. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#9. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#4. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#5. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1120. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#12. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1150. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#239. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#373. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1522. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#374. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#429. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#503. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#2. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#504. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#3. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#505. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#5. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#511. See the console output for more details.
--conflicts |
Converting this PR to draft for now, this PR is something we may well want to return too as part of wider look at improving write performance |
Tested with the commits here, compared with the current state of develop. Note that I see similar results to what @dgault observed above. For things with lots of small IFDs, there is some improvement, but not necessarily enough to be noticeable in terms of overall conversion time. For large images with few IFDs, performance is the same or slightly worse. I tested with https://downloads.openmicroscopy.org/images/Hamamatsu-NDPI/openslide/CMU-1/CMU-1.ndpi and Given the inconclusive testing results, I propose to close this PR but add #3983, #3480, and ome/ome-common-java#78 to the 8.0.0 milestone (so that we prioritize measurable progress on this topic), unless any objections in the next week. |
out as “random access stream around the given file” exchange by temporary random access stream around a ByteArrayHandle.