Don't use slow H5Iget_name #198
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
|
@Ri0n Can you please force-push again to trigger the Travis CI pipeline. Thanks. |
Ri0n
force-pushed
the
build-variable-paths-master
branch
from
37e899d
to
777645f
Compare
1 similar comment
|
Argh, I missed that #198 failed the test suite in 18 tests, see https://app.travis-ci.com/github/tbeu/matio/jobs/593339008#L5197 Unfortunately, this is hidden and not marked as failure by Travis CI. |
|
I needed to revert the changes by 4459088 to make the testsuite passing again, |
|
sure. I had no time to take a look at this myself. but the second patch I provided we currently use in production without any issue. |
tbeu
added a commit
that referenced
this pull request
Oct 23, 2023
* The performance gain is obtained by removing the slow HDF5 API function H5Iget_name being the main bottleneck. Handles of HDF5 groups or datasets are now kept open for the lifetime of the matvar_t instance. * As a side-effect, the hdf5_name could be removed from matvar_t.internal, too. * Fix reference counting in Mat_VarDuplicate * As reported by #65 and #198
seanm
pushed a commit
to seanm/matio
that referenced
this pull request
Jan 31, 2024
See https://forum.hdfgroup.org/t/h5i-get-name-call-is-very-slow-for-hdf5-file-5-gb/8246 Co-authored-by: EXTERNAL Ilinykh Sergey (EPAM, ETAS-DAP/ENG-HS) <Sergey_Ilinykh@epam.com>
seanm
pushed a commit
to seanm/matio
that referenced
this pull request
Jan 31, 2024
This reverts commit 792af49.
seanm
pushed a commit
to seanm/matio
that referenced
this pull request
Jan 31, 2024
* The performance gain is obtained by removing the slow HDF5 API function H5Iget_name being the main bottleneck. Handles of HDF5 groups or datasets are now kept open for the lifetime of the matvar_t instance. * As a side-effect, the hdf5_name could be removed from matvar_t.internal, too. * Fix reference counting in Mat_VarDuplicate * As reported by tbeu#65 and tbeu#198
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.
we had files with approximately same data in mat5 and mat73/hdf5 format.
reading of mat73 file was many times slower than mat5. It turned out Matio calls H5Iget_name way too often, and H5Iget_name is a very resource-expensive function.
See similar issue https://forum.hdfgroup.org/t/h5i-get-name-call-is-very-slow-for-hdf5-file-5-gb/8246