[svn-r13744] Description:

    Initial changes to take advantage of readers/writer locking on metadata
cache entries.  Reduce the # of protect/unprotect pairs during B-tree iteration
by holding reader lock on B-tree and symbol table nodes while iteration occurs.

    This also has the benefit of preventing an iteration callback from
modifying the B-tree being iterated over (which would probably confuse the
B-tree iteration code terribly)

Tested on:
    Mac OS X/32 10.4.9 (amazon)
    Linux/32 2.6 (chicago)
    Linux/64 2.6 (chicago2)

2 files changed, 68 insertions, 115 deletions

diff --git a/src/H5B.c b/src/H5B.c
index 5f847a8..ddf4fc5 100644
--- a/src/H5B.c
+++ b/src/H5B.c
@@ -1172,95 +1172,91 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5B_iterate (H5F_t *f, hid_t dxpl_id, const H5B_class_t *type, H5B_operator_t op, haddr_t addr, void *udata)
+H5B_iterate(H5F_t *f, hid_t dxpl_id, const H5B_class_t *type, H5B_operator_t op, haddr_t addr, void *udata)
 {
-    H5B_t		*bt = NULL;
-    H5B_shared_t        *shared;        /* Pointer to shared B-tree info */
-    haddr_t		next_addr;
-    haddr_t		cur_addr = HADDR_UNDEF;
-    haddr_t		*child = NULL;
-    uint8_t		*key = NULL;
-    unsigned		nchildren;      /* Number of children of B-tree node */
-    unsigned		u;              /* Local index variable */
-    unsigned            level;
-    haddr_t             left_child;
-    herr_t		ret_value;
+    H5B_t		*bt = NULL;     /* Pointer to current B-tree node */
+    herr_t		ret_value;      /* Return value */
 
     FUNC_ENTER_NOAPI(H5B_iterate, FAIL)
 
     /*
      * Check arguments.
      */
-    assert(f);
-    assert(type);
-    assert(op);
-    assert(H5F_addr_defined(addr));
-    assert(udata);
-
-    if (NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, addr, type, udata, H5AC_READ)))
+    HDassert(f);
+    HDassert(type);
+    HDassert(op);
+    HDassert(H5F_addr_defined(addr));
+    HDassert(udata);
+
+    /* Protect the initial/current node */
+    if(NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, addr, type, udata, H5AC_READ)))
 	HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "unable to load B-tree node")
-    shared=H5RC_GET_OBJ(bt->rc_shared);
-    HDassert(shared);
 
-    level = bt->level;
-    left_child = bt->child[0];
-
-    if (H5AC_unprotect(f, dxpl_id, H5AC_BT, addr, bt, H5AC__NO_FLAGS_SET) < 0)
-        HGOTO_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node")
-    bt = NULL;  /* Make certain future references will be caught */
-
-    if (level > 0) {
+    if(bt->level > 0) {
 	/* Keep following the left-most child until we reach a leaf node. */
-	if ((ret_value=H5B_iterate(f, dxpl_id, type, op, left_child, udata))<0)
+	if((ret_value = H5B_iterate(f, dxpl_id, type, op, bt->child[0], udata)) < 0)
 	    HGOTO_ERROR(H5E_BTREE, H5E_CANTLIST, FAIL, "unable to list B-tree node")
-    } else {
+    } /* end if */
+    else {
 	/*
 	 * We've reached the left-most leaf.  Now follow the right-sibling
 	 * pointer from leaf to leaf until we've processed all leaves.
 	 */
-	if (NULL==(child=H5FL_SEQ_MALLOC(haddr_t,(size_t)shared->two_k)) ||
-                NULL==(key=H5FL_BLK_MALLOC(native_block,shared->sizeof_keys)))
-	    HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
-
-	for (cur_addr=addr, ret_value=0; H5F_addr_defined(cur_addr) && !ret_value; cur_addr=next_addr) {
-	    /*
-	     * Save all the child addresses and native keys since we can't
-	     * leave the B-tree node protected during an application
-	     * callback.
-	     */
-	    if (NULL == (bt = H5AC_protect(f, dxpl_id, H5AC_BT, cur_addr, type, udata, H5AC_READ)))
-		HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "B-tree node")
-
-            HDmemcpy(child, bt->child, bt->nchildren*sizeof(haddr_t));
-            HDmemcpy(key, bt->native, shared->sizeof_keys);
-
-	    next_addr = bt->right;
-	    nchildren = bt->nchildren;
-
-            if (H5AC_unprotect(f, dxpl_id, H5AC_BT, cur_addr, bt, H5AC__NO_FLAGS_SET) < 0)
-                HGOTO_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node")
-	    bt = NULL;
+        ret_value = H5_ITER_CONT;
+	while(bt && ret_value == H5_ITER_CONT) {
+            haddr_t	*child;         /* Pointer to node's child addresses */
+            uint8_t	*key;           /* Pointer to node's native keys */
+            unsigned	u;              /* Local index variable */
 
 	    /*
 	     * Perform the iteration operator, which might invoke an
 	     * application callback.
 	     */
-	    for(u = 0, ret_value = H5_ITER_CONT; u < nchildren && !ret_value; u++) {
-		ret_value = (*op)(f, dxpl_id, key + (u * type->sizeof_nkey),
-                         child[u], key + ((u + 1) * type->sizeof_nkey), udata);
+	    for(u = 0, child = bt->child, key = bt->native; u < bt->nchildren && ret_value == H5_ITER_CONT; u++, child++, key += type->sizeof_nkey) {
+		ret_value = (*op)(f, dxpl_id, key, *child, key + type->sizeof_nkey, udata);
 		if(ret_value < 0)
                     HERROR(H5E_BTREE, H5E_CANTLIST, "iterator function failed");
 	    } /* end for */
+
+            /* Check for continuing iteration */
+            if(ret_value == H5_ITER_CONT) {
+                H5B_t	*next_bt;       /* Pointer to next B-tree node */
+                haddr_t	next_addr;      /* Address of next node to iterate over */
+
+                /* Protect the next node to the right, if there is one */
+                if(H5F_addr_defined(bt->right)) {
+                    next_addr = bt->right;
+                    if(NULL == (next_bt = H5AC_protect(f, dxpl_id, H5AC_BT, next_addr, type, udata, H5AC_READ)))
+                        HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, FAIL, "B-tree node")
+                } /* end if */
+                else {
+                    next_addr = HADDR_UNDEF;
+                    next_bt = NULL;
+                } /* end if */
+
+                /* Unprotect this node */
+                if(H5AC_unprotect(f, dxpl_id, H5AC_BT, addr, bt, H5AC__NO_FLAGS_SET) < 0) {
+                    if(next_bt) {
+                        HDassert(H5F_addr_defined(next_addr));
+                        if(H5AC_unprotect(f, dxpl_id, H5AC_BT, next_addr, next_bt, H5AC__NO_FLAGS_SET) < 0)
+                            HDONE_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node")
+                    } /* end if */
+                    HGOTO_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node")
+                } /* end if */
+
+                /* Advance to the next node */
+                bt = next_bt;
+                addr = next_addr;
+            } /* end if */
 	} /* end for */
     } /* end else */
 
 done:
-    if(child!=NULL)
-        H5FL_SEQ_FREE(haddr_t,child);
-    if(key!=NULL)
-        H5FL_BLK_FREE(native_block,key);
+    if(bt && H5AC_unprotect(f, dxpl_id, H5AC_BT, addr, bt, H5AC__NO_FLAGS_SET) < 0)
+        HDONE_ERROR(H5E_BTREE, H5E_PROTECT, FAIL, "unable to release B-tree node")
+
     FUNC_LEAVE_NOAPI(ret_value)
-}
+} /* end H5B_iterate() */
 
 
 /*-------------------------------------------------------------------------
diff --git a/src/H5Gnode.c b/src/H5Gnode.c
index 9486e1a..59e31f9 100644
--- a/src/H5Gnode.c
+++ b/src/H5Gnode.c
@@ -1418,12 +1418,9 @@ H5G_node_iterate(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t ad
     H5G_bt_it_it_t	*udata = (H5G_bt_it_it_t *)_udata;
     H5G_node_t		*sn = NULL;
     H5HL_t              *heap = NULL;
-    char		buf[1024], *s = buf;    /* Buffer for link name & pointer to link name */
-    size_t              buf_size = sizeof(buf); /* Size of link name buffer */
-    unsigned		nsyms;                  /* # of symbols in node */
-    H5G_entry_t		*ents = NULL;           /* Copy of entries in this node */
+    H5G_entry_t		*ents;                  /* Pointer to entries in this node */
     unsigned		u;                      /* Local index variable */
-    int	                ret_value;
+    int	                ret_value = H5_ITER_CONT;
 
     FUNC_ENTER_NOAPI(H5G_node_iterate, H5_ITER_ERROR)
 
@@ -1434,63 +1431,30 @@ H5G_node_iterate(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t ad
     HDassert(H5F_addr_defined(addr));
     HDassert(udata);
 
-    /*
-     * Save information about the symbol table node since we can't lock it
-     * because we're about to call an application function.
-     */
+    /* Protect the symbol table node & local heap while we iterate over entries */
     if(NULL == (sn = H5AC_protect(f, dxpl_id, H5AC_SNODE, addr, NULL, NULL, H5AC_READ)))
 	HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, H5_ITER_ERROR, "unable to load symbol table node")
-    nsyms = sn->nsyms;
-    if(NULL == (ents = H5FL_SEQ_MALLOC(H5G_entry_t, (size_t)nsyms)))
-	HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, H5_ITER_ERROR, "memory allocation failed")
-    HDmemcpy(ents, sn->entry, sizeof(H5G_entry_t) * nsyms);
-
-    if(H5AC_unprotect(f, dxpl_id, H5AC_SNODE, addr, sn, H5AC__NO_FLAGS_SET) != SUCCEED) {
-        sn = NULL;
-        HGOTO_ERROR(H5E_SYM, H5E_PROTECT, H5_ITER_ERROR, "unable to release object header")
-    } /* end if */
-    sn = NULL;    /* Make certain future references will be caught */
+    if(NULL == (heap = H5HL_protect(f, dxpl_id, udata->heap_addr, H5AC_READ)))
+        HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, H5_ITER_ERROR, "unable to protect symbol name")
 
     /*
      * Iterate over the symbol table node entries.
      */
-    for(u = 0, ret_value = H5_ITER_CONT; u < nsyms && !ret_value; u++) {
+    for(u = 0, ents = sn->entry; u < sn->nsyms && ret_value == H5_ITER_CONT; u++) {
         if(udata->skip > 0)
             --udata->skip;
         else {
-            size_t		n;      /* Length of link name */
             const char		*name;  /* Pointer to link name in heap */
 
-            if(NULL == (heap = H5HL_protect(f, dxpl_id, udata->heap_addr, H5AC_READ)))
-                HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, H5_ITER_ERROR, "unable to protect symbol name")
-
+            /* Get the pointer to the name of the link in the heap */
             name = H5HL_offset_into(f, heap, ents[u].name_off);
             HDassert(name);
-            n = HDstrlen(name);
-
-            /* Allocate space or point to existing buffer */
-            if((n + 1) > sizeof(buf)) {
-                if(s != buf)
-                    H5MM_xfree(s);
-                if(NULL == (s = H5MM_malloc(n + 1)))
-                    HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, H5_ITER_ERROR, "memory allocation failed")
-                buf_size = n + 1;
-            } /* end if */
-            else
-                s = buf;
-
-            /* Make a copy of the name */
-            HDstrcpy(s, name);
-
-            if(H5HL_unprotect(f, dxpl_id, heap, udata->heap_addr, H5AC__NO_FLAGS_SET) < 0)
-                HGOTO_ERROR(H5E_SYM, H5E_PROTECT, H5_ITER_ERROR, "unable to unprotect symbol name")
-            heap = NULL; name = NULL;
 
             /* Check which type of callback to make */
             switch(udata->lnk_op->op_type) {
                 case H5G_LINK_OP_OLD:
                     /* Make the old-type application callback */
-                    ret_value = (udata->lnk_op->u.old_op)(udata->group_id, s, udata->op_data);
+                    ret_value = (udata->lnk_op->u.old_op)(udata->group_id, name, udata->op_data);
                     break;
 
                 case H5G_LINK_OP_APP:
@@ -1502,7 +1466,7 @@ H5G_node_iterate(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t ad
                             HGOTO_ERROR(H5E_SYM, H5E_CANTGET, H5_ITER_ERROR, "unable to get info for symbol table entry")
 
                         /* Make the application callback */
-                        ret_value = (udata->lnk_op->u.app_op)(udata->group_id, s, &info, udata->op_data);
+                        ret_value = (udata->lnk_op->u.app_op)(udata->group_id, name, &info, udata->op_data);
                     }
                     break;
 
@@ -1512,7 +1476,7 @@ H5G_node_iterate(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t ad
                         H5O_link_t lnk;         /* Link for entry */
 
                         /* Convert the entry to a link */
-                        if(H5G_ent_to_link(f, dxpl_id, &lnk, udata->heap_addr, NULL, &ents[u], s) < 0)
+                        if(H5G_ent_to_link(f, dxpl_id, &lnk, udata->heap_addr, NULL, &ents[u], name) < 0)
                             HGOTO_ERROR(H5E_SYM, H5E_CANTCONVERT, H5_ITER_ERROR, "unable to convert symbol table entry to link")
 
                         /* Call the library's callback */
@@ -1534,19 +1498,12 @@ H5G_node_iterate(H5F_t *f, hid_t dxpl_id, const void UNUSED *_lt_key, haddr_t ad
         HERROR(H5E_SYM, H5E_CANTNEXT, "iteration operator failed");
 
 done:
-    /* Free the memory for the name, if we used a dynamically allocated buffer */
-    if(s != buf)
-        H5MM_xfree(s);
-
+    /* Release resources */
     if(heap && H5HL_unprotect(f, dxpl_id, heap, udata->heap_addr, H5AC__NO_FLAGS_SET) < 0)
         HDONE_ERROR(H5E_SYM, H5E_PROTECT, H5_ITER_ERROR, "unable to unprotect symbol name")
-
     if(sn && H5AC_unprotect(f, dxpl_id, H5AC_SNODE, addr, sn, H5AC__NO_FLAGS_SET) != SUCCEED)
         HDONE_ERROR(H5E_SYM, H5E_PROTECT, H5_ITER_ERROR, "unable to release object header")
 
-    if(ents)
-        H5FL_SEQ_FREE(H5G_entry_t, ents);
-
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5G_node_iterate() */