search

bigdataviewer / bigdataviewer-core

ImgLib2-based viewer for registered SPIM stacks and more
BSD 2-Clause "Simplified" License
33 stars 35 forks source link

strong delay when opening h5/xml from network share (introduced in bigdataviewer-core-10.4.6) #165

Closed CellKai closed 12 months ago

CellKai commented 1 year ago

issue description

reproduce the issue

note: I truncated the dataset (original: 1 angle, 3 channels, 1 illumination, 100 tiles, 12 timepoints, ~400 GB, registered but not fused) and included only timepoint 0, the other partitions are missing on purpose so the upload is smaller (38 GB).

crosspost

https://forum.image.sc/t/bigstitcher-n5-api-fused-n5-hdf5-has-no-pyramids-and-does-not-display-well-in-bdv/80925/21

initial debugging by @NicoKiaru

https://forum.image.sc/t/bigstitcher-n5-api-fused-n5-hdf5-has-no-pyramids-and-does-not-display-well-in-bdv/80925/16?u=cellkai

and following posts

additional tests

loading a 400 GB 16 tile h5/xml from a network location using bigdataviewer-core-10.4.6 and above does not have such a strong delay. It seems hence that the delay correlates with the number of tiles.

NicoKiaru commented 1 year ago

@CellKai maybe precise your OS ?

tpietzsch commented 1 year ago

@NicoKiaru @CellKai thanks for debugging!!!

It looks like it could be really caused by the H5Oget_info_by_name call. This was introduced between bigdataviewer-core-10.4.5 and bigdataviewer-core-10.4.6.

This is used here https://github.com/bigdataviewer/bigdataviewer-core/blob/d43472e7c9afed4b0caec9794b6b12ec80b4a895/src/main/java/bdv/img/hdf5/HDF5Access.java#L154-L165 to check for the existence of a dataset. I was expecting this to be better performance than just trying to open the dataset (LOL... Always a good idea to try to improve performance by guessing instead of measuring...)

Here is a thread about H5Oget_info_by_name performance problems: https://www.mail-archive.com/hdf-forum@hdfgroup.org/msg02058.html

So this should be replaced by just trying to opening and closing the dataset. @NicoKiaru Would you try to see if it can be fixed this way? (Otherwise, I'll try to do it next week).

imagesc-bot commented 1 year ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/bigstitcher-n5-api-fused-n5-hdf5-has-no-pyramids-and-does-not-display-well-in-bdv/80925/22

CellKai commented 1 year ago

@CellKai maybe precise your OS ?

certainly! I tested using Fiji-win64 on Windows 10.

NicoKiaru commented 1 year ago

Not sure how to test it, but I commented the additional check...

            if ( dataSet == null ) //&& datasetExists( pathName ) )

... and then it's opening fast.

mkitti commented 1 year ago

It would be better if we could use H5Ovisit3 or H5Literate2.

https://docs.hdfgroup.org/hdf5/develop/group___h5_o.html#ga6d03115ae0e5e5b516bbf35bb492266a

https://docs.hdfgroup.org/hdf5/develop/group___t_r_a_v.html#gad7ca4206f06b5ada85b6ec5867ec6c73

mkitti commented 1 year ago

Note that upstream version of H5Oget_info_by_name is now on version 3:

https://docs.hdfgroup.org/hdf5/develop/group___h5_o.html#gabb69c962999e027cef0079bbb1282199

mkitti commented 1 year ago

Here is the stack trace

The problem is that you have requested H5O_INFO_ALL while you just need H5O_INFO_BASIC.

The quickest fix is to use hdf.hdf5lib.H5.H5Oget_info_by_name(loc_id, name, HDF5Constants.H5O_INFO_BASIC, lapl_id)

https://docs.hdfgroup.org/hdf5/v1_14/classhdf_1_1hdf5lib_1_1_h_d_f5_constants.html#ae0c383b646260260d6f1626c21e738b9

tpietzsch commented 12 months ago

Thanks a lot for the pointers @mkitti! I pushed the fix to master https://github.com/bigdataviewer/bigdataviewer-core/commit/20a70342ef02f0997c6899029dac417bd88ff300

@NicoKiaru Could you try whether this solves the performance? (https://maven.scijava.org/service/local/repositories/snapshots/content/sc/fiji/bigdataviewer-core/10.4.9-SNAPSHOT/bigdataviewer-core-10.4.9-20230803.083612-2.jar)

If it does, then I should apply the fix also for export, and in https://github.com/saalfeldlab/n5-hdf5.

NicoKiaru commented 12 months ago

Yep, all good, it opens fast now.

mkitti commented 12 months ago

This kind of issue make me think we should move to using https://central.sonatype.com/artifact/org.bytedeco/hdf5/1.14.1-1.5.9 sooner rather than later. The JHDF5 API is just making very bad default choices. That package exposes the Java, C, and C++ APIs in Java.

<dependency>
    <groupId>org.bytedeco</groupId>
    <artifactId>hdf5</artifactId>
    <version>1.14.1-1.5.9</version>
</dependency>

I'm working on a JHDF5 compat layer to ease the transition.

StephanPreibisch commented 12 months ago

Awesome :)

tpietzsch commented 12 months ago

I released bigdataviewer-core-10.4.9 with the fixes, and made a PR https://github.com/saalfeldlab/n5-hdf5/pull/31 to fix n5-hdf5