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.

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

We need to change the way we set caching, because HDF5 has changed... #1553

Closed
edwardhartnett opened this issue Nov 22, 2019 · 4 comments · Fixed by #1560
Closed

We need to change the way we set caching, because HDF5 has changed... #1553

edwardhartnett opened this issue Nov 22, 2019 · 4 comments · Fixed by #1560

Comments

@edwardhartnett
Copy link
Contributor

I was looking into caching as part of #1543 and I found a problem.

We have the following cache setting set/get functions:

/* Set the cache size, nelems, and preemption policy. */
EXTERNL int
nc_set_chunk_cache(size_t size, size_t nelems, float preemption);

/* Get the cache size, nelems, and preemption policy. */
EXTERNL int
nc_get_chunk_cache(size_t *sizep, size_t *nelemsp, float *preemptionp);

/* Set the per-variable cache size, nelems, and preemption policy. */
EXTERNL int
nc_set_var_chunk_cache(int ncid, int varid, size_t size, size_t nelems,
                       float preemption);

/* Get the per-variable cache size, nelems, and preemption policy. */
EXTERNL int
nc_get_var_chunk_cache(int ncid, int varid, size_t *sizep, size_t *nelemsp,
                       float *preemptionp);

Note that we are setting size, nelems, and preemption.

But the HDF5 function is: H5Pset_cache() (see https://support.hdfgroup.org/HDF5/doc/RM/RM_H5P.html#Property-SetCache)

This function now takes an extra parameter, nslots:

herr_t H5Pset_cache(hid_t plist_id, int mdc_nelmts, size_t rdcc_nslots, size_t rdcc_nbytes, double rdcc_w0)

According to the docs:

The mdc_nelmts parameter is no longer used; any value passed in that parameter will be ignored.
So we are passing nelems and it is being ignored, and they want nslots and we don't provide it.

I am quite tempted to just change the name of the parameter in the existing function! That might be the easiest solution.

@edwardhartnett
Copy link
Contributor Author

on the other hand, as I read the docs, nslots is exactly what nelems was. So I'm not sure what the difference is supposed to be. Maybe we just leave our API alone and set nslots in addition to nelems, to the same value?

@edwardhartnett
Copy link
Contributor Author

OK, it seems that this change has already been made, perhaps by me. ;-)

When I look in the code, I see that we are using the correct parameters. From hdf5create.c:

    if (H5Pset_cache(fapl_id, 0, nc4_chunk_cache_nelems, nc4_chunk_cache_size,
                     nc4_chunk_cache_preemption) < 0)
        BAIL(NC_EHDFERR);

Here's the HDF5 prototype:

herr_t H5Pset_cache(hid_t plist_id, int mdc_nelmts, size_t rdcc_nslots, size_t rdcc_nbytes, double rdcc_w0)

So although we call this nelems, we are actually using this for nsliots. I will document and we will leave this alone.

@epourmal
Copy link

epourmal commented Dec 2, 2019

Ed,

Documentation @ https://support.hdfgroup.org is not supported. Please use https://portal.hdfgorup.org instead for the current documentation.

We didn't touch the function since 1.8.0 release ;-) Please see https://portal.hdfgroup.org/display/HDF5/H5P_SET_CACHE.

Maybe you can also check this function https://portal.hdfgroup.org/display/HDF5/H5P_SET_CHUNK_CACHE that sets chunk cache per each dataset. BTW, you may consider disabling chunk cache in some cases (when application writes all data, or hyperslabs that correspond to a chunk or several chunks). Application will use less memory if chunk cache is disabled.

@edhartnett
Copy link
Contributor

Thanks! Maybe you should take down your old docs - it's what google sent me to when I googled the function name...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants