From c751cdab335b24e069570dcec8a39856dfa2cd98 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Thu, 10 May 2007 10:31:34 -0500 Subject: [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) --- src/H5B.c | 118 ++++++++++++++++++++++++++++------------------------------ src/H5Gnode.c | 65 ++++++-------------------------- 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() */ -- cgit v0.12