-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improve intermediate file performance #271
Conversation
bbee3ee
to
27e9cca
Compare
More info on hdf5 chunked + uncompressedViewing the nsys profile reports in nsight reveals that the ~70s "wait" before writing some blocks to intermediate data occurs firstly after the first time the paganin filter method is executed (taking ~59s). It then occurs several times in the first block being processed in the section containing FBP (taking ~70s each time). More specifically, the stripe removal method's output being written in that section doesn't cause the wait, but
all only on the first block iteration within that section), see the screenshot below Info that could help understand why this is happeningRegarding my comment about this possibly being related to MPI synchronisations occurring, this was due to seeing function calls in the region where the waiting/blocking occurs mentioning mutexes: Another interesting function call that seems to appear early on in every instance where the waiting/blocking occurs mentions "memcpy" and "unaligned": The name of the region being "vdso" also seems noteworthy. At first there was a suspicion it was referring to "VDS (virtual datasets)", a feature of hdf5. However, this feature in hdf5 isn't being used when writing data in httomo. Given that some of the functions in that region appear to be system calls, the latest speculation is that maybe it's referring to "vDSO" that is part of some binary executables https://www.man7.org/linux/man-pages/man7/vdso.7.html |
Note that `--compress-intermediate` has been configured to imply `--chunk-intermediate`. This is because if compression is applied without also triggering the code to specify the chunk shape to be 1 chunk per projection/sinogram, then `h5py` will guess the chunk shape to write. This is often not as optimal as choosing 1 chunk per projection/sinogram, in terms of compression ratio.
Creation of `Group` objects via `zarr.open()` and `zarr.open_group()` suffers a race condition regarding no synchronisation between processes when attempting to initialise group metadata. Therefore, `Group` creation via `open_group()` has been replaced in favour of: - the rank 0 process being the only one to create any groups (see auxiliary data creation code changes) - passing a `DirectoryStore` object to `zarr.open_array()`, which does not exhibit the same synchronisation issues
27e9cca
to
18a3b60
Compare
A few comments here (without investigating deeper yet):
|
Hi @ptim0626, please see the comment from @team-gpu above, could be helpful. Worth trying to reduce the size of the chunk and see if it fixes the compression-related segfault. Also an interesting point about the driver and this pthread mutex wait. |
Hi @dkazanc, thanks for the info! Actually I just think I found out the reason of segfault with compressed chunk (literally just few hours ago!). I am just doing some reading to make sure I have more understanding about it. In short, if you |
FYI I opened a separate issue for tracking the compressed chunk segmentation fault #389 |
I think I am able to reproduce the wait. Looking at my profile report, I guess the wait relates to the creation of chunks (a lot of I have been trying to turn off this behaviour, so far I have tried:
However, the wait is still there... I will continue to investigate in this direction. I could mix up the hdf5/h5py version or setting it in the incorrect place etc. So useful links: |
So the wait is only happening with chunked writes, and not if chunking isn't used? |
@team-gpu That's what my experience was when I came across the issue: writing hdf5 unchunked (serial or parallel) -> waiting doesn't occur, writing hdf5 chunked in parallel -> waiting occurs |
Ok my bad I set in the wrong place...so
Preliminary profiling results to compare: without setting anything (the default in h5py): We need to find a nicer way to override the default setting, for my own testing purpose I just change it in the source code :) h5py seems to hard code this, even if you provide your own dataset creation property list (dcpl) via @yousefmoazzam @team-gpu the above links should give a much better explanation than me about why it is only on chunks. |
It would be good to know why that line in h5py has the comment "prevent resize glitch" (shame there's no issue number in the code comment or something): if chunks is not None:
plist.set_chunk(chunks)
plist.set_fill_time(h5d.FILL_TIME_ALLOC) # prevent resize glitch Ie, what is this "resize glitch", and in what cases can this "resize glitch" happen. Without knowing anything else apart from:
it makes me wonder if bad things can happen if the value is changed to something else. |
This is a good point to consider. My guess is hdf5 dataset can have a maximum shape specified (something like That's my guess only. EDIT: just git blame it, the line was added 13 years ago! |
Here is a nice description what those flags do: https://docs.hdfgroup.org/hdf5/v1_14/_h5_d__u_g.html#subsubsec_dataset_allocation_store So @ptim0626 understanding makes sense - the fill time settings determines what to do with the elements that have never been written. By default, HDF5 fills the content of a dataset with a fill value (zero for example) as soon as it's created (the allocation flag controls when - incremental would mean as soon as a chunk is created). This makes sure that there is no uninitialised data in the file. Re resizing - only chunked datasets are resizable in HDF5. Otherwise the full dataset's space is allocated as soon as it is created - and filled with the fill value by default. This could be the reason why unchunked HDF5 appears slower. I would guess that it's much faster with the fill time never setting... For the MPI driver I couldn't find docs, so I'm trying an educated guess. Dynamic allocation of chunks can be quite the bottleneck in a parallel mode - imagine 100 processes working on the same file and allocating chunks all over the dataset. Now I don't know how chunk allocation is managed in HDF5, but there would certainly be some synchronisation needed as all processes work on the same file. It could very well be that the MPI driver does some allocation up-front in any case, to avoid too much synchronisation needed during larger writes, or something like that. Re httomo - we never resize the dataset, so that comment @yousefmoazzam doesn't affect us (whatever it means). We are also reasonably sure (by tests) that we write all values in the file eventually. From that, I'd guess that never filling is a good choice. |
It can be controlled on a lower level in the h5py library... https://groups.google.com/g/h5py/c/bivqPsEmg1w/m/QWi6mtfIAwAJ which is also what they are doing in their own tests: https://github.com/h5py/h5py/blob/44ba349e5ecd21ecbc75b1314063e96d30aec872/h5py/tests/test_dataset_getitem.py#L589 |
@team-gpu thanks for all the information! We can use the low-level interface all the way to create the dataset, but we will miss the validation tests etc provided by the high-level Either we go for the 'low-level all the way' solution and ensure sufficient validation is done, or we patch (monkey-patch?) the |
Perhaps raise it as an issue to them and monkey-patch in the meantime? It should be possible to configure it and avoid being ignored. |
Another option may be this:
Then this calls the filter's fill_dcpl function, but because I think this can work without any monkey patching or low-level functions. |
Yes this would work! We will just miss some chunk validations which are much easier to do ourselves. I agree |
I have modified the dataset creation using dcpl following @team-gpu approach. With compression, One surprising consequence is that with Blosc compression is now slower than uncompressed dataset... I have played with some Blosc settings but no success. For the FBP results it is very compressible so the timing is comparable, but others look like not very compressible and those have some delays when saving them. I will continue to investigate. Btw my first commit to httomo so feel free to let me know any rule I should have followed :) |
Thanks for that - that makes a lot of sense. In general, since lossless compression benefits from redundancy in the data, and we have binary floating point data (where all states for the bytes are possible), it makes sense that less processed, less smooth data gets smaller compression ratios. A fully random floating point array would likely get not much more than 1.0 compression ratio. And the further we progress in the pipeline, the further we "smooth" the data, i.e. extract patterns and remove noise. I'm guessing the FBP result has a lot of zeros, or similar values in the neighborhood? For the reconstruction result, it probably even makes sense to apply lossy compression - i.e. to manually convert to 8bit integer data as we do for images and scale the data accordingly. There's that |
Thanks a lot @ptim0626 for this work and @team-gpu for the valuable comments. The FBP compression from ~55 Gib to ~210 MiB is impressive.
Out of curiosity, if the compressed reconstruction is loaded in Dawn, how much time it takes to slice through it? There was a related issue about chunk size should be small, I guess we need to sort that out first before trying? Thanks! |
Sorry @ptim0626 I probably have missed something, but how we ended up from 2 minutes of saving time the result of the reconstruction to few seconds? :) You might have seen this weird behaviour and it also reported by @yousefmoazzam above? |
@dkazanc I actually haven't tried opening any FBP results (I just The chunk cache is for reading, and it depends on the access pattern. I think it makes sense to have a chunk cache at least the size of the uncompressed single chunk so looking at values inside a chunk is quick. I remember the maximum chunk size is ~4GiB, so as long as the chunk isn't too big it shouldn't be a problem (the chunk cache should be adjusted accordingly if the chunk is bigger than 1 MiB) |
Thanks @ptim0626, it looks like this branch has not been updated to |
Hi. Adding my 2 cents for the code itself:
|
thanks @team-gpu . I guess I'll branch out the zarr addition for now as it will require more work for testing etc. And yes, we probably need few more tests here for compression at least. |
How the benchmarking runs were organised
Various runs on the production SLURM cluster at DLS were performed using
and varying the parameters relevant to intermediate data saving
Hardware
The fixed hardware configuration was as follows:
Pipeline
The fixed pipeline was https://github.com/dkazanc/dls_pipelines/blob/main/pipelines/bench_pipeline_gpu_intense_separate_rescale.yaml.
Data
The fixed dataset was "the sandstone data"
119647.nxs
(20GB).Parameters investigated
The parameters that were varied were:
--save-all
flag or notTimes
The speeds of pipeline execution are ordered from fastest to slowest, grouped under the headings of whether the
save-all
flag was used or not.The full time taken for the pipeline to execute was taken from the logfile that httomo creates.
Without
--save-all
With
--save-all
Concluding remarks
--save-all
: 243s (chunked+uncompressed) vs. 353s (unchunked+uncompressed) = 110s speedup (243/353 * 100 = 68.8% of original time, so ~30% decrease in time taken)--save-all
: 794s (chunked+uncompressed) vs. 1148s (unchunked+uncompressed) = 354s speedup (794/1148 * 100 = 69.1% of original time, so ~30% decrease in time taken)