Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up ros3 VFD stats code #4579

Merged
merged 10 commits into from
Jun 18, 2024
Merged

Clean up ros3 VFD stats code #4579

merged 10 commits into from
Jun 18, 2024

Conversation

derobins
Copy link
Member

  • Removes printf debugging
  • Simplifies and centralizes stats code
  • Other misc tidying

derobins added 10 commits June 17, 2024 12:01
@derobins
Remove ROS3_DEBUG printf debugging scheme
3eb9bc1
@derobins
Clean up ros3 VFD stats
ce4b13d 
* Move ROS3_STATS code around
* Add _g to ros3_stats_boundaries global variable
* Add H5FD__ prefix to stats functions to avoid FUNC_ENTER errors
@derobins
Switch #if ROS3_STATS to #ifdef ROS3_STATS
7b6f585
@derobins
ros3_statsbin --> H5FD_ros3_stats_bin_t
5339a01
@derobins
Format source
c269904
@derobins
Switch ros3 stat type to uint64_t
476239c
@derobins
Removed ROS3_STATS_POW()
8ae8b98 
Replaced the over-engineered macro with a simple bit shift
@derobins
Removed ROS3_STATS_STARTING_MIN
0347aec 
0 is fine...
@derobins
H5FD__ros3_fprint_stats --> H5FD__ros3_print_stats
eedf405 
Also tidied some comments
@derobins
Move stats functions to a common area
350faf7 
Moves the stats to a common ifdef block and adds an
H5FD__ros3_log_read_stats() helper function.
@derobins derobins added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - C Library Core C library issues (usually in the src directory) Type - Improvement Improvements that don't add a new feature or functionality labels Jun 18, 2024
derobins
derobins commented Jun 18, 2024
View reviewed changes
src/H5FDros3.c
unsigned suffix_i = 0;
const char suffixes[] = {' ', 'K', 'M', 'G', 'T', 'P'};
H5FD_ros3_t *file = (H5FD_ros3_t *)_file;
herr_t ret_value = SUCCEED;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, GitHub is very confused about what changes were made. print_stats() was moved to the end of the file and git is now interspersing nearby functions in its diff due to the same-y boilerplate.

@derobins
Copy link
Member Author

The actual changes are smaller than appears, due to git being confused. You can look at the individual commits to see what I was doing.

FWIW, the stats code didn't even compile when I started working on it. I'll look into adding it to developer mode and/or CI in a future PR.

@derobins derobins merged commit 49e3253 into HDFGroup:develop Jun 18, 2024
56 checks passed
byrnHDF pushed a commit to byrnHDF/hdf5 that referenced this pull request Jun 26, 2024
@derobins @byrnHDF
Clean up ros3 VFD stats code (HDFGroup#4579)
482d1bb 
* Removes printf debugging
* Simplifies and centralizes stats code
* Use #ifdef ROS3_STATS instead of #if
* Other misc tidying
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Jul 2, 2024
@derobins @lrknox
Clean up ros3 VFD stats code (HDFGroup#4579)
d73dcf7 
* Removes printf debugging
* Simplifies and centralizes stats code
* Use #ifdef ROS3_STATS instead of #if
* Other misc tidying
lrknox added a commit that referenced this pull request Jul 3, 2024
@lrknox
Sync develop changes June 6 - July 2 to hdf5_1_14 (#4623)
95fa8f1 
* Fix typos in context/property documentation (#4550)

* Fix CI markdown link check http 500 errors (#4556)

Sites like GitLab can have internal problems that return http 500
errors while they fix their problems. Some sites also return http
200 OK, which is fine.

This PR adds a config file to the markdown link check so those
are considered "passing" and don't break the CI.

* Simplify property copying between lists internally (#4551)

* Add Python examples (#4546)

These examples are referred to from the replacement page of https://portal.hdfgroup.org/display/HDF5/Other+Examples.

* Correct property cb signatures in docs (#4554)

* Correct property cb signatures in docs
* Correct delete callback type name in docs
* add missing word to H5P__free_prop doc

* Move C++ and Fortran and examples to HDF5Examples folder (#4552)

* Document 'return-and-read' field in API context (#4560)

* Add compression includes to tests needing zlib support (#4561)

* Allow usage of page buffering for serial file access from parallel HDF5 builds (#4568)

* Remove old version of libaec (#4567)

* Add property names to context field docs (#4563)

* Document property shared name behavior (#4565)

* Clarify H5CX macro documentation (#4569)

* Document H5Punregister modifying default properties (#4570)

* Update NVHPC to 24.5 (#4171)

We don't test parallel in other GitHub actions, so this also converts the
NVHPC check to configure and build only while we discuss how we'll
test parallel HDF5 in GitHub.

There is a blocking GitHub issue to address the test failures for
HDF5 1.14.5 (#4571).

* Clean up comments in H5FDros3.c (#4572)

* Rename INSTALL_Auto.txt to INSTALL_Autotools.txt (#4575)

* Clean up ros3 VFD stats code (#4579)

* Removes printf debugging
* Simplifies and centralizes stats code
* Use #ifdef ROS3_STATS instead of #if
* Other misc tidying

* Turn off ros3 VFD stat collection by default (#4581)

Not a new change - an artifact from a previous check-in.

* Pause recording errors instead of clearing the error stack (#4475)

An internal capability that's similar to the H5E_BEGIN_TRY / H5E_END_TRY
macros in H5Epublic.h, but more efficient since we can avoid pushing errors on
the stack entirely (and those macros use public API routines).

This capability (and other techniques) can be used to remove use of
H5E_clear_stack() and H5E_BEGIN_TRY / H5E_END_TRY within library routines.

We want to remove H5E_clear_stack() because it can trigger calls to the H5I
interface from within the H5E code, which creates a great deal of complexity
for threadsafe code.  And we want to remove H5E_BEGIN_TRY / H5E_END_TRY's
because they make public API calls from within the library code.

Also some other minor tidying in routines related to removing the use of
H5E_clear_stack() and H5E_BEGIN_TRY / H5E_END_TRY from H5Fint.c

* Add page buffer cache command line option to tools (#4562)


Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Clarify documentation for H5CX_get_data_transform (#4580)

* Correct comment for H5CX_get_data_transform

* Document why data transform ctx field doesnt use macro

* Remove public API call from ros3 VFD (#4583)

* Remove printf debugging from H5FDs3comms.c (#4584)

* Cleanup of ros3 test (#4587)

* Removed JS* macro scheme (replaced w/ h5test.h macros)
* Moved curl setup/teardown to main()
* A lot of cleanup and simplification

* Removed unused code from H5FDs3comms.c (#4588)

* H5FD_s3comms_nlowercase()
* H5FD_s3comms_trim()
* H5FD_s3comms_uriencode()

* Remove magic fields from s3comms structs (#4589)

* Remove dead H5FD_s3comms_percent_encode_char() (#4591)

* Rework the TestExpress usage and refactor dead code (#4590)

* Skip examples if running sanitizers (#4592)

* Clean up s3comms test code (#4594)

* Remove JS* macros
* Remove dead code
* Bring in line with other test code

* Add publish to bucket workflow (#4566)

* Update abi report CI workflow for last release (#4596)

* Update abi report workflow to handle 1.14.4.3 release

* Update name of java report

* Document that ctx VOL property isn't drawn from the FAPL (#4597)

* Update macos workflow to 14 (keep 13 as alternate) (#4603)

* Removed unnecessary call to H5E_clear_stack (#4607)

H5FO_opened and H5SL_search don't push errors on the stack

* Bring subfiling VFD code closer to typical library code (#4595)

Remove API calls, use FUNC_ENTER/LEAVE macros, use the library's error macros,
rename functions to have more standardized names, etc.

* Correct documentation for return-and-read fields (#4598)

* These two generators create strings without NUL for testing (#4608)

* Fix Fortran pkconfig to indicate full path of modules (#4593)

* Updated release schedule (#4615)

1.16 and 2.0 information

* Document VOL object wrapping context (#4611)

* Earray.c and farray.c in hdf5_1_14 still need time_t curr_time for HDsrandom.

* Remove line to use future 116_API from CMakeListat.txt files in HDF5
examples directories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byrnHDF byrnHDF byrnHDF approved these changes

@qkoziol qkoziol qkoziol approved these changes

@lrknox lrknox Awaiting requested review from lrknox lrknox is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@jhendersonHDF jhendersonHDF Awaiting requested review from jhendersonHDF jhendersonHDF is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@mattjala mattjala Awaiting requested review from mattjala mattjala is a code owner

@brtnfld brtnfld Awaiting requested review from brtnfld brtnfld is a code owner

Assignees
No one assigned
Labels
Component - C Library Core C library issues (usually in the src directory) Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@derobins @qkoziol @byrnHDF