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

Allow users to specify data alignment #2177

Closed
hmaarrfk opened this issue Jan 8, 2022 · 44 comments · Fixed by #2206
Closed

Allow users to specify data alignment #2177

hmaarrfk opened this issue Jan 8, 2022 · 44 comments · Fixed by #2206

Comments

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Jan 8, 2022

In the mailing list I raised the question about data being aligned.

I provided a small code example that showed how data could be misaligned (code in python with a recent version of netcdf4-python)

Code Example
import xarray as xr
import numpy as np
import netCDF4
from pathlib import Path

basic_filename = "basic_file_netcdf4.nc"
if Path(basic_filename).exists():
    Path(basic_filename).unlink()

dataset = xr.DataArray(
    np.zeros((3072, 3072), dtype='uint8'),
    dims=("y", "x"),
    coords={
        "y": np.arange(3072, dtype=int),
        "x": np.arange(3072, dtype=int),
    },
    name='images').to_dataset()

dataset.to_netcdf(basic_filename, format="NETCDF4", engine="netcdf4")

import h5py
h5file = h5py.File(basic_filename)
h5dataset = h5file.get("images")
offset = h5dataset.id.get_offset()
print(offset % 4096)
print(offset % 2048)
print(offset % 1024)
print(offset % 512)
print(offset % 128)
print(offset % 64)

"""
3206
1158
134
134
6
6
"""

We came to the point where we determined that he user should set the File Access Property at Creation time, near:

if ((fapl_id = H5Pcreate(H5P_FILE_ACCESS)) < 0)

I'm opening this as an issue to keep track of the conversation on github.

cc: @edhartnett

Mailing list link: https://www.unidata.ucar.edu/mailing_lists/archives/netcdfgroup/2022/msg00000.html

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 8, 2022

Ed asked:

Is this change always on? Or does the user turn it on and off?

I believe this change is "always on" from my understanding, at file opening time???

Data alignment
    Sometimes file access is faster if certain data elements are aligned in a specific manner. 
    This can be controlled by setting alignment properties via the H5Pset_alignment function. 
    There are two values involved:

        A threshhold value
        An alignment interval

    Any allocation request at least as large as the threshold will be aligned on an address that 
    is a multiple of the alignment interval.

I presume you could change the alignment by closing and opening the file. It seems silly that it is a property for the file as a whole at opening time, but I understand that memory management is tricky and they might be trying to avoid a corner case.

My code for patching h5netcdf is the following, and should be quite similar

from h5py._hl.files import make_fapl as _make_fapl
import h5py
from functools import wraps

@wraps(_make_fapl)
def make_fapl(driver, libver, rdcc_nslots, rdcc_nbytes, rdcc_w0, locking,
              page_buf_size, min_meta_keep, min_raw_keep, **kwds):
    fapl = _make_fapl(driver, libver, rdcc_nslots, rdcc_nbytes, rdcc_w0, locking,
                      page_buf_size, min_meta_keep, min_raw_keep, **kwds)
    fapl.set_alignment(1024 * 1024 - 4096, 4096)
    return fapl

h5py._hl.files.make_fapl = make_fapl

fapl is called at creation / opening time:
https://github.com/h5py/h5py/blob/9c9033849e647d4254923bf9b50fcdedc1a320b3/h5py/_hl/files.py#L502

Cross referencing the post on h5py: h5py/h5py#2034

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 8, 2022

Just writing down other notes: it seems that there is some interplay between the file space strategy too:
https://docs.hdfgroup.org/hdf5/v1_12/group___f_a_p_l.html#gab99d5af749aeb3896fd9e3ceb273677a

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 8, 2022

cc @edwardhartnett i realize you have two accounts.

@DennisHeimbigner
Copy link
Collaborator

I am going to try to set the alignment on an open file. Can it be detected by h5dump?

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 9, 2022

I don't think the data alignment is persistent.

It is part of the File Access Property List, not File Creation Property List.

I've been:
Using the following h5py code for chunked data

import h5py
h5file = h5py.File(file_saved)
h5dataset = h5file.get("data")

for i in range(h5dataset.id.get_num_chunks()):
    print(h5dataset.id.get_chunk_info(i).byte_offset % 4096)

and for non chunked data

import h5py
h5file = h5py.File(file_saved)
h5dataset = h5file.get("data")
offset = h5dataset.id.get_offset() % 4096
print(offset)

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 9, 2022

The datasets are generated by:

  1. Using h5netcdf with a similar hack.
  2. Data that is larger than the threshold is created.
  3. Confirmed that it is aligned.

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 9, 2022

I think you can use:

h5ls --verbose --address YOUR_NC_FILE.nc

@hmaarrfk
Copy link
Contributor Author

In light of this whole discussion (especially around the backward compatible API), there may be a slew of other options that would help those of us that are performance minded. Things like the cache size, and other access property list items.

It might be a good idea to think of how to allow the users to have the most flexibility for future tweaks.

@Dave-Allured
Copy link
Contributor

How about this? Add a new function to the netCDF API, to be optionally called BEFORE opening a file. A new function does not harm backward API compatibility. This is for the C API, similar for Python. Tweak the name as you see fit, I don't care.

nc_set_alignment (threshold, alignment)

This would simply change the default behavior of nc_create and nc_open, and invoke H5Pset_alignment under the hood, at the appropriate moment before H5Fcreate or H5Fopen.

Because there is no existing file handle or FAPL at the user level before the netCDF open calls, the simplest implementation would be for the alignment settings to be global in some sense, and persist for multiple file opens, unless changed though another call.

My understanding is that changing object alignment like this, should have no impact at all on backward format compatibility.

@DennisHeimbigner
Copy link
Collaborator

That is a really good solution, thanks. And nothing is lost because the user can set and reset
the global flag before opening a file. so it would be possible to effectively make it per-file
if necessary.

Looking ahead, and as has been noted, there are other properties that may need to be set
via either an access fapl or a create fcpl. So we can either make this a one-off function,
or can generalize it to set other kinds of properties. I suppose it is ironic that we
are in-effect re-inventing the HDF5 properties mechanism :-)

@DennisHeimbigner
Copy link
Collaborator

In order to implement Greg's solution. I propose adding this function
to the netcdf API:

nc_set_hdf5_alignment(int threshold, int alignment)

@hmaarrfk
Copy link
Contributor Author

I see alot of trouble with "global settings".

The main challenge is that you want this to happen at the "application" level, but the definition of an "application" can change quite a bit.

It starts to somewhat start a conflict between different libraries, each optimizing for a different "default" usecase. In python at least, where "loading" a library is not very different from "executing" the script, it may start to conflict with the import order of things. Things might get lazy loaded in different order causing conflict between settings.

Furthermore, global settings also start to require "an access control mechanism", locks, coordination, or other.

If you want like to have global settings to start, I think that is a good way to begin things. However, would it be possible to request have an explicit API too? I'm happy to count it a "future" addition, but so long as it is understood that the "global settings" are not always desirable.

@edwardhartnett
Copy link
Contributor

@hmaarrfk I do not understand your point about global settings. Actually we already have some global settings:

/* Set the default nc_create format to NC_FORMAT_CLASSIC, NC_FORMAT_64BIT,
 * NC_FORMAT_CDF5, NC_FORMAT_NETCDF4, or NC_FORMAT_NETCDF4_CLASSIC */
EXTERNL int
nc_set_default_format(int format, int *old_formatp);

/* 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);

So this function would act the same way and should be named similarly. I would suggest not mentioning "hdf5" in the function name, since it's certainly conceivable that some other backend might one day allow control of alignment. (Does Zarr?)

I would suggest simply:
nc_set_alignment()/nc_get_alignment().

Just as with, for example nc_set_chunk_cache(), calling nc_set_alignment() will apply to all subsequent file opens.

@hmaarrfk
Copy link
Contributor Author

It is very conceivable to me, that one application may be using two netcdf files with vastly different optimization settings.

One might be on a network drive, the other on a local drive.

As such, global settings like this typically cause a clash. It would be nice to be able to override them, even if the initial API is "global-first".

@DennisHeimbigner
Copy link
Collaborator

Good point. I am going to have to reconsider your original solution, although
it will require some significant refactoring of existing code, I think,

@hmaarrfk
Copy link
Contributor Author

I too am not too excited that you can't change the memory allocator settings in HDF5 after a file is open. It seems a little silly. However, I know they are very interested in parallel access to files in a shared underlying file system. A tough problem.

I think the API as i proposed it would also me alot of work for me exposing it to high level libraries.

@DennisHeimbigner
Copy link
Collaborator

In terms of exposing it to higher level libraries, the nc_set_alignment API is in my experience
the easiest. Ed is correct that the nc_set_alignment approach already has analogs in the
chunk cache API.

@hmaarrfk
Copy link
Contributor Author

already has analogs in the chunk cache API.

Right. This means that you aren't really making this kind of optimization tuning any worse than it was before.

I presume once you start to understand the scope of parameters that your users want later, you can start to build an API that can override the defaults on a per function call.

@hmaarrfk
Copy link
Contributor Author

Perfect is the enemy of good enough!

@Dave-Allured
Copy link
Contributor

@hmaarrfk, by "global setting" I meant only that the alignment parameters would persist from one nc_open call to the next, if the user neglected to call nc_set_alignment between nc_open calls. This function would not change alignment for any previously opened files.

@DennisHeimbigner
Copy link
Collaborator

Here is a description of what I propose to implement.

Add support for setting HDF5 alignment property when creating a file

re: #2177
re: #2178

Provide get/set functions to store global data alignment information
and apply it when a file is created.

The api is as follows:

int nc_set_alignment(int threshold, int alignment);
int nc_get_alignment(int* thresholdp, int* alignmentp);

If defined, then for every file created (via nc_create())
after the call to nc_set_alignment the HDF5 function
H5Pset_alignment is applied to the created function using
the current global threshold and alignment values.

The nc_get_alignment function return the last values set by nc_set_alignment.
If nc_set_alignment has not been called, then it returns the value 0 for both
threshold and alignment.

@Dave-Allured
Copy link
Contributor

Add "for every file opened", as well as every file created. (Alignment is a non-persistent access property. As such, the same alignment options should be provided for re-opened existing files, as well as for new files.)

Same paragraph, I think you meant to say "... H5Pset_alignment is applied to the created FILE using ..."

Suggest alternate wording, "using the MOST RECENTLY SET threshold and alignment values." This to avoid misunderstanding about the term "global".

@DennisHeimbigner
Copy link
Collaborator

It is not clear to me that setting alignment on nc_open has any meaning.
because it would seem to interfere with the alignment of the variables
already in the file. Note that although alignment is technically not persistent,
it does have a detectable effect on data allocated in an existing file.
[Mark - can you enlighten me about this?]

@DennisHeimbigner
Copy link
Collaborator

Also, I need a test case; any suggestions?

@hmaarrfk
Copy link
Contributor Author

i can provide a test case (pseudo code)

  1. without alignment set, the variables are allocated at any offset. for uint8, they offset can quite well be anything.
  2. Create a file with alignment settings of Threshold 512, alignment 4096
  3. Create a variable 513 bytes in chunk size.
  4. Write the variable will have a byte offset that is %4096 == 0 in storage.
  5. close the file
  6. Open the file with HDF5 API.
  7. Find the HDF5 dataset check that the single chunk, or multiple chunks are all located at an offset of 4096.

For files that are edited, existing variables will remain in place. New variables (that are larger than the threshold) will be aligned.

In my test suite, I've avoided checking that without defining the alignment settings variable would not be aligned because there is a 1/4096 that it will be aligned.

@edwardhartnett
Copy link
Contributor

@DennisHeimbigner the idea is that these two functions would be added to the dispatch table, correct?

@hmaarrfk
Copy link
Contributor Author

Just an FYI:
On the HDF5 forum, it is confirmed that it must be specified at open time:
https://forum.hdfgroup.org/t/dynamically-change-the-file-access-property-list/9314/3

@DennisHeimbigner
Copy link
Collaborator

No, since they do not depend on the file, they are free-standing.

@Dave-Allured
Copy link
Contributor

It is not clear to me that setting alignment on nc_open has any meaning.
because it would seem to interfere with the alignment of the variables
already in the file. Note that although alignment is technically not persistent,
it does have a detectable effect on data allocated in an existing file.

H5Pset_alignment settings operate only when allocating low-level file objects, such as data chunks, at the physical disk space level. At this level, there is no sense of physical address alignment between chunks or other file objects. There is never any interference with alignment of previous variables, or parts of variables, etc.

For this type of alignment API to be complete for netCDF, it should apply to re-opened files as well as newly created files. It would be good if someone would check my analysis.

https://portal.hdfgroup.org/display/HDF5/H5P_SET_ALIGNMENT
or
https://docs.hdfgroup.org/hdf5/develop/group___f_a_p_l.html#title41

@Dave-Allured
Copy link
Contributor

Dave-Allured commented Jan 14, 2022

Alternatively, try a modification of the above test case from @hmaarrfk. Close and re-open the file, then write a second chunk, then close and test as per nos. 6-7 above. I predict that the first chunk will be aligned and the second chunk not aligned, unless nc_open is upgraded for alignment settings.

@DennisHeimbigner
Copy link
Collaborator

Here is a test program, but it does not seem to work as expected.

#include <nc_tests.h>
#include "err_macros.h"

#include <hdf5.h>
#include <H5DSpublic.h>

#define THRESHOLD 512
#define ALIGNMENT 4096

#define CHUNKSIZE 513

int
main(int argc, char **argv)
{
    int i,ncid, varid, dimids[1];
    size_t chunks[1];
    unsigned char data[CHUNKSIZE];
    hid_t fileid, grpid, datasetid;
    hid_t dxpl_id = H5P_DEFAULT; /*data transfer property list */
    unsigned int filter_mask = 0;
    hsize_t hoffset[1];

    H5Eset_auto2(H5E_DEFAULT,(H5E_auto2_t)H5Eprint1,stderr);

    printf("\n*** Testing HDF5 alignment.\n");

    if(nc_set_alignment(THRESHOLD,ALIGNMENT)) ERR;
    if (nc_create("tst_alignment.nc", NC_NETCDF4, &ncid)) ERR;
    if (nc_def_dim(ncid, "d0", CHUNKSIZE, &dimids[0])) ERR;
    if (nc_def_var(ncid, "var", NC_UBYTE, 1, dimids, &varid)) ERR;
    chunks[0] = CHUNKSIZE;
    if (nc_def_var_chunking(ncid, varid, NC_CHUNKED, chunks)) ERR;
    if (nc_enddef(ncid)) ERR;     

    for(i=0;i<CHUNKSIZE;i++) data[i] = (i)%2;
    if(nc_put_var(ncid,varid,data)) ERR;

    if (nc_close(ncid)) ERR;

    /* Use HDF5 API to verify */

    if ((fileid = H5Fopen("tst_alignment.nc", H5F_ACC_RDONLY, H5P_DEFAULT)) < 0) ERR;
    if ((grpid = H5Gopen1(fileid, "/")) < 0) ERR;
    if ((datasetid = H5Dopen1(grpid, "var")) < 0) ERR;

    memset(data,0,sizeof(data));
    hoffset[0] = 0;
    if(H5Dread_chunk(datasetid, dxpl_id, hoffset, &filter_mask, data) < 0) ERR;

    printf("offset=%lu   offset %% alignment=%lu\n",(unsigned long)hoffset, (((unsigned long)hoffset) % ALIGNMENT));
    for(i=0;i<CHUNKSIZE;i++) {
        if(data[i] != (i)%2) {
	    printf("data[%d] mismatch\n",i);
	}
    }    

    if (H5Dclose(datasetid) < 0) ERR;
    if (H5Gclose(grpid) < 0) ERR;
    if (H5Fclose(fileid) < 0) ERR;

    SUMMARIZE_ERR;
    FINAL_RESULTS;
}

@DennisHeimbigner
Copy link
Collaborator

I also tried using H5Dget_offset on the whole dataset, I get this.

chunksize=513 threshold=512 alignment=4096
H5Dget_offset: addr=18446744073709551615 addr % alignment=4095
H5Dread_chunk: offset=140737488347024   offset % alignment=3984

@Dave-Allured
Copy link
Contributor

Use one of the H5Dget_chunk_info* functions, not H5Dread_chunk, to get the physical offset of a chunk within the file.

H5Dget_offset should have worked for the whole dataset offset. I do not see the problem there. However, that might be displaying the offset of the dataset head node, rather than the first data chunk.

The utility command h5ls --verbose --address YOUR_NC_FILE.nc suggested by @hmaarrfk seems to display file offsets correctly, but labeled as "Location" rather than "Address". You might give that a try on your test file.

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 17, 2022

For what its worth, I had to add the following patch to my builds to make the alignment workout. You have to hit both the creation of the files, and the opening of the files

diff --git a/libhdf5/hdf5create.c b/libhdf5/hdf5create.c
index 0475c525..f5e108d9 100644
--- a/libhdf5/hdf5create.c
+++ b/libhdf5/hdf5create.c
@@ -125,6 +125,10 @@ nc4_create_file(const char *path, int cmode, size_t initialsz,
         BAIL(NC_EHDFERR);
     if (H5Pset_fclose_degree(fapl_id, H5F_CLOSE_WEAK))
         BAIL(NC_EHDFERR);
+    // if (H5Pset_alignment(fapl_id, alignment_threshold, alignment_interval) < 0) {
+    if (H5Pset_alignment(fapl_id, 15 * 4096, 4096) < 0) {
+        BAIL(NC_EHDFERR);
+    }

 #ifdef USE_PARALLEL4
     /* If this is a parallel file create, set up the file creation
diff --git a/libhdf5/hdf5open.c b/libhdf5/hdf5open.c
index f3ede3ed..bd787a6a 100644
--- a/libhdf5/hdf5open.c
+++ b/libhdf5/hdf5open.c
@@ -775,6 +775,10 @@ nc4_open_file(const char *path, int mode, void* parameters, int ncid)
     if (H5Pset_fclose_degree(fapl_id, H5F_CLOSE_WEAK) < 0)
         BAIL(NC_EHDFERR);

+    // if (H5Pset_alignment(fapl_id, alignment_threshold, alignment_interval) < 0) {
+    if (H5Pset_alignment(fapl_id, 15 * 4096, 4096) < 0) {
+        BAIL(NC_EHDFERR);
+    }
 #ifdef USE_PARALLEL4
     if (!(mode & (NC_INMEMORY | NC_DISKLESS)) && mpiinfo != NULL) {
         /* If this is a parallel file create, set up the file creation

edit: simplify patch to remove unecessary "cleanup" from demo.

@DennisHeimbigner
Copy link
Collaborator

if (H5Pset_alignment(fapl_id, 15 * 4096, 4096) < 0)

where do those constants come from: 15 and 4096?

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 17, 2022

Generally speaking, for "large" reads you want things to be aligned to something.

The linux kernel likes PAGES. So things that are "4096" in size are "nice'.

I'm defining something as large when it takes more than 16 or more pages (an arbitrary number). So I'm defining the threshold as 15 * 4096. It is entirely possible that I'm off by one in my "threshold" computation. It is possible that it is 15 * 4096 + 1 but honestly, I'm not so interested in edge behavior. My data is typically multiple megabytes large. The threshold is set to be around 60 kiB

@hmaarrfk
Copy link
Contributor Author

@hmaarrfk
Copy link
Contributor Author

In my demo code, I left placeholders for variable names that might be used for the global configuration.

@edwardhartnett
Copy link
Contributor

Certainly there should not be bare constants, so at least use defines for 15 and 4096...

@hmaarrfk
Copy link
Contributor Author

Of course, please see
image

It seems that the best way would be to allow the user to define the global constants you suggested:

  • alignment_threshold
  • alignment_interval

In my "patched" version, I didn't want to build up a parallel API to yours. This "works" for my specific application, I simply wanted to show the two locations I identified that had to be modified.

These constants would be set in the proposed nc_set_alignment.

@DennisHeimbigner
Copy link
Collaborator

I have a PR for this, but there are a large number of outstanding PRs, so I will wait until
Ward has had time to clear them.

@WardF
Copy link
Member

WardF commented Jan 25, 2022

I'm working through them now @DennisHeimbigner so feel free :)

@hmaarrfk
Copy link
Contributor Author

wow! thank you both so much for working through this. Excited to integrated this in our system.

DennisHeimbigner added a commit to DennisHeimbigner/netcdf-c that referenced this issue Jan 29, 2022
re: Unidata#2177
re: Unidata#2178

Provide get/set functions to store global data alignment
information and apply it when a file is created.

The api is as follows:
````
int nc_set_alignment(int threshold, int alignment);
int nc_get_alignment(int* thresholdp, int* alignmentp);
````

If defined, then for every file created opened after the call to
nc_set_alignment, for every new variable added to the file, the
most recently set threshold and alignment values will be applied
to that variable.

The nc_get_alignment function return the last values set by
nc_set_alignment.  If nc_set_alignment has not been called, then
it returns the value 0 for both threshold and alignment.

The alignment parameters are stored in the NCglobalstate object
(see below) for use as needed. Repeated calls to nc_set_alignment
will overwrite any existing values in NCglobalstate.

The alignment parameters are applied in libhdf5/hdf5create.c
and libhdf5/hdf5open.c

The set/get alignment functions are defined in libsrc4/nc4internal.c.

A test program was added as nc_test4/tst_alignment.c.

## Misc. Changes Unrelated to Alignment

* The NCRCglobalstate type was renamed to NCglobalstate to
  indicate that it represented more general global state than
  just .rc data.  It was also moved to nc4internal.h.  This led
  to a large number of small changes: mostly renaming. The
  global state management functions were moved to nc4internal.c.

* The global chunk cache variables have been moved into
  NCglobalstate.  As warranted, other global state will be moved
  as well.

* Some misc. problems with the nczarr performance tests were corrected.
@DennisHeimbigner
Copy link
Collaborator

Fixed by #2206

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