From b8f50f1dc70fd366799323c72672b8bb368f28da Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Fri, 29 Mar 2024 07:43:39 -0700 Subject: [PATCH] Relaxed behavior of H5Pset_page_buffer_size() when opening files This API call sets the size of a file's page buffer cache. This call was extremely strict about matching its parameters to the file strategy and page size used to create the file, requiring a separate open of the file to obtain these parameters. These requirements have been relaxed when using the fapl to open a previously-created file: * When opening a file that does not use the H5F_FSPACE_STRATEGY_PAGE strategy, the setting is ignored and the file will be opened, but without a page buffer cache. This was previously an error. * When opening a file that has a page size larger than the desired page buffer cache size, the page buffer cache size will be increased to the file's page size. This was previously an error. The behavior when creating a file using H5Pset_page_buffer_size() is unchanged. Fixes GitHub issue #3382 --- release_docs/RELEASE.txt | 23 ++++++++ src/H5Fint.c | 16 +++++- src/H5Ppublic.h | 11 +++- test/page_buffer.c | 113 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 159 insertions(+), 4 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index d96e2f2a6f5..9fd4be30ed1 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -368,6 +368,29 @@ New Features Library: -------- + - Relaxed behavior of H5Pset_page_buffer_size() when opening files + + This API call sets the size of a file's page buffer cache. This call + was extremely strict about matching its parameters to the file strategy + and page size used to create the file, requiring a separate open of the + file to obtain these parameters. + + These requirements have been relaxed when using the fapl to open + a previously-created file: + + * When opening a file that does not use the H5F_FSPACE_STRATEGY_PAGE + strategy, the setting is ignored and the file will be opened, but + without a page buffer cache. This was previously an error. + + * When opening a file that has a page size larger than the desired + page buffer cache size, the page buffer cache size will be increased + to the file's page size. This was previously an error. + + The behavior when creating a file using H5Pset_page_buffer_size() is + unchanged. + + Fixes GitHub issue #3382 + - Added support for _Float16 16-bit half-precision floating-point datatype Support for the _Float16 C datatype has been added on platforms where: diff --git a/src/H5Fint.c b/src/H5Fint.c index 3f5a1379834..45ded824a7e 100644 --- a/src/H5Fint.c +++ b/src/H5Fint.c @@ -2073,7 +2073,21 @@ H5F_open(const char *name, unsigned flags, hid_t fcpl_id, hid_t fapl_id) if (H5F__super_read(file, a_plist, true) < 0) HGOTO_ERROR(H5E_FILE, H5E_READERROR, NULL, "unable to read superblock"); - /* Create the page buffer before initializing the superblock */ + /* Skip trying to create a page buffer if the file space strategy + * stored in the superblock isn't paged. + */ + if (shared->fs_strategy != H5F_FSPACE_STRATEGY_PAGE) + page_buf_size = 0; + + /* If the page buffer is enabled, the strategy is paged, and the size in + * the fapl is smaller than the file's page size, bump the page buffer + * size up to the file's page size. + */ + if (page_buf_size > 0 && shared->fs_strategy == H5F_FSPACE_STRATEGY_PAGE && + shared->fs_page_size > page_buf_size) + page_buf_size = shared->fs_page_size; + + /* Create the page buffer *after* reading the superblock */ if (page_buf_size) if (H5PB_create(shared, page_buf_size, page_buf_min_meta_perc, page_buf_min_raw_perc) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTINIT, NULL, "unable to create page buffer"); diff --git a/src/H5Ppublic.h b/src/H5Ppublic.h index 804e7880280..2eb03c5a160 100644 --- a/src/H5Ppublic.h +++ b/src/H5Ppublic.h @@ -5748,7 +5748,16 @@ H5_DLL herr_t H5Pset_mdc_image_config(hid_t plist_id, H5AC_cache_image_config_t * If a non-zero page buffer size is set, and the file space strategy * is not set to paged or the page size for the file space strategy is * larger than the page buffer size, the subsequent call to H5Fcreate() - * or H5Fopen() using the \p plist_id will fail. + * using the \p plist_id will fail. + * + * \note As of HDF5 1.14.4, this property will be ignored when an existing + * file is being opened and the file space strategy stored in the + * file isn't paged. This was previously a failure. + * + * \note As of HDF5 1.14.4, if a file with a paged file space strategy is + * opened with a page size that is smaller than the file's page size, + * the page cache size will be rounded up to the file's page size. + * This was previously a failure. * * The function also allows setting the minimum percentage of pages for * metadata and raw data to prevent a certain type of data to evict hot diff --git a/test/page_buffer.c b/test/page_buffer.c index 8c977fedf44..05fa148a152 100644 --- a/test/page_buffer.c +++ b/test/page_buffer.c @@ -1650,6 +1650,114 @@ test_min_threshold(hid_t orig_fapl, const char *driver_name) } /* test_min_threshold */ +/*------------------------------------------------------------------------- + * Function: test_pb_fapl_tolerance_at_open() + * + * Purpose: Tests if the library tolerates setting fapl page buffer + * values via H5Pset_page_buffer_size() when opening a file + * that does not use page buffering or has a size smaller + * than the file's page size. + * + * As of HDF5 1.14.4, these should succeed. + * + * Return: 0 if test is successful + * 1 if test fails + * + *------------------------------------------------------------------------- + */ +static unsigned +test_pb_fapl_tolerance_at_open(void) +{ + const char *filename = "pb_fapl_tolerance.h5"; + hid_t fapl = H5I_INVALID_HID; + hid_t fcpl = H5I_INVALID_HID; + hid_t fid = H5I_INVALID_HID; + H5F_t *f = NULL; + + TESTING("if opening non-page-buffered files works w/ H5Pset_page_buffer_size()"); + + /* Create a file WITHOUT page buffering */ + if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Fclose(fid) < 0) + TEST_ERROR; + + /* Set up page buffering values on a fapl */ + if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) + TEST_ERROR; + if (H5Pset_page_buffer_size(fapl, 512, 0, 0) < 0) + TEST_ERROR; + + /* Attempt to open non-page-buf file w/ page buf fapl. Should succeed, + * but without a page buffer. + */ + if ((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0) + TEST_ERROR; + if (NULL == (f = (H5F_t *)H5VL_object(fid))) + TEST_ERROR; + if (f->shared->fs_strategy == H5F_FSPACE_STRATEGY_PAGE) + TEST_ERROR; + if (f->shared->page_buf != NULL) + TEST_ERROR; + if (H5Fclose(fid) < 0) + TEST_ERROR; + + /* Set up a fcpl with a page size that is larger than the fapl size */ + if ((fcpl = H5Pcreate(H5P_FILE_CREATE)) < 0) + TEST_ERROR; + if (H5Pset_file_space_strategy(fcpl, H5F_FSPACE_STRATEGY_PAGE, false, 1) < 0) + TEST_ERROR; + if (H5Pset_file_space_page_size(fcpl, 4096) < 0) + TEST_ERROR; + + /* Create a file that uses page buffering with a larger page size */ + if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, fcpl, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Fclose(fid) < 0) + TEST_ERROR; + + /* Attempt to open page-buf file w/ fapl page buf size that is too small. + * Should succeed with a page buffer size that matches the file's page size. + */ + if ((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0) + TEST_ERROR; + if (NULL == (f = (H5F_t *)H5VL_object(fid))) + TEST_ERROR; + if (f->shared->fs_strategy != H5F_FSPACE_STRATEGY_PAGE) + TEST_ERROR; + if (f->shared->page_buf == NULL) + TEST_ERROR; + if (f->shared->fs_page_size != 4096) + TEST_ERROR; + if (H5Fclose(fid) < 0) + TEST_ERROR; + + /* Shut down */ + if (H5Pclose(fcpl) < 0) + TEST_ERROR; + if (H5Pclose(fapl) < 0) + TEST_ERROR; + + HDremove(filename); + + PASSED(); + + return 0; + +error: + + H5E_BEGIN_TRY + { + H5Pclose(fapl); + H5Pclose(fcpl); + H5Fclose(fid); + } + H5E_END_TRY + + return 1; + +} /* test_pb_fapl_tolerance_at_open */ + /*------------------------------------------------------------------------- * Function: test_stats_collection() * @@ -2083,12 +2191,12 @@ main(void) SKIPPED(); puts("Skip page buffering test because paged aggregation is disabled for multi/split drivers"); exit(EXIT_SUCCESS); - } /* end if */ + } if ((fapl = h5_fileaccess()) < 0) { nerrors++; PUTS_ERROR("Can't get VFD-dependent fapl"); - } /* end if */ + } /* Push API context */ if (H5CX_push() < 0) @@ -2107,6 +2215,7 @@ main(void) nerrors += test_lru_processing(fapl, driver_name); nerrors += test_min_threshold(fapl, driver_name); nerrors += test_stats_collection(fapl, driver_name); + nerrors += test_pb_fapl_tolerance_at_open(); #endif /* H5_HAVE_PARALLEL */