From 401bc333397e96da5696ee2d74a905a0222348a0 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 04:08:08 -0600 Subject: [PATCH 01/13] added function check_for_classic_model() --- libhdf5/hdf5file.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 0bc1c337d3..ef27fafc95 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2465,6 +2465,40 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) return retval; } +/** + * @internal Check for the attribute that indicates that netcdf + * classic model is in use. + * + * @param root_grp pointer to the group info for the root group of the + * file. + * + * @return NC_NOERR No error. + * @author Ed Hartnett + */ +static int +check_for_classic_model(NC_GRP_INFO_T *root_grp, int *is_classic) +{ + hid_t attid; + + /* Check inputs. */ + assert(!root_grp->parent && is_classic); + + /* If this attribute exists in the root group, then classic model + * is in effect. */ + if ((attid = H5Aopen_name(root_grp->hdf_grpid, NC3_STRICT_ATT_NAME)) < 0) + { + *is_classic = 0; + } + else + { + *is_classic = 1; + if (H5Aclose(attid) < 0) + return NC_EHDFERR; + } + + return NC_NOERR; +} + /** * @internal Open a netcdf-4 file. Things have already been kicked off * in ncfunc.c in nc_open, but here the netCDF-4 part of opening a @@ -2484,7 +2518,8 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc) hid_t fapl_id = H5P_DEFAULT; int retval; unsigned flags; - NC_HDF5_FILE_INFO_T* nc4_info = NULL; + NC_HDF5_FILE_INFO_T *nc4_info = NULL; + int is_classic; #ifdef USE_PARALLEL4 NC_MPI_INFO* mpiinfo = NULL; @@ -2605,6 +2640,12 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc) } else if ((nc4_info->hdfid = H5Fopen(path, flags, fapl_id)) < 0) BAIL(NC_EHDFERR); + /* Check for classic model attribute. */ + if ((retval = check_for_classic_model(nc4_info->root_grp, &is_classic))) + BAIL(retval); + if (is_classic) + nc4_info->cmode |= NC_CLASSIC_MODEL; + /* Now read in all the metadata. Some types and dimscale * information may be difficult to resolve here, if, for example, a * dataset of user-defined type is encountered before the From 5f850408c303b5b04db9f64c4117aa02f1fc5a42 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 04:40:00 -0600 Subject: [PATCH 02/13] adding lazy att handling --- include/nc4internal.h | 2 ++ libhdf5/hdf5file.c | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/nc4internal.h b/include/nc4internal.h index cc5e9feb83..75a56ef7fc 100644 --- a/include/nc4internal.h +++ b/include/nc4internal.h @@ -281,6 +281,7 @@ typedef struct NC_GRP_INFO hid_t hdf_grpid; struct NC_HDF5_FILE_INFO *nc4_info; struct NC_GRP_INFO *parent; + int atts_read; NCindex* children; /* NCindex */ NCindex* dim; /* NCindex * */ NCindex* att; /* NCindex * */ @@ -370,6 +371,7 @@ int nc4_rec_write_groups_types(NC_GRP_INFO_T *grp); int nc4_enddef_netcdf4_file(NC_HDF5_FILE_INFO_T *h5); int nc4_reopen_dataset(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var); int nc4_adjust_var_cache(NC_GRP_INFO_T *grp, NC_VAR_INFO_T * var); +int nc4_read_grp_atts(NC_GRP_INFO_T *grp); /* The following functions manipulate the in-memory linked list of metadata, without using HDF calls. */ diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index ef27fafc95..07ed88f88f 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2083,8 +2083,8 @@ read_var(NC_GRP_INFO_T *grp, hid_t datasetid, const char *obj_name, * @return ::NC_EHDFERR HDF5 returned error. * @author Ed Hartnett */ -static int -read_grp_atts(NC_GRP_INFO_T *grp) +int +nc4_read_grp_atts(NC_GRP_INFO_T *grp) { hid_t attid = -1; hsize_t num_obj, i; @@ -2436,7 +2436,7 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) } /* Scan the group for global (i.e. group-level) attributes. */ - if ((retval = read_grp_atts(grp))) + if ((retval = nc4_read_grp_atts(grp))) BAIL(retval); /* when exiting define mode, mark all variable written */ From dad70cf8809a68d6930d592a35ed9a41807f494c Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 04:54:03 -0600 Subject: [PATCH 03/13] more lazy atts --- libhdf4/hdf4file.c | 1 + libhdf5/hdf5file.c | 3 +++ libsrc4/nc4attr.c | 6 ++++++ 3 files changed, 10 insertions(+) diff --git a/libhdf4/hdf4file.c b/libhdf4/hdf4file.c index f02794036b..4488efde3f 100644 --- a/libhdf4/hdf4file.c +++ b/libhdf4/hdf4file.c @@ -645,6 +645,7 @@ NC_HDF4_open(const char *path, int mode, int basepe, size_t *chunksizehintp, for (a = 0; a < num_gatts; a++) if ((retval = hdf4_read_att(h5, NULL, a))) break; + h5->root_grp->atts_read = 1; /* Read each dataset. */ if (!retval) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 07ed88f88f..d78c45f02f 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2138,6 +2138,9 @@ nc4_read_grp_atts(NC_GRP_INFO_T *grp) attid = -1; } + /* Remember that we have read the atts for this group. */ + grp->atts_read = 1; + exit: if (attid > 0) { if(H5Aclose(attid) < 0) diff --git a/libsrc4/nc4attr.c b/libsrc4/nc4attr.c index 39f23cc526..438e8011f9 100644 --- a/libsrc4/nc4attr.c +++ b/libsrc4/nc4attr.c @@ -150,6 +150,12 @@ nc4_get_att(int ncid, int varid, const char *name, nc_type *xtype, if ((retval = nc4_normalize_name(name, norm_name))) BAIL(retval); + /* Read the global atts for this group, if they have not been + * read. */ + if (varid == NC_GLOBAL && !grp->atts_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + /* If this is one of the reserved atts, use nc_get_att_special. */ if (nc->ext_ncid == ncid && varid == NC_GLOBAL) { const NC_reservedatt* ra = NC_findreserved(norm_name); From 6b901692784f3b8dd62c70ed9eedc9aa674943df Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 05:05:44 -0600 Subject: [PATCH 04/13] switching to att_not_read --- include/nc4internal.h | 2 +- libhdf4/hdf4file.c | 1 - libhdf5/hdf5file.c | 7 ++++++- libsrc4/nc4attr.c | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/nc4internal.h b/include/nc4internal.h index 75a56ef7fc..97cee883e7 100644 --- a/include/nc4internal.h +++ b/include/nc4internal.h @@ -281,7 +281,7 @@ typedef struct NC_GRP_INFO hid_t hdf_grpid; struct NC_HDF5_FILE_INFO *nc4_info; struct NC_GRP_INFO *parent; - int atts_read; + int atts_not_read; NCindex* children; /* NCindex */ NCindex* dim; /* NCindex * */ NCindex* att; /* NCindex * */ diff --git a/libhdf4/hdf4file.c b/libhdf4/hdf4file.c index 4488efde3f..f02794036b 100644 --- a/libhdf4/hdf4file.c +++ b/libhdf4/hdf4file.c @@ -645,7 +645,6 @@ NC_HDF4_open(const char *path, int mode, int basepe, size_t *chunksizehintp, for (a = 0; a < num_gatts; a++) if ((retval = hdf4_read_att(h5, NULL, a))) break; - h5->root_grp->atts_read = 1; /* Read each dataset. */ if (!retval) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index d78c45f02f..4d9d0e0db4 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2139,7 +2139,7 @@ nc4_read_grp_atts(NC_GRP_INFO_T *grp) } /* Remember that we have read the atts for this group. */ - grp->atts_read = 1; + grp->atts_not_read = 0; exit: if (attid > 0) { @@ -3060,6 +3060,11 @@ NC4_inq(int ncid, int *ndimsp, int *nvarsp, int *nattsp, int *unlimdimidp) } if (nattsp) { + /* Do we need to read the atts? */ + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + *nattsp = ncindexcount(grp->att); } diff --git a/libsrc4/nc4attr.c b/libsrc4/nc4attr.c index 438e8011f9..1d6704d841 100644 --- a/libsrc4/nc4attr.c +++ b/libsrc4/nc4attr.c @@ -152,7 +152,7 @@ nc4_get_att(int ncid, int varid, const char *name, nc_type *xtype, /* Read the global atts for this group, if they have not been * read. */ - if (varid == NC_GLOBAL && !grp->atts_read) + if (varid == NC_GLOBAL && grp->atts_not_read) if ((retval = nc4_read_grp_atts(grp))) return retval; From 2dc6676bd8b0d00c665dbbb55195b29c3a6d2285 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 07:30:08 -0600 Subject: [PATCH 05/13] now checking for classic model --- libhdf5/hdf5file.c | 27 ++++++++++----------------- nc_test4/tst_files.c | 1 + 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 4d9d0e0db4..15f3a4e74b 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2481,23 +2481,16 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) static int check_for_classic_model(NC_GRP_INFO_T *root_grp, int *is_classic) { - hid_t attid; + htri_t attr_exists = -1; /* Check inputs. */ assert(!root_grp->parent && is_classic); /* If this attribute exists in the root group, then classic model * is in effect. */ - if ((attid = H5Aopen_name(root_grp->hdf_grpid, NC3_STRICT_ATT_NAME)) < 0) - { - *is_classic = 0; - } - else - { - *is_classic = 1; - if (H5Aclose(attid) < 0) - return NC_EHDFERR; - } + if ((attr_exists = H5Aexists(root_grp->hdf_grpid, NC3_STRICT_ATT_NAME)) < 0) + return NC_EHDFERR; + *is_classic = attr_exists ? 1 : 0; return NC_NOERR; } @@ -2643,12 +2636,6 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc) } else if ((nc4_info->hdfid = H5Fopen(path, flags, fapl_id)) < 0) BAIL(NC_EHDFERR); - /* Check for classic model attribute. */ - if ((retval = check_for_classic_model(nc4_info->root_grp, &is_classic))) - BAIL(retval); - if (is_classic) - nc4_info->cmode |= NC_CLASSIC_MODEL; - /* Now read in all the metadata. Some types and dimscale * information may be difficult to resolve here, if, for example, a * dataset of user-defined type is encountered before the @@ -2656,6 +2643,12 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc) if ((retval = nc4_rec_read_metadata(nc4_info->root_grp))) BAIL(retval); + /* Check for classic model attribute. */ + if ((retval = check_for_classic_model(nc4_info->root_grp, &is_classic))) + BAIL(retval); + if (is_classic) + nc4_info->cmode |= NC_CLASSIC_MODEL; + /* Now figure out which netCDF dims are indicated by the dimscale * information. */ if ((retval = nc4_rec_match_dimscales(nc4_info->root_grp))) diff --git a/nc_test4/tst_files.c b/nc_test4/tst_files.c index 27e0021ba8..629ac14aef 100644 --- a/nc_test4/tst_files.c +++ b/nc_test4/tst_files.c @@ -82,6 +82,7 @@ main(int argc, char **argv) /* Check the contents. Then define a new variable. Since it's * netcdf-4, nc_enddef isn't required - it's called * automatically. */ + hdf5_set_log_level(3); if (nc_open(FILE_NAME, NC_WRITE, &ncid)) ERR; if (nc_inq(ncid, &ndims, &nvars, &natts, &unlimdimid)) ERR; if (ndims != 2 || nvars != 2 || natts != 0 || unlimdimid != -1) ERR; From 81add527f27aedd2589ce774dcfd2c9e2bd65a66 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 07:39:42 -0600 Subject: [PATCH 06/13] lazy read of global atts now working --- libhdf5/hdf5attr.c | 8 +++++++- libhdf5/hdf5file.c | 5 +++-- libsrc4/nc4internal.c | 8 ++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/libhdf5/hdf5attr.c b/libhdf5/hdf5attr.c index 86ae0a61f0..1e0a6b5b04 100644 --- a/libhdf5/hdf5attr.c +++ b/libhdf5/hdf5attr.c @@ -175,9 +175,9 @@ NC4_del_att(int ncid, int varid, const char *name) NC_ATT_INFO_T *att; NCindex* attlist = NULL; hid_t locid = 0, datasetid = 0; - int retval = NC_NOERR; int i; size_t deletedid; + int retval; if (!name) return NC_EINVAL; @@ -204,6 +204,12 @@ NC4_del_att(int ncid, int varid, const char *name) BAIL(retval); } + /* Do we need to read the atts? */ + if (varid == NC_GLOBAL) + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + /* Get either the global or a variable attribute list. Also figure out the HDF5 location it's attached to. */ attlist = getattlist(grp,varid,&var); diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 15f3a4e74b..1fe400f53c 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2439,8 +2439,9 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) } /* Scan the group for global (i.e. group-level) attributes. */ - if ((retval = nc4_read_grp_atts(grp))) - BAIL(retval); + grp->atts_not_read = 1; + /* if ((retval = nc4_read_grp_atts(grp))) */ + /* BAIL(retval); */ /* when exiting define mode, mark all variable written */ for (i=0; ivars); i++) { diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index 8f4409fc6d..3a18e35fbf 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -460,6 +460,7 @@ nc4_find_grp_att(NC_GRP_INFO_T *grp, int varid, const char *name, int attnum, { NC_VAR_INFO_T *var; NCindex* attlist = NULL; + int retval; assert(grp && grp->hdr.name); LOG((4, "nc4_find_grp_att: grp->name %s varid %d name %s attnum %d", @@ -467,7 +468,14 @@ nc4_find_grp_att(NC_GRP_INFO_T *grp, int varid, const char *name, int attnum, /* Get either the global or a variable attribute list. */ if (varid == NC_GLOBAL) + { attlist = grp->att; + + /* Do we need to read the atts? */ + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + } else { var = (NC_VAR_INFO_T*)ncindexith(grp->vars,varid); From a01da62481f80864c9c0110618e93e8e1977d639 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 07:51:49 -0600 Subject: [PATCH 07/13] isolating code to read variable attributes --- include/nc4internal.h | 4 +--- libhdf5/hdf5file.c | 47 +++++++++++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/include/nc4internal.h b/include/nc4internal.h index 97cee883e7..49dd2507a8 100644 --- a/include/nc4internal.h +++ b/include/nc4internal.h @@ -180,9 +180,7 @@ typedef struct NC_VAR_INFO nc_bool_t written_to; /* True if variable has data written to it */ struct NC_TYPE_INFO *type_info; hid_t hdf_datasetid; -#if 0 - int natts; /* Use explicit index because there may be gaps in numbers */ -#endif + int atts_not_read; /* If true, the atts have not yet been read. */ NCindex* att; /* NCindex */ nc_bool_t no_fill; /* True if no fill value is defined for var */ void *fill_value; diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 1fe400f53c..a32192a105 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -1800,6 +1800,36 @@ read_type(NC_GRP_INFO_T *grp, hid_t hdf_typeid, char *type_name) return retval; } +/** + * @internal This function reads all the attributes of a variable. + * + * @param grp Pointer to the group info. + * @param var Pointer to the var info. + * + * @return NC_NOERR No error. + * @author Ed Hartnett + */ +int +nc4_read_var_atts(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var) +{ + att_iter_info att_info; /* Custom iteration information */ + + /* Check inputs. */ + assert(grp && var); + + /* Assign var and grp in struct. */ + att_info.var = var; + att_info.grp = grp; + + /* Now read all the attributes of this variable, ignoring the + ones that hold HDF5 dimension scale information. */ + if ((H5Aiterate2(var->hdf_datasetid, H5_INDEX_CRT_ORDER, H5_ITER_INC, NULL, + att_read_var_callbk, &att_info)) < 0) + return NC_EATTMETA; + + return NC_NOERR; +} + /** * @internal This function is called by read_dataset(), (which is called * by nc4_rec_read_metadata()) when a netCDF variable is found in the @@ -1825,7 +1855,6 @@ read_var(NC_GRP_INFO_T *grp, hid_t datasetid, const char *obj_name, hid_t access_pid = 0; int incr_id_rc = 0; /* Whether the dataset ID's ref count has been incremented */ int d; - att_iter_info att_info; /* Custom iteration information */ H5Z_filter_t filter; int num_filters; unsigned int cd_values_zip[CD_NELEMS_ZLIB]; @@ -2040,15 +2069,9 @@ read_var(NC_GRP_INFO_T *grp, hid_t datasetid, const char *obj_name, } } - /* Now read all the attributes of this variable, ignoring the - ones that hold HDF5 dimension scale information. */ - - att_info.var = var; - att_info.grp = grp; - - if ((H5Aiterate2(var->hdf_datasetid, H5_INDEX_CRT_ORDER, H5_ITER_INC, NULL, - att_read_var_callbk, &att_info)) < 0) - BAIL(NC_EATTMETA); + /* Read variable attributes. */ + if ((retval = nc4_read_var_atts(grp, var))) + BAIL(retval); /* Is this a deflated variable with a chunksize greater than the * current cache size? */ @@ -2438,10 +2461,8 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) BAIL(NC_EHDFERR); } - /* Scan the group for global (i.e. group-level) attributes. */ + /* Defer the reading of global atts until someone asks for one. */ grp->atts_not_read = 1; - /* if ((retval = nc4_read_grp_atts(grp))) */ - /* BAIL(retval); */ /* when exiting define mode, mark all variable written */ for (i=0; ivars); i++) { From b524cd04bcef2bc0a5c06274121b55cd24c564e0 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 07:56:54 -0600 Subject: [PATCH 08/13] adding calls to get var atts when needed --- include/nc4internal.h | 1 + libsrc4/nc4var.c | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/include/nc4internal.h b/include/nc4internal.h index 49dd2507a8..a16f27408a 100644 --- a/include/nc4internal.h +++ b/include/nc4internal.h @@ -370,6 +370,7 @@ int nc4_enddef_netcdf4_file(NC_HDF5_FILE_INFO_T *h5); int nc4_reopen_dataset(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var); int nc4_adjust_var_cache(NC_GRP_INFO_T *grp, NC_VAR_INFO_T * var); int nc4_read_grp_atts(NC_GRP_INFO_T *grp); +int nc4_read_var_atts(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var); /* The following functions manipulate the in-memory linked list of metadata, without using HDF calls. */ diff --git a/libsrc4/nc4var.c b/libsrc4/nc4var.c index ea0854c20f..81aeaa27df 100644 --- a/libsrc4/nc4var.c +++ b/libsrc4/nc4var.c @@ -752,6 +752,11 @@ NC4_inq_var_all(int ncid, int varid, char *name, nc_type *xtypep, { if (nattsp) { + /* Do we need to read the atts? */ + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + *nattsp = ncindexcount(grp->att); } return NC_NOERR; @@ -775,6 +780,9 @@ NC4_inq_var_all(int ncid, int varid, char *name, nc_type *xtypep, dimidsp[d] = var->dimids[d]; if (nattsp) { + if (var->atts_not_read) + if ((retval = nc4_read_var_atts(grp, var))) + return retval; *nattsp = ncindexcount(var->att); } From 77936f2884392ae991cf761ef10c3834b6c1a31a Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 08:25:19 -0600 Subject: [PATCH 09/13] more progress towards lazy read of atts for vars --- libhdf5/hdf5attr.c | 84 ++++++++++++++++++++++++++++++++++--------- libhdf5/hdf5file.c | 3 ++ libsrc4/nc4internal.c | 6 ++++ 3 files changed, 76 insertions(+), 17 deletions(-) diff --git a/libhdf5/hdf5attr.c b/libhdf5/hdf5attr.c index 1e0a6b5b04..e761dd910f 100644 --- a/libhdf5/hdf5attr.c +++ b/libhdf5/hdf5attr.c @@ -43,6 +43,54 @@ getattlist(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp) } } +/** + * @internal Get the attribute list for either a varid or NC_GLOBAL + * + * @param grp Group + * @param varid Variable ID | NC_BLOGAL + * @param varp Pointer that gets pointer to NC_VAR_INFO_T + * instance. Ignored if NULL. + * @param attlist Pointer that gets pointer to attribute list. + * + * @return NC_NOERR No error. + * @author Dennis Heimbigner, Ed Hartnett + */ +static int +getattlist2(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp, + NCindex **attlist) +{ + NC_VAR_INFO_T* var; + int retval; + + if (varid == NC_GLOBAL) + { + /* Do we need to read the atts? */ + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + + if (varp) + *varp = NULL; + *attlist = grp->att; + } + else + { + if (!(var = (NC_VAR_INFO_T *)ncindexith(grp->vars, varid))) + return NC_ENOTVAR; + assert(var->hdr.id == varid); + + /* Do we need to read the atts? */ + if (var->atts_not_read) + if ((retval = nc4_read_var_atts(grp, var))) + return retval; + + if (varp) + *varp = var; + *attlist = var->att; + } + return NC_NOERR; +} + /** * @internal I think all atts should be named the exact same thing, to * avoid confusion! @@ -92,11 +140,13 @@ NC4_rename_att(int ncid, int varid, const char *name, const char *newname) if ((retval = nc4_check_name(newname, norm_newname))) return retval; - /* Is new name in use? */ - list = getattlist(grp,varid,&var); - if(list == NULL) - return NC_ENOTVAR; + /* Get the list of attributes. */ + /* if (!(list = getattlist(grp,varid,&var))) */ + /* return NC_ENOTVAR; */ + if ((retval = getattlist2(grp, varid, &var, &list))) + return retval; + /* Is new name in use? */ att = (NC_ATT_INFO_T*)ncindexlookup(list,norm_newname); if(att != NULL) return NC_ENAMEINUSE; @@ -204,17 +254,15 @@ NC4_del_att(int ncid, int varid, const char *name) BAIL(retval); } - /* Do we need to read the atts? */ - if (varid == NC_GLOBAL) - if (grp->atts_not_read) - if ((retval = nc4_read_grp_atts(grp))) - return retval; + /* Get either the global or a variable attribute list. */ + /* if (!(attlist = getattlist(grp,varid,NULL))) */ + /* return NC_ENOTVAR; */ + if ((retval = getattlist2(grp, varid, &var, &attlist))) + return retval; + /* if (!(attlist = getattlist(grp,varid,NULL))) */ + /* return NC_ENOTVAR; */ - /* Get either the global or a variable attribute list. Also figure - out the HDF5 location it's attached to. */ - attlist = getattlist(grp,varid,&var); - if(attlist == NULL) - return NC_ENOTVAR; + /* Determine the location id in the HDF5 file. */ if (varid == NC_GLOBAL) locid = grp->hdf_grpid; else if (var->created) @@ -303,9 +351,11 @@ NC4_put_att(int ncid, int varid, const char *name, nc_type file_type, /* Find att, if it exists. (Must check varid first or nc_test will * break.) */ - attlist = getattlist(grp,varid,&var); - if(attlist == NULL) - return NC_ENOTVAR; + if ((ret = getattlist2(grp, varid, &var, &attlist))) + return ret; + /* attlist = getattlist(grp,varid,&var); */ + /* if(attlist == NULL) */ + /* return NC_ENOTVAR; */ /* The length needs to be positive (cast needed for braindead systems with signed size_t). */ diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index a32192a105..9b52090e51 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -1827,6 +1827,9 @@ nc4_read_var_atts(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var) att_read_var_callbk, &att_info)) < 0) return NC_EATTMETA; + /* Remember that we have read the atts for this var. */ + var->atts_not_read = 0; + return NC_NOERR; } diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index 3a18e35fbf..87f9f51798 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -480,6 +480,12 @@ nc4_find_grp_att(NC_GRP_INFO_T *grp, int varid, const char *name, int attnum, { var = (NC_VAR_INFO_T*)ncindexith(grp->vars,varid); if (!var) return NC_ENOTVAR; + + /* Do we need to read the var attributes? */ + if (var->atts_not_read) + if ((retval = nc4_read_var_atts(grp, var))) + return retval; + attlist = var->att; assert(var->hdr.id == varid); } From eafe151f1317267410e4ed8ce0683bc565918ac4 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Tue, 19 Jun 2018 14:59:07 -0600 Subject: [PATCH 10/13] added test --- libhdf5/hdf5attr.c | 38 ++--------- libhdf5/hdf5file.c | 7 +- libsrc4/nc4attr.c | 25 +++++--- nc_test4/Makefile.am | 4 +- nc_test4/perftest.sh | 6 +- nc_test4/tst_attsperf.c | 138 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 49 deletions(-) mode change 100644 => 100755 nc_test4/perftest.sh create mode 100644 nc_test4/tst_attsperf.c diff --git a/libhdf5/hdf5attr.c b/libhdf5/hdf5attr.c index e761dd910f..264015d6b7 100644 --- a/libhdf5/hdf5attr.c +++ b/libhdf5/hdf5attr.c @@ -18,31 +18,6 @@ int nc4typelen(nc_type type); -/** - * @internal Get the attribute list for either a varid or NC_GLOBAL - * - * @param grp Group - * @param varid Variable ID | NC_BLOGAL - * @param varp Pointer into which to return created NC_VAR_INFO_T instance - * - * @return Attribute list | NULL - * @author Dennis Heimbigner - */ -static NCindex * -getattlist(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp) -{ - if (varid == NC_GLOBAL) { - if(varp) *varp = NULL; - return grp->att; - } else { - NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)ncindexith(grp->vars,varid); - if (!var) return NULL; - assert(var->hdr.id == varid); - if(varp) *varp = var; - return var->att; - } -} - /** * @internal Get the attribute list for either a varid or NC_GLOBAL * @@ -56,7 +31,7 @@ getattlist(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp) * @author Dennis Heimbigner, Ed Hartnett */ static int -getattlist2(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp, +getattlist(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp, NCindex **attlist) { NC_VAR_INFO_T* var; @@ -143,7 +118,7 @@ NC4_rename_att(int ncid, int varid, const char *name, const char *newname) /* Get the list of attributes. */ /* if (!(list = getattlist(grp,varid,&var))) */ /* return NC_ENOTVAR; */ - if ((retval = getattlist2(grp, varid, &var, &list))) + if ((retval = getattlist(grp, varid, &var, &list))) return retval; /* Is new name in use? */ @@ -257,10 +232,8 @@ NC4_del_att(int ncid, int varid, const char *name) /* Get either the global or a variable attribute list. */ /* if (!(attlist = getattlist(grp,varid,NULL))) */ /* return NC_ENOTVAR; */ - if ((retval = getattlist2(grp, varid, &var, &attlist))) + if ((retval = getattlist(grp, varid, &var, &attlist))) return retval; - /* if (!(attlist = getattlist(grp,varid,NULL))) */ - /* return NC_ENOTVAR; */ /* Determine the location id in the HDF5 file. */ if (varid == NC_GLOBAL) @@ -351,11 +324,8 @@ NC4_put_att(int ncid, int varid, const char *name, nc_type file_type, /* Find att, if it exists. (Must check varid first or nc_test will * break.) */ - if ((ret = getattlist2(grp, varid, &var, &attlist))) + if ((ret = getattlist(grp, varid, &var, &attlist))) return ret; - /* attlist = getattlist(grp,varid,&var); */ - /* if(attlist == NULL) */ - /* return NC_ENOTVAR; */ /* The length needs to be positive (cast needed for braindead systems with signed size_t). */ diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 9b52090e51..42962f878a 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -2073,8 +2073,9 @@ read_var(NC_GRP_INFO_T *grp, hid_t datasetid, const char *obj_name, } /* Read variable attributes. */ - if ((retval = nc4_read_var_atts(grp, var))) - BAIL(retval); + var->atts_not_read = 1; + /* if ((retval = nc4_read_var_atts(grp, var))) */ + /* BAIL(retval); */ /* Is this a deflated variable with a chunksize greater than the * current cache size? */ @@ -2466,6 +2467,8 @@ nc4_rec_read_metadata(NC_GRP_INFO_T *grp) /* Defer the reading of global atts until someone asks for one. */ grp->atts_not_read = 1; + /* if ((retval = nc4_read_grp_atts(grp))) */ + /* return retval; */ /* when exiting define mode, mark all variable written */ for (i=0; ivars); i++) { diff --git a/libsrc4/nc4attr.c b/libsrc4/nc4attr.c index 1d6704d841..1f69707203 100644 --- a/libsrc4/nc4attr.c +++ b/libsrc4/nc4attr.c @@ -116,6 +116,7 @@ nc4_get_att(int ncid, int varid, const char *name, nc_type *xtype, NC_GRP_INFO_T *grp; NC_HDF5_FILE_INFO_T *h5; NC_ATT_INFO_T *att = NULL; + NC_VAR_INFO_T *var; int my_attnum = -1; int need_to_convert = 0; int range_error = NC_NOERR; @@ -136,9 +137,9 @@ nc4_get_att(int ncid, int varid, const char *name, nc_type *xtype, return retval; /* Check varid */ - if (varid != NC_GLOBAL) { - NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)ncindexith(grp->vars,varid); - if(var == NULL) + if (varid != NC_GLOBAL) + { + if (!(var = (NC_VAR_INFO_T*)ncindexith(grp->vars,varid))) return NC_ENOTVAR; assert(var->hdr.id == varid); } @@ -150,11 +151,19 @@ nc4_get_att(int ncid, int varid, const char *name, nc_type *xtype, if ((retval = nc4_normalize_name(name, norm_name))) BAIL(retval); - /* Read the global atts for this group, if they have not been - * read. */ - if (varid == NC_GLOBAL && grp->atts_not_read) - if ((retval = nc4_read_grp_atts(grp))) - return retval; + /* Read the atts for this group/var, if they have not been read. */ + if (varid == NC_GLOBAL) + { + if (grp->atts_not_read) + if ((retval = nc4_read_grp_atts(grp))) + return retval; + } + else + { + if (var->atts_not_read) + if ((retval = nc4_read_var_atts(grp, var))) + return retval; + } /* If this is one of the reserved atts, use nc_get_att_special. */ if (nc->ext_ncid == ncid && varid == NC_GLOBAL) { diff --git a/nc_test4/Makefile.am b/nc_test4/Makefile.am index f28637ab40..242a94658d 100644 --- a/nc_test4/Makefile.am +++ b/nc_test4/Makefile.am @@ -121,8 +121,8 @@ TESTS += run_par_test.sh endif if ENABLE_METADATA_PERF -check_PROGRAMS += bigmeta openbigmeta -TESTS += perftest.sh +check_PROGRAMS += bigmeta openbigmeta tst_attsperf +TESTS += tst_attsperf perftest.sh endif EXTRA_DIST = run_par_test.sh run_bm.sh run_bm_test1.sh \ diff --git a/nc_test4/perftest.sh b/nc_test4/perftest.sh old mode 100644 new mode 100755 index 04caa4900a..1facd9b575 --- a/nc_test4/perftest.sh +++ b/nc_test4/perftest.sh @@ -1,8 +1,8 @@ #!/bin/sh -#PROF=1 -#DEBUG=1 -#MEM=1 +PROF=1 +DEBUG=1 +MEM=1 if test "x$srcdir" = x ; then srcdir=`pwd`; fi . ../test_common.sh diff --git a/nc_test4/tst_attsperf.c b/nc_test4/tst_attsperf.c new file mode 100644 index 0000000000..20f6b54cd6 --- /dev/null +++ b/nc_test4/tst_attsperf.c @@ -0,0 +1,138 @@ +/* This is part of the netCDF package. Copyright 2018 University + * Corporation for Atmospheric Research/Unidata. See COPYRIGHT file + * for conditions of use. + * + * Test the netCDF-4 attribute code. + * + * WARNING: do not attempt to run this under windows because of the use + * of gettimeofday(). + * + * Ed Hartnett 6/19/18 +*/ + +#include +#include +#include "err_macros.h" +#include "nc4internal.h" +#include + +#define TEST "tst_attsperf" +#define VAR "bigvar" +#define NDIMS 2 +#define DIM0 "d0" +#define DIM1 "d1" +#define DIMSIZE0 16 +#define DIMSIZE1 512 +#define TOTALSIZE (DIMSIZE0 * DIMSIZE1) +#define NUM_ATTS 100 +#define ATT_LEN 10 +#define NUM_VARS 100 + +int +add_attributes(int ncid, int varid) +{ + char att_name[NC_MAX_NAME + 1]; + double att_data[ATT_LEN]; + int i, a; + + /* Fill up data. */ + for (i = 0; i < ATT_LEN; i++) + att_data[i] = i; + + /* Write a bunch of attributes. */ + for (a = 0; a < NUM_ATTS; a++) + { + sprintf(att_name, "%s_varid_%d_att_%d", TEST, varid, a); + if (nc_put_att_double(ncid, varid, att_name, NC_DOUBLE, + ATT_LEN, att_data)) ERR; + } + + return 0; +} + +int +buildfile(int file_no) +{ + int ncid, varid; + int dimids[NDIMS]; + char file_name[NC_MAX_NAME + 1]; + int v; + + sprintf(file_name, "%s_%d.nc", TEST, file_no); + + if (nc_create(file_name, NC_NETCDF4, &ncid)) ERR; + + if (nc_def_dim(ncid, DIM0, DIMSIZE0, &dimids[0])) ERR; + if (nc_def_dim(ncid, DIM1, DIMSIZE1, &dimids[1])) ERR; + for (v = 0; v < NUM_VARS; v++) + { + char var_name[NC_MAX_NAME + 1]; + sprintf(var_name, "%s_var_%d", TEST, v); + if (nc_def_var(ncid, var_name, NC_INT, NDIMS, dimids, &varid)) ERR; + if (add_attributes(ncid, v)) ERR; + } + if (add_attributes(ncid, NC_GLOBAL)) ERR; + if (nc_enddef(ncid)) ERR; + + if (nc_close(ncid)) ERR; + return 0; +} + +long long +readfile(int inq_all) +{ + int ncid; + struct timeval starttime, endtime; + long long delta; + long long startt, endt; + char file_name[NC_MAX_NAME + 1]; + + sprintf(file_name, "%s_%d.nc", TEST, inq_all); + + /* Start the clock. */ + gettimeofday(&starttime, NULL); + + /* Open the file. */ + if (nc_open(file_name, NC_NETCDF4, &ncid)) ERR; + + /* Simulate old open by triggering attribute reads, if desired. */ + if (inq_all) + { + int natts; + int v; + + /* When checking the number of atts, we trigger the read. */ + if (nc_inq(ncid, NULL, NULL, &natts, NULL)) ERR; + for (v = 0; v < NUM_VARS; v++) + if (nc_inq_varnatts(ncid, v, &natts)) ERR; + } + gettimeofday(&endtime, NULL); + + /* Close the file. */ + if (nc_close(ncid)) ERR; + + /* Compute the time delta */ + startt = (1000000 * starttime.tv_sec) + starttime.tv_usec; + endt = (1000000 * endtime.tv_sec) + endtime.tv_usec; + delta = endt - startt; + return delta; +} + +int +main(int argc, char **argv) +{ + long long zerodelta, onedelta, factor; + + printf("testing speed of open with files with lots of metadata...\n"); + if (buildfile(0)) ERR; + if (buildfile(1)) ERR; + if ((zerodelta = readfile(0)) == -1) ERR; + if ((onedelta = readfile(1)) == -1) ERR; + + /* Print results to the millisec */ + factor = onedelta / zerodelta; + printf("Lazy Atts time=%lld Read Atts at Open time=%lld Speedup=%lld\n", + zerodelta, onedelta, factor); + SUMMARIZE_ERR; + FINAL_RESULTS; +} From d5baca7f978ccf7e2599cb04f67053c250290692 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 21 Jun 2018 06:36:12 -0600 Subject: [PATCH 11/13] fixed test issue --- nc_test4/perftest.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nc_test4/perftest.sh b/nc_test4/perftest.sh index 1facd9b575..04caa4900a 100755 --- a/nc_test4/perftest.sh +++ b/nc_test4/perftest.sh @@ -1,8 +1,8 @@ #!/bin/sh -PROF=1 -DEBUG=1 -MEM=1 +#PROF=1 +#DEBUG=1 +#MEM=1 if test "x$srcdir" = x ; then srcdir=`pwd`; fi . ../test_common.sh From 97faffe0721e6746489ed58abecaac9b13ac10a9 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 21 Jun 2018 06:51:45 -0600 Subject: [PATCH 12/13] fixed test issue --- nc_test4/tst_files.c | 1 - 1 file changed, 1 deletion(-) diff --git a/nc_test4/tst_files.c b/nc_test4/tst_files.c index 629ac14aef..27e0021ba8 100644 --- a/nc_test4/tst_files.c +++ b/nc_test4/tst_files.c @@ -82,7 +82,6 @@ main(int argc, char **argv) /* Check the contents. Then define a new variable. Since it's * netcdf-4, nc_enddef isn't required - it's called * automatically. */ - hdf5_set_log_level(3); if (nc_open(FILE_NAME, NC_WRITE, &ncid)) ERR; if (nc_inq(ncid, &ndims, &nvars, &natts, &unlimdimid)) ERR; if (ndims != 2 || nvars != 2 || natts != 0 || unlimdimid != -1) ERR; From d4967405b407e0a8e564cdf94ea83c785609f7a0 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 21 Jun 2018 19:49:30 -0600 Subject: [PATCH 13/13] cleaned up --- libhdf5/hdf5attr.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libhdf5/hdf5attr.c b/libhdf5/hdf5attr.c index 264015d6b7..bc876f3894 100644 --- a/libhdf5/hdf5attr.c +++ b/libhdf5/hdf5attr.c @@ -116,8 +116,6 @@ NC4_rename_att(int ncid, int varid, const char *name, const char *newname) return retval; /* Get the list of attributes. */ - /* if (!(list = getattlist(grp,varid,&var))) */ - /* return NC_ENOTVAR; */ if ((retval = getattlist(grp, varid, &var, &list))) return retval; @@ -230,8 +228,6 @@ NC4_del_att(int ncid, int varid, const char *name) } /* Get either the global or a variable attribute list. */ - /* if (!(attlist = getattlist(grp,varid,NULL))) */ - /* return NC_ENOTVAR; */ if ((retval = getattlist(grp, varid, &var, &attlist))) return retval;