Support H5Dset_extent #49
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattjala
commented
Aug 4, 2023
test/test_rest_vol.c
Outdated
| @@ -1473,8 +1473,6 @@ test_unused_file_API_calls(void) | |||
|
|
|||
| if (H5Fflush(file_id, H5F_SCOPE_GLOBAL) >= 0) | |||
| TEST_ERROR | |||
| if (H5Fget_obj_count(file_id, H5F_OBJ_DATASET) >= 0) | |||
There was a problem hiding this comment.
This is supported now, so it's no longer expected to fail. Actual tests for it are in h5vl_test.
mattjala
force-pushed
the
set_extent_get_object_count
branch
from
f41abd2
to
054aa91
Compare
mattjala
added a commit
to mattjala/vol-rest
that referenced
this pull request
Sep 25, 2023
This implementation doesn't use RV_type_info_array_g, unlike the proposed version in HDFGroup#49. H5Fget_obj_count, when file_id = H5F_OBJ_ALL, erroneously counts copied transient datatypes as open objects. This is especially noticeable since the VOL copies the datatype whenever it creates an attribute or a dataset. This is an inherited issue from the HDF5 library itself and needs to be fixed there (see HDFGroup/hdf5#3316).
mattjala
added a commit
to mattjala/vol-rest
that referenced
this pull request
Sep 25, 2023
This implementation doesn't use RV_type_info_array_g, unlike the proposed version in HDFGroup#49. H5Fget_obj_count, when file_id = H5F_OBJ_ALL, erroneously counts copied transient datatypes as open objects. This is especially noticeable since the VOL copies the datatype whenever it creates an attribute or a dataset. This is an inherited issue from the HDF5 library itself and needs to be fixed there (see HDFGroup/hdf5#3316).
mattjala
added a commit
to mattjala/vol-rest
that referenced
this pull request
Sep 28, 2023
This implementation doesn't use RV_type_info_array_g, unlike the proposed version in HDFGroup#49. H5Fget_obj_count, when file_id = H5F_OBJ_ALL, erroneously counts copied transient datatypes as open objects. This is especially noticeable since the VOL copies the datatype whenever it creates an attribute or a dataset. This is an inherited issue from the HDF5 library itself and needs to be fixed there (see HDFGroup/hdf5#3316).
mattjala
force-pushed
the
set_extent_get_object_count
branch
from
054aa91
to
a680f50
Compare
mattjala
changed the title
mattjala
force-pushed
the
set_extent_get_object_count
branch
from
3e6dc5f
to
677bc83
Compare
mattjala
force-pushed
the
set_extent_get_object_count
branch
from
0a86067
to
ec8ec75
Compare
| /* If this is another view of an already-opened dataset, make them share the same dataspace | ||
| * so that changes to it (e.g. resizes) are visible to both views */ | ||
| while (rv_hash_table_iter_has_more(&iterator)) { | ||
| other_dataset = (RV_object_t *)rv_hash_table_iter_next(&iterator); |
There was a problem hiding this comment.
This is fine for now functionality wise, but we need a better solution for this rather than iterating through the hash table on each dataset open. I'd suggest refactoring this whole PR later where rather than inserting each object pointer into a hash table for the ID type, we hash the object's URI and insert that hashed value into the hash table for the ID type. Then, on each dataset (of other object) open, you only need to hash the URI here and do a single lookup to see if an object with this URI is already open.
| if (CURLE_OK != curl_easy_setopt(curl, CURLOPT_URL, request_url)) | ||
| FUNC_GOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set cURL request URL: %s", curl_err_buf); | ||
|
|
||
| /* First, make a GET request for the dataset's dataspace to do some checks */ |
There was a problem hiding this comment.
I'd assume we should already have this information locally as a result of the create/open request? Is there a case where we need updated information from the server?
There was a problem hiding this comment.
This call (and the later once to get DCPL) were redundant - removed in #99
jhendersonHDF
approved these changes
Nov 9, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to keep track of whether two open datasets reference the same dataset, and should share information about their dataspace, I introduced
RV_type_info_array_g, a global array keeping track of opened objects in the REST VOL, similar to the library'sH5I_type_info_array_g.Addresses #44.