Multi Dataset #2120

fortnern · 2022-09-24T21:07:32Z

The big PR for multi dataset changes. A couple notes:

There is an apparently unrelated bug in parallel compression that is restricting the number of cases for the parallel mdset test. Jordan will investigate this soon.

Resolved notes:

I was able to disable H5D_pre_read/write without breaking it. However, it made the mdset test run substantially slower (shouldn't affect single dataset I/O). Jordan and I are investigating this. If we can't fix it, we will need to decide whether to still remove it and accept the lower performance when using multi dataset in serial. The advantage to still removing these calls is it allows the library to pass the full I/O to selection/vectore enabled VFDs even in serial. Resolved - type conversion buffers were exceeding the free list limits. We decided this issue is really specific to the test and isn't a reason to prevent full use of the multi dataset path, and removed H5D__pre_read/write. I will try out a performance fix for this.
There is an unrelated bug in dataspace code that I am investigating that is causing a restriction on the serial mdset test. I am working on this. Resolved - bug fixed and test re-enabled.

No testing yet.

Untested, probably does not work yet.

SWMR tests.

Sync with fork

multi dataset.

of the parameter struct.

Update to new multi dataset API

* Update to new multi dataset Fortran API and tests. * Sync Fortran with develop. * skipping h5pget_mpio_actual_io_mode_f for now

jhendersonHDF · 2022-10-14T04:17:23Z

testpar/t_pmulti_dset.c

@@ -0,0 +1,767 @@
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Copyright by The HDF Group.                                               *
+ * Copyright by the Board of Trustees of the University of Illinois.         *


If this is new code, we probably want to avoid the University of Illinois portion here. Otherwise, it's unclear whether/how that portion applies.

src/H5Defl.c

src/H5Dcontig.c

src/H5Dio.c

jhendersonHDF · 2022-10-17T03:43:34Z

src/H5VLcallback.c

@@ -2060,21 +2110,44 @@ H5VL__dataset_read(void *obj, const H5VL_class_t *cls, hid_t mem_type_id, hid_t
 *-------------------------------------------------------------------------
 */
 herr_t
-H5VL_dataset_read(const H5VL_object_t *vol_obj, hid_t mem_type_id, hid_t mem_space_id, hid_t file_space_id,
-                  hid_t dxpl_id, void *buf, void **req)
+H5VL_dataset_read(size_t count, const H5VL_object_t *vol_obj[], hid_t mem_type_id[], hid_t mem_space_id[],


It looks like there's only one place remaining in the feature branch where H5VL_dataset_read is used and it could probably be converted to the form that the new H5VL_dataset_read_direct is expecting. It also looks like H5VL_dataset_write is no longer used, unless I missed a spot while searching. Based on that, does it seem worth it to just modify H5VL_dataset_read and H5VL_dataset_write rather than having two different internal H5VL I/O calls for read and write?

I left those in for consistency with how other the other VOL callbacks work. We could maybe eliminate one of each function but I don't think the remaining functions should use the same naming convention as the other single underscore H5VL callback functions.

Sure. I don't think this needs to hold up a merge but it's something to think about to lower some of the maintenance overhead.

src/H5Dchunk.c

jhendersonHDF · 2022-10-17T04:19:02Z

src/H5Dchunk.c

            HGOTO_ERROR(H5E_DATASPACE, H5E_CANTINSERT, FAIL, "can't insert chunk into skip list")
        } /* end if */

+        /* get chunk file address */
+        if (H5D__chunk_lookup(di->dset, new_piece_info->scaled, &udata) < 0)


I'm guessing these chunk lookups have been moved from somewhere else? Just checking that we aren't adding extra metadata overhead.

Just spent a few hours trying to eliminate this. I think we're going to have to accept that a small amount of memory will be temporarily wasted for unallocated chunks (1 pointer per chunk) and skip this lookup here. Caching the result doesn't work because the chunk can be evicted between io_init and read/write. This was in here for the previous algorithm which used a global skip list sorted by address for all the pieces in the I/O. Addresses are no longer needed here since this is now an unsorted array. We'll have to add this in for the parallel code path to eventually get the file address.

This call (and similar ones throughout the io_init functions) has been eliminted by commit 37cdba4

Address other comments from code review.

Address comments from code review

jhendersonHDF

I believe I've reviewed this PR as thoroughly as I can. I still have a couple of lower order concerns (the conversations left unresolved), but I don't think they're anything that should hold up a merge.

overhead to single dataset I/O.

Delay chunk index lookup from io_init to mdio_init

Sync with canonical

Sync with develop

src/H5Dio.c

Fix inappropriate use of piece_count

* Fix bug with cross platform compatibility of references within vlens. No testing yet. * Merge from multi_rd_wd_coll_io to a more recent branch from develop. Untested, probably does not work yet. * Committing clang-format changes * Committing clang-format changes * Fix many bugs in multi dataset branch. Mostly works, some issues in SWMR tests. * Committing clang-format changes * Disable test in swmr.c that was failing due to bug in HDF5 unrelated to multi dataset. * Committing clang-format changes * Fixed fortran multi-dataset tests * Fixed xlf errors * Added benchmark code for multi-datasets * loops over datasets * added missing error arg. * Added gnuplot formatting * Jonathan Kim original MD benchmarking code * updated MD benchmarking code * code clean-up * Only make files in feature test mode * misc clean-up * removed TEST_MDSET_NO_LAST_DSET_2ND_PROC option * Committing clang-format changes * Change multi dataset API to use arrays of individual parameters instead of the parameter struct. * Committing clang-format changes * Update to new multi dataset Fortran API and tests. (HDFGroup#1724) * Update to new multi dataset Fortran API and tests. * Sync Fortran with develop. * skipping h5pget_mpio_actual_io_mode_f for now * Fixed issue with dxpl_id, changed to variable size dim. (HDFGroup#1770) * Remove "is_coll_broken" field from H5D_io_info_t struct * Committing clang-format changes * Minor cleanup in multi dataset code. * Committing clang-format changes * Clean up in multi dataset code. * Committing clang-format changes * Committing clang-format changes * Fix speeling * Fix bug in parallel compression. Switch base_maddr in io_info to be a union. * Committing clang-format changes * Implement selection I/O support with multi dataset. Will be broken in parallel until PR 1803 is merged to develop then the MDS branch. * Committing clang-format changes * Spelling * Fix bug in multi dataset that could cause errors when only some of the datasets in the multi dataset I/O used type conversion. * Committing clang-format changes * Integrate multi dataset APIs with VOL layer. Add async versions of multi dataset APIs. * Committing clang-format changes * Spelling fixes * Fix bug in non-parallel HDF5 compilation. * Committing clang-format changes * Fix potential memory/free list error. Minor performance fix. Other minor changes. * Committing clang-format changes * Fix memory leak with memory dataspace for I/O. * Committing clang-format changes * Fix stack variables too large. Rename H5D_dset_info_t to H5D_dset_io_info_t. * Committing clang-format changes * Remove mem_space_alloc field from H5D_dset_io_info_t. Each function is now responsible for freeing any spaces it adds to dset_info. * Committing clang-format changes * fixed _multi Fortran declaration * Refactor various things in (mostly) the serial I/O code path to make things more maintainable. * Committing clang-format changes * updated to array based, doxygen, and examples * Reinstate H5D_chunk_map_t, stored (via pointer) inside H5D_dset_io_info_t. * Change from calloc to malloc for H5D_dset_io_info_t and H5D_chunk_map_t. Switch temporary dset_infos to be local stack variables. * Committing clang-format changes * format cleanup * format cleanup * added coll and ind * Modify all parallel I/O paths to take dset_info instead of assuming dset_info[0]. * Committing clang-format changes * fixed output * Rework parallel I/O code to work properly with multi dataset in more cases. Fix bug in parallel compression. * Committing clang-format changes * Prevent H5D__multi_chunk_collective_io() from messing up collective opt property for other datasets in I/O. Other minor cleanup. Add new test case to t_pmulti_dset.c for H5FD_MPIO_INDIVIDUAL_IO, disabled for now due to failures apparently unrelated to multi dataset code. * Fix spelling * Committing clang-format changes * Replace N log N algorithm for finding chunk in H5D__multi_chunk_collective_io() with O(N) algorithm, and remove use of io_info->sel_pieces in that function. * Committing clang-format changes * Replace sel_pieces skiplist in io_info with flat array of pointers, use qsort in I/O routine only when necessary. * Committing clang-format changes * Add new test case to mdset.c * Committing clang-format changes * Fix spelling * Very minor fix in H5VL__native_dataset_read() * Fix bug that could affect filtered parallel multi-dataset I/O. * Add RM entries for H5Dread_multi(), H5Dread_multi_async(), H5Dwrite_multi(), and H5Dwrite_multi_async() * Unskip test in swmr.c * Committing clang-format changes * Eliminate H5D__pre_read and H5D__pre_write * Remove examples/ph5mdsettest.c. Will fix and re-add as a test. * Enable hyperslab combinations in mdset test * Committing clang-format changes * Clarify H5Dread/write_multi documentation. * Fix bugs in multi-dataset I/O. Expand serial multi dataset test. Update macro in parallel multi dataset test. * Committing clang-format changes * Spelling * Remove obsolete entry in bin/trace * Rework type conversion buffer allocation. Only one buffer is shared between datasets in mdset mode, and it is malloced instead of calloced. * Committing clang-format changes * Fix bug in error handling in H5D__read/write * added multi-dataset fortran check with optional dataset creation id (HDFGroup#2150) * removed dup. dll entry * Address comments from code review. * Remove spurious changes in H5Fmpi.c * Fix issue with reading unallocated datasets in multi-dataset mode. Address other comments from code review. * Committing clang-format changes * Delay chunk index lookup from io_init to mdio_init so it doesn't add overhead to single dataset I/O. * Committing clang-format changes * Fix inappropriate use of piece_count * updated copyright on new file, removed benchmark from testing dir. Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: M. Scot Breitenfeld <brtnfld@hdfgroup.org> Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>

fortnern and others added 30 commits September 30, 2021 16:11

Fix bug with cross platform compatibility of references within vlens.

e18f0cf

No testing yet.

Merge branch 'hdf5_canon-develop' into develop

d958fa5

Merge branch 'HDFGroup:develop' into develop

e754c2a

Merge from multi_rd_wd_coll_io to a more recent branch from develop.

f6eb89f

Untested, probably does not work yet.

Committing clang-format changes

6f2d93d

Committing clang-format changes

9a9e2c0

Fix many bugs in multi dataset branch. Mostly works, some issues in

57b0119

SWMR tests.

Committing clang-format changes

fd071e2

Merge pull request #1694 from fortnern/mds_merge

d8edb5e

Sync with fork

Disable test in swmr.c that was failing due to bug in HDF5 unrelated to

ce873af

multi dataset.

Committing clang-format changes

a5b3cd8

Fixed fortran multi-dataset tests

7e80c7a

Fixed xlf errors

d52fad1

Added benchmark code for multi-datasets

9d3f386

loops over datasets

c416a94

added missing error arg.

9054556

Added gnuplot formatting

2f0078b

Jonathan Kim original MD benchmarking code

fa3e780

updated MD benchmarking code

7d03d03

code clean-up

670723c

Only make files in feature test mode

80f4167

misc clean-up

2c3e7c0

removed TEST_MDSET_NO_LAST_DSET_2ND_PROC option

63a4448

Committing clang-format changes

8b3ac0b

Change multi dataset API to use arrays of individual parameters instead

41dffc5

of the parameter struct.

Merge branch 'mds_merge' of github.com:fortnern/hdf5 into mds_merge

d0eb62d

Committing clang-format changes

b39a123

Merge pull request #1710 from fortnern/mds_merge

8298fa6

Update to new multi dataset API

Update to new multi dataset Fortran API and tests. (#1724)

249bcf4

* Update to new multi dataset Fortran API and tests. * Sync Fortran with develop. * skipping h5pget_mpio_actual_io_mode_f for now

Fixed issue with dxpl_id, changed to variable size dim. (#1770)

1671635

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Defl.c Outdated Show resolved Hide resolved

Merge branch 'develop' into feature/multi_dataset

c7d33e6

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Dcontig.c Show resolved Hide resolved

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

jhendersonHDF reviewed Oct 14, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

jhendersonHDF reviewed Oct 17, 2022

View reviewed changes

src/H5Dchunk.c Show resolved Hide resolved

jhendersonHDF reviewed Oct 17, 2022

View reviewed changes

fortnern and others added 3 commits October 17, 2022 14:13

Fix issue with reading unallocated datasets in multi-dataset mode.

7fcc27e

Address other comments from code review.

Committing clang-format changes

dbab411

Merge pull request #2168 from fortnern/mds_merge

2cec94a

Address comments from code review

jhendersonHDF approved these changes Oct 18, 2022

View reviewed changes

fortnern and others added 6 commits October 18, 2022 13:05

Delay chunk index lookup from io_init to mdio_init so it doesn't add

37cdba4

overhead to single dataset I/O.

Committing clang-format changes

a4b5cc0

Merge pull request #2170 from fortnern/mds_merge

eddff72

Delay chunk index lookup from io_init to mdio_init

Merge pull request #14 from HDFGroup/feature/multi_dataset

3181632

Sync with canonical

Merge branch 'develop' into mds_merge

76d6b3f

Merge pull request #2172 from fortnern/mds_merge

c511fa5

Sync with develop

jhendersonHDF reviewed Oct 18, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

jhendersonHDF reviewed Oct 18, 2022

View reviewed changes

src/H5Dio.c Outdated Show resolved Hide resolved

fortnern and others added 3 commits October 18, 2022 16:23

Fix inappropriate use of piece_count

17a41f1

Merge pull request #2174 from fortnern/mds_merge

f0bc2c2

Fix inappropriate use of piece_count

updated copyright on new file, removed benchmark from testing dir.

1566e48

derobins approved these changes Oct 19, 2022

View reviewed changes

derobins merged commit 93754ca into develop Oct 19, 2022

derobins deleted the feature/multi_dataset branch April 18, 2023 16:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi Dataset #2120

Multi Dataset #2120

fortnern commented Sep 24, 2022 •

edited

Loading

jhendersonHDF Oct 14, 2022

jhendersonHDF Oct 17, 2022 •

edited

Loading

fortnern Oct 17, 2022

jhendersonHDF Oct 17, 2022

jhendersonHDF Oct 17, 2022

fortnern Oct 17, 2022

fortnern Oct 18, 2022

jhendersonHDF left a comment

Multi Dataset #2120

Multi Dataset #2120

Conversation

fortnern commented Sep 24, 2022 • edited Loading

jhendersonHDF Oct 14, 2022

Choose a reason for hiding this comment

jhendersonHDF Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

fortnern Oct 17, 2022

Choose a reason for hiding this comment

jhendersonHDF Oct 17, 2022

Choose a reason for hiding this comment

jhendersonHDF Oct 17, 2022

Choose a reason for hiding this comment

fortnern Oct 17, 2022

Choose a reason for hiding this comment

fortnern Oct 18, 2022

Choose a reason for hiding this comment

jhendersonHDF left a comment

Choose a reason for hiding this comment

fortnern commented Sep 24, 2022 •

edited

Loading

jhendersonHDF Oct 17, 2022 •

edited

Loading