[svn-r16482] Purpose: Improve chunk cache

Description:
The meaning of the "nbytes" field in H5D_rdcc_t was not clear, and some places
assumed it was the maximum size of the chunk cache, while some assumed it was
the current size of the chunk cache.  The end result was that only 1 chunk could
be held in cache at a time.  This field has been replaced by "nbytes_max" and
"nbytes_used".  Performance of cached I/O should improve greatly.

Tested: jam, smirom (h5committest)

5 files changed, 244 insertions, 10 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 5690b68..88cca65 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -81,6 +81,8 @@ Bug Fixes since HDF5-1.6.8 Release
 
     Library
     -------
+    - Fixed a bug that prevented more than one dataset chunk from being cached
+            at a time. NAF - 2009/02/12
     - H5Dset_extent: when shrinking dimensions, some chunks were not deleted. 
             Shrinking to a zero size dimension caused an assertion in the library.
             (PVN - 2009/01/8)
diff --git a/src/H5Distore.c b/src/H5Distore.c
index 1da66f2..b2a7097 100644
--- a/src/H5Distore.c
+++ b/src/H5Distore.c
@@ -919,7 +919,7 @@ H5D_istore_init (const H5F_t *f, const H5D_t *dset)
     FUNC_ENTER_NOAPI(H5D_istore_init, FAIL)
 
     if (H5F_RDCC_NBYTES(f)>0 && H5F_RDCC_NELMTS(f)>0) {
-        rdcc->nbytes=H5F_RDCC_NBYTES(f);
+        rdcc->nbytes_max = H5F_RDCC_NBYTES(f);
 	rdcc->nslots = H5F_RDCC_NELMTS(f);
 	rdcc->slot = H5FL_SEQ_CALLOC (H5D_rdcc_ent_ptr_t,rdcc->nslots);
 	if (NULL==rdcc->slot)
@@ -1101,7 +1101,7 @@ H5D_istore_preempt(const H5D_io_info_t *io_info, H5D_rdcc_ent_t * ent, hbool_t f
     /* Remove from cache */
     rdcc->slot[ent->idx] = NULL;
     ent->idx = UINT_MAX;
-    rdcc->nbytes -= ent->chunk_size;
+    rdcc->nbytes_used -= ent->chunk_size;
     --rdcc->nused;
 
     /* Free */
@@ -1337,7 +1337,7 @@ H5D_istore_prune (const H5D_io_info_t *io_info, size_t size)
 {
     int		i, j, nerrors=0;
     const H5D_rdcc_t	*rdcc = &(io_info->dset->shared->cache.chunk);
-    size_t		total = rdcc->nbytes;
+    size_t		total = rdcc->nbytes_max;
     const int		nmeth=2;	/*number of methods		*/
     int		        w[1];		/*weighting as an interval	*/
     H5D_rdcc_ent_t	*p[2], *cur;	/*list pointers			*/
@@ -1360,7 +1360,7 @@ H5D_istore_prune (const H5D_io_info_t *io_info, size_t size)
     p[0] = rdcc->head;
     p[1] = NULL;
 
-    while ((p[0] || p[1]) && rdcc->nbytes+size>total) {
+    while ((p[0] || p[1]) && rdcc->nbytes_used+size>total) {
 
 	/* Introduce new pointers */
 	for (i=0; i<nmeth-1; i++)
@@ -1372,7 +1372,7 @@ H5D_istore_prune (const H5D_io_info_t *io_info, size_t size)
             n[i] = p[i] ? p[i]->next : NULL;
 
 	/* Give each method a chance */
-	for (i=0; i<nmeth && rdcc->nbytes+size>total; i++) {
+	for (i=0; i<nmeth && rdcc->nbytes_used+size>total; i++) {
 	    if (0==i && p[0] && !p[0]->locked &&
                     ((0==p[0]->rd_count && 0==p[0]->wr_count) ||
                      (0==p[0]->rd_count && p[0]->chunk_size==p[0]->wr_count) ||
@@ -1699,7 +1699,7 @@ H5D_istore_lock(const H5D_io_info_t *io_info,
     }
     assert (found || chunk_size>0);
 
-    if (!found && rdcc->nslots>0 && chunk_size<=rdcc->nbytes &&
+    if (!found && rdcc->nslots>0 && chunk_size<=rdcc->nbytes_max &&
             (!ent || !ent->locked)) {
         /*
          * Add the chunk to the cache only if the slot is not already locked.
@@ -1732,7 +1732,7 @@ H5D_istore_lock(const H5D_io_info_t *io_info,
         assert(NULL==rdcc->slot[idx]);
         rdcc->slot[idx] = ent;
         ent->idx = idx;
-        rdcc->nbytes += chunk_size;
+        rdcc->nbytes_used += chunk_size;
         rdcc->nused++;
 
         /* Add it to the linked list */
@@ -1951,7 +1951,7 @@ HDfprintf(stderr,"%s: buf=%p\n",FUNC,buf);
      * writing to other elements in the same chunk.  Do a direct
      * read-through of only the elements requested.
      */
-    if (dset->shared->dcpl_cache.pline.nused==0 && ((dset->shared->layout.u.chunk.size>dset->shared->cache.chunk.nbytes && chunk_addr!=HADDR_UNDEF)
+    if (dset->shared->dcpl_cache.pline.nused==0 && ((dset->shared->layout.u.chunk.size>dset->shared->cache.chunk.nbytes_max && chunk_addr!=HADDR_UNDEF)
             || (IS_H5FD_MPI(dset->ent.file) && (H5F_ACC_RDWR & H5F_get_intent(dset->ent.file))))) {
         H5D_io_info_t chk_io_info;      /* Temporary I/O info object */
         H5D_storage_t chk_store;        /* Chunk storage information */
@@ -2151,7 +2151,7 @@ HDfprintf(stderr,"%s: mem_offset_arr[%Zu]=%Hu\n",FUNC,*mem_curr_seq,mem_offset_a
     } /* end if */
 #endif /* H5_HAVE_PARALLEL */
 
-    if (dset->shared->dcpl_cache.pline.nused==0 && ((dset->shared->layout.u.chunk.size>dset->shared->cache.chunk.nbytes && chunk_addr!=HADDR_UNDEF)
+    if (dset->shared->dcpl_cache.pline.nused==0 && ((dset->shared->layout.u.chunk.size>dset->shared->cache.chunk.nbytes_max && chunk_addr!=HADDR_UNDEF)
             || (IS_H5FD_MPI(dset->ent.file) && (H5F_ACC_RDWR & H5F_get_intent(dset->ent.file))))) {
         H5D_io_info_t chk_io_info;      /* Temporary I/O info object */
         H5D_storage_t chk_store;        /* Chunk storage information */
diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h
index 3559a9e..9ea97e7 100644
--- a/src/H5Dpkg.h
+++ b/src/H5Dpkg.h
@@ -123,10 +123,11 @@ typedef struct H5D_rdcc_t {
     unsigned		nmisses;/* Number of cache misses		*/
     unsigned		nflushes;/* Number of cache flushes		*/
 #endif /* H5D_ISTORE_DEBUG */
-    size_t		nbytes;	/* Current cached raw data in bytes	*/
+    size_t		nbytes_max; /* Maximum cached raw data in bytes	*/
     size_t		nslots;	/* Number of chunk slots allocated	*/
     struct H5D_rdcc_ent_t *head; /* Head of doubly linked list		*/
     struct H5D_rdcc_ent_t *tail; /* Tail of doubly linked list		*/
+    size_t		nbytes_used; /* Current cached raw data in bytes */
     int		nused;	/* Number of chunk slots in use		*/
     struct H5D_rdcc_ent_t **slot; /* Chunk slots, each points to a chunk*/
 } H5D_rdcc_t;
@@ -332,6 +333,7 @@ H5_DLL htri_t H5D_mpio_opt_possible(const H5D_io_info_t *io_info, const H5S_t *m
 #ifdef H5D_TESTING
 H5_DLL herr_t H5D_layout_version_test(hid_t did, unsigned *version);
 H5_DLL herr_t H5D_layout_contig_size_test(hid_t did, hsize_t *size);
+H5_DLL herr_t H5D_current_cache_size_test(hid_t did, size_t *nbytes_used, int *nused);
 #endif /* H5D_TESTING */
 
 #endif /*_H5Dpkg_H*/
diff --git a/src/H5Dtest.c b/src/H5Dtest.c
index 630055d..0723b7d 100644
--- a/src/H5Dtest.c
+++ b/src/H5Dtest.c
@@ -108,3 +108,49 @@ done:
     FUNC_LEAVE_NOAPI(ret_value);
 }   /* H5D_layout_contig_size_test() */
 
+
+/*--------------------------------------------------------------------------
+ NAME
+    H5D_current_cache_size_test
+ PURPOSE
+    Determine current the size of the dataset's chunk cache
+ USAGE
+    herr_t H5D_layout_contig_size_test(did, size)
+        hid_t did;              IN: Dataset to query
+        hsize_t *size;          OUT: Pointer to location to place size info
+ RETURNS
+    Non-negative on success, negative on failure
+ DESCRIPTION
+    Checks the size of a contiguous dataset's storage.
+ GLOBAL VARIABLES
+ COMMENTS, BUGS, ASSUMPTIONS
+    DO NOT USE THIS FUNCTION FOR ANYTHING EXCEPT TESTING
+ EXAMPLES
+ REVISION LOG
+--------------------------------------------------------------------------*/
+herr_t
+H5D_current_cache_size_test(hid_t did, size_t *nbytes_used, int *nused)
+{
+    H5D_t	*dset;          /* Pointer to dataset to query */
+    herr_t ret_value = SUCCEED;   /* return value */
+
+    FUNC_ENTER_NOAPI(H5D_current_cache_size_test, FAIL)
+
+    /* Check args */
+    if(NULL == (dset = (H5D_t *)H5I_object_verify(did, H5I_DATASET)))
+        HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a dataset")
+
+    if(nbytes_used) {
+        HDassert(dset->shared->layout.type == H5D_CHUNKED);
+        *nbytes_used = dset->shared->cache.chunk.nbytes_used;
+    } /* end if */
+
+    if(nused) {
+        HDassert(dset->shared->layout.type == H5D_CHUNKED);
+        *nused = dset->shared->cache.chunk.nused;
+    } /* end if */
+
+done:
+    FUNC_LEAVE_NOAPI(ret_value)
+}   /* H5D_current_cache_size_test() */
+
diff --git a/test/tmisc.c b/test/tmisc.c
index 3e38b8f..8dd6561 100644
--- a/test/tmisc.c
+++ b/test/tmisc.c
@@ -286,6 +286,11 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2];          /* Data read from dataset
 #define MISC27_FILE             "tbad_msg_count.h5"
 #define MISC27_GROUP            "Group"
 
+/* Definitions for misc. test #28 */
+#define MISC28_FILE             "tmisc28.h5"
+#define MISC28_SIZE             10
+#define MISC28_NSLOTS           10000
+
 /****************************************************************
 **
 **  test_misc1(): test unlinking a dataset from a group and immediately
@@ -4247,6 +4252,183 @@ test_misc27(void)
     CHECK(ret, FAIL, "H5Fclose");
 } /* end test_misc27() */
 
+
+/****************************************************************
+**
+**  test_misc28(): Ensure that the dataset chunk cache will hold
+**                 the correct number of chunks in cache without
+**                 evicting them.
+**
+****************************************************************/
+static void
+test_misc28(void)
+{
+    hid_t   fid;            /* File ID */
+    hid_t   sidf;           /* File Dataspace ID */
+    hid_t   sidm;           /* Memory Dataspace ID */
+    hid_t   did;            /* Dataset ID */
+    hid_t   dcpl, fapl;     /* Property List IDs */
+    hsize_t dims[] = {MISC28_SIZE, MISC28_SIZE};
+    hsize_t mdims[] = {MISC28_SIZE};
+    hsize_t cdims[] = {1, 1};
+    hsize_t start[] = {0,0};
+    hsize_t count[] = {MISC28_SIZE, 1};
+    size_t  nbytes_used;
+    int     nused;
+    char    buf[MISC28_SIZE];
+    int     i;
+    herr_t  ret;            /* Generic return value */
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Dataset chunk cache\n"));
+
+    /* Create the fapl and set the cache size.  Set nelmts to larger than the
+     * file size so we can be guaranteed that no chunks will be evicted due to
+     * a hash collision.  Set nbytes to fit exactly 1 column of chunks (10
+     * bytes). */
+    fapl = H5Pcreate(H5P_FILE_ACCESS);
+    CHECK(fapl, FAIL, "H5Pcreate");
+    ret = H5Pset_cache(fapl, MISC28_NSLOTS, MISC28_NSLOTS, MISC28_SIZE, 0.75);
+    CHECK(ret, FAIL, "H5Pset_cache");
+
+    /* Create the dcpl and set the chunk size */
+    dcpl = H5Pcreate(H5P_DATASET_CREATE);
+    CHECK(dcpl, FAIL, "H5Pcreate");
+    ret = H5Pset_chunk(dcpl, 2, cdims);
+    CHECK(ret, FAIL, "H5Pset_chunk");
+
+
+    /* Create a new file and datasets within that file that use these
+     * property lists
+     */
+    fid = H5Fcreate(MISC28_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, fapl);
+    CHECK(fid, FAIL, "H5Fcreate");
+
+    sidf = H5Screate_simple(2, dims, NULL);
+    CHECK(sidf, FAIL, "H5Screate_simple");
+
+    did = H5Dcreate(fid, "dataset", H5T_NATIVE_CHAR, sidf, dcpl);
+    CHECK(did, FAIL, "H5Dcreate2");
+
+    /* Verify that the chunk cache is empty */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) 0, "H5D_current_cache_size_test");
+    VERIFY(nused, 0, "H5D_current_cache_size_test");
+
+    /* Initialize write buffer */
+    for(i=0; i<MISC28_SIZE; i++)
+        buf[i] = i;
+
+    /* Create memory dataspace and selection in file dataspace */
+    sidm = H5Screate_simple(1, mdims, NULL);
+    CHECK(sidm, FAIL, "H5Screate_simple");
+
+    ret = H5Sselect_hyperslab(sidf, H5S_SELECT_SET, start, NULL, count, NULL);
+    CHECK(ret, FAIL, "H5Sselect_hyperslab");
+
+    /* Write hypserslab */
+    ret = H5Dwrite(did, H5T_NATIVE_CHAR, sidm, sidf, H5P_DEFAULT, buf);
+    CHECK(ret, FAIL, "H5Dwrite");
+
+    /* Verify that all 10 chunks written have been cached */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) MISC28_SIZE, "H5D_current_cache_size_test");
+    VERIFY(nused, MISC28_SIZE, "H5D_current_cache_size_test");
+
+    /* Initialize write buffer */
+    for(i=0; i<MISC28_SIZE; i++)
+        buf[i] = MISC28_SIZE - 1 - i;
+
+    /* Select new hyperslab */
+    start[1] = 1;
+    ret = H5Sselect_hyperslab(sidf, H5S_SELECT_SET, start, NULL, count, NULL);
+    CHECK(ret, FAIL, "H5Sselect_hyperslab");
+
+    /* Write hyperslab */
+    ret = H5Dwrite(did, H5T_NATIVE_CHAR, sidm, sidf, H5P_DEFAULT, buf);
+    CHECK(ret, FAIL, "H5Dwrite");
+
+    /* Verify that the size of the cache remains at 10 */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) MISC28_SIZE, "H5D_current_cache_size_test");
+    VERIFY(nused, MISC28_SIZE, "H5D_current_cache_size_test");
+
+    /* Close dataset */
+    ret = H5Dclose(did);
+    CHECK(ret, FAIL, "H5Dclose");
+
+
+    /* Re open dataset */
+    did = H5Dopen(fid, "dataset");
+    CHECK(did, FAIL, "H5Dopen2");
+
+    /* Verify that the chunk cache is empty */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) 0, "H5D_current_cache_size_test");
+    VERIFY(nused, 0, "H5D_current_cache_size_test");
+
+    /* Select hyperslabe for reading */
+    start[1] = 0;
+    ret = H5Sselect_hyperslab(sidf, H5S_SELECT_SET, start, NULL, count, NULL);
+    CHECK(ret, FAIL, "H5Sselect_hyperslab");
+
+    /* Read hypserslab */
+    ret = H5Dread(did, H5T_NATIVE_CHAR, sidm, sidf, H5P_DEFAULT, buf);
+    CHECK(ret, FAIL, "H5Dread");
+
+    /* Verify the data read */
+    for(i=0; i<MISC28_SIZE; i++)
+        VERIFY(buf[i], i, "H5Dread");
+
+    /* Verify that all 10 chunks read have been cached */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) MISC28_SIZE, "H5D_current_cache_size_test");
+    VERIFY(nused, MISC28_SIZE, "H5D_current_cache_size_test");
+
+    /* Select new hyperslab */
+    start[1] = 1;
+    ret = H5Sselect_hyperslab(sidf, H5S_SELECT_SET, start, NULL, count, NULL);
+    CHECK(ret, FAIL, "H5Sselect_hyperslab");
+
+    /* Read hyperslab */
+    ret = H5Dread(did, H5T_NATIVE_CHAR, sidm, sidf, H5P_DEFAULT, buf);
+    CHECK(ret, FAIL, "H5Dread");
+
+    /* Verify the data read */
+    for(i=0; i<MISC28_SIZE; i++)
+        VERIFY(buf[i], MISC28_SIZE - 1 - i, "H5Dread");
+
+    /* Verify that the size of the cache remains at 10 */
+    ret = H5D_current_cache_size_test(did, &nbytes_used, &nused);
+    CHECK(ret, FAIL, "H5D_current_cache_size_test");
+    VERIFY(nbytes_used, (size_t) MISC28_SIZE, "H5D_current_cache_size_test");
+    VERIFY(nused, MISC28_SIZE, "H5D_current_cache_size_test");
+
+    /* Close dataset */
+    ret = H5Dclose(did);
+    CHECK(ret, FAIL, "H5Dclose");
+
+
+    /* Close the dataspaces and file */
+    ret = H5Sclose(sidf);
+    CHECK_I(ret, "H5Sclose");
+    ret = H5Sclose(sidm);
+    CHECK_I(ret, "H5Sclose");
+    ret = H5Fclose(fid);
+    CHECK_I(ret, "H5Fclose");
+
+    /* Close the property lists.  */
+    ret = H5Pclose(dcpl);
+    CHECK_I(ret, "H5Pclose");
+    ret = H5Pclose(fapl);
+    CHECK_I(ret, "H5Pclose");
+} /* end test_misc28() */
+
 /****************************************************************
 **
 **  test_misc(): Main misc. test routine.
@@ -4288,6 +4470,7 @@ test_misc(void)
     test_misc25b();     /* Exercise null object header message merge bug on existing file */
     /* misc. test #26 only in 1.8 branch/trunk */
     test_misc27();      /* Test opening file with object that has bad # of object header messages */
+    test_misc28();      /* Test that chunks are cached appropriately */
 
 } /* test_misc() */
 
@@ -4338,5 +4521,6 @@ cleanup_misc(void)
 #endif /* H5_HAVE_FILTER_SZIP */
     HDremove(MISC24_FILE);
     HDremove(MISC25A_FILE);
+    HDremove(MISC28_FILE);
 }