Integrate shared chunk cache (H5SC) package with upper layers of library #5246
Conversation
fortnern
requested review from
lrknox,
derobins,
byrnHDF,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
| if (dset->shared->layout.sc_ops) { | ||
| H5D_chunk_scc_udata_t udata; | ||
| size_t buf_size = SIZE_MAX; | ||
| assert(0 && "set up the buf size from API when available. Either point directly into args " |
There was a problem hiding this comment.
Was this hardcoded assertion failure supposed to be left in for now?
There was a problem hiding this comment.
Yes. We should be updating the API soon and will be able to get rid of this assertion.
| @@ -2900,6 +2900,40 @@ H5CX_set_mpio_actual_chunk_opt(H5D_mpio_actual_chunk_opt_mode_t mpio_actual_chun | |||
| FUNC_LEAVE_NOAPI_VOID | |||
| } /* end H5CX_set_mpio_actual_chunk_opt() */ | |||
|
|
|||
| /*------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
Added these since this should be more convenient than get/or/set. I think we could possibly use these in a few more places instead of tracking the property in a separate variable.
| @@ -8222,20 +8234,16 @@ H5D__chunk_iter(H5D_t *dset, H5D_chunk_iter_op_t op, void *op_data) | |||
| } /* end H5D__chunk_iter() */ | |||
|
|
|||
| /*------------------------------------------------------------------------- | |||
| * Function: H5D__chunk_get_offset_copy | |||
| * Function: H5D__chunk_verify_offset | |||
There was a problem hiding this comment.
Turns out nothing in the direct chunk code modifes the offset array so there's no need to copy it and we can just pass it directly from the user qualified as const. Added this function to perform verification.
| /* Perform second phase of type info initialization */ | ||
| if (H5D__typeinfo_init_phase2(&io_info) < 0) | ||
| HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to set up type info (second phase)"); | ||
| /* Initialize type info differently depending on if weŕe using the shared chunk cache or not */ |
There was a problem hiding this comment.
The shared chunk cache will need to perform its own type conversion initialization.
| @@ -115,6 +115,13 @@ | |||
| /* Package Private Typedefs */ | |||
| /****************************/ | |||
|
|
|||
| /* Typedef for cached dataset creation property list information */ | |||
| typedef struct H5D_dcpl_cache_t { | |||
There was a problem hiding this comment.
This is not used outside of the H5D package so I demoted it to this header to minimize issues with circular dependencies
| 0) | ||
| HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "can't read unprocessed chunk data"); | ||
| /* Read the raw chunk */ | ||
| if (H5D__chunk_direct_read(dset, chunk_read_args->offset, &chunk_read_args->filters, |
There was a problem hiding this comment.
Checking the validity of the offset is layout specific and has been moved to H5Dchunk.c for the chunked non-SCC case.
| @@ -235,6 +235,9 @@ test_sparse_direct_chunk(hid_t fapl) | |||
|
|
|||
| TESTING("APIs for direct chunk I/O on structured chunks"); | |||
|
|
|||
| SKIPPED(); | |||
There was a problem hiding this comment.
Skipped since struct chunk direct I/O requires a shared chunk cache client implementation, which we don't have yet.
mattjala
added
Merge - Develop Only
Various other fixes/enhancements.
…eature/sparse_data
when it isn't supported. Also replace inappropriate usage of FAIL_STACK_ERROR with TEST_ERROR.
|
@fortnern - Is this ready (enough :-) to review? |
You're welcome to review it if you like. It's nowhere near production ready yet though - just part of the machinery for the shared chunk cache (designed to support sparse chunks), which hasn't been implemented yet. Note the PR is to a feature branch not develop. |
Also some updates to shared chunk cache interface, and other general fixes. Non functional currently since the shared chunk cache hasn't been implemented.