From d88239752cdeacfac8858439334599b3ec471803 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Thu, 27 Sep 2018 16:07:10 +0200 Subject: erts: Improve deallocation of CATree nodes Update table memory stats before scheduling free to not be dependent on deallocation order with main table struct. --- erts/emulator/beam/erl_db.c | 31 +++------ erts/emulator/beam/erl_db.h | 26 +++++++ erts/emulator/beam/erl_db_catree.c | 135 ++++++++++++++++++++----------------- erts/emulator/beam/erl_db_catree.h | 2 - erts/emulator/beam/erl_db_hash.c | 10 +-- 5 files changed, 113 insertions(+), 91 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/erl_db.c b/erts/emulator/beam/erl_db.c index 1df972f4b6..3653c0bf7c 100644 --- a/erts/emulator/beam/erl_db.c +++ b/erts/emulator/beam/erl_db.c @@ -409,30 +409,17 @@ static void free_dbtable(void *vtb) { DbTable *tb = (DbTable *) vtb; -#ifdef HARDDEBUG - if (erts_atomic_read_nob(&tb->common.memory_size) != sizeof(DbTable)) { - erts_fprintf(stderr, "ets: free_dbtable memory remain=%ld fix=%x\n", - erts_atomic_read_nob(&tb->common.memory_size)-sizeof(DbTable), - tb->common.fixations); - } -#endif - if (erts_atomic_read_nob(&tb->common.memory_size) > sizeof(DbTable)) { - /* The CA tree implementation use delayed freeing and the DbTable needs to - be freed after all other memory blocks that are allocated by the table. */ - erts_schedule_thr_prgr_later_cleanup_op(free_dbtable, - (void *) tb, - &tb->release.data, - sizeof(DbTable)); - return; - } - erts_rwmtx_destroy(&tb->common.rwlock); - erts_mtx_destroy(&tb->common.fixlock); - ASSERT(is_immed(tb->common.heir_data)); - if (tb->common.btid) - erts_bin_release(tb->common.btid); + ASSERT(erts_atomic_read_nob(&tb->common.memory_size) == sizeof(DbTable)); + + erts_rwmtx_destroy(&tb->common.rwlock); + erts_mtx_destroy(&tb->common.fixlock); + ASSERT(is_immed(tb->common.heir_data)); + + if (tb->common.btid) + erts_bin_release(tb->common.btid); - erts_db_free(ERTS_ALC_T_DB_TABLE, tb, (void *) tb, sizeof(DbTable)); + erts_db_free(ERTS_ALC_T_DB_TABLE, tb, (void *) tb, sizeof(DbTable)); } static void schedule_free_dbtable(DbTable* tb) diff --git a/erts/emulator/beam/erl_db.h b/erts/emulator/beam/erl_db.h index 45d120ac0e..7a915ccea2 100644 --- a/erts/emulator/beam/erl_db.h +++ b/erts/emulator/beam/erl_db.h @@ -286,6 +286,12 @@ ERTS_GLB_INLINE void erts_db_free(ErtsAlcType_t type, void *ptr, Uint size); +ERTS_GLB_INLINE void erts_schedule_db_free(DbTableCommon* tab, + void (*free_func)(void *), + void *ptr, + ErtsThrPrgrLaterOp *lop, + Uint size); + ERTS_GLB_INLINE void erts_db_free_nt(ErtsAlcType_t type, void *ptr, Uint size); @@ -305,6 +311,26 @@ erts_db_free(ErtsAlcType_t type, DbTable *tab, void *ptr, Uint size) erts_free(type, ptr); } +ERTS_GLB_INLINE void +erts_schedule_db_free(DbTableCommon* tab, + void (*free_func)(void *), + void *ptr, + ErtsThrPrgrLaterOp *lop, + Uint size) +{ + ASSERT(ptr != 0); + ASSERT(((void *) tab) != ptr); + ASSERT(size == ERTS_ALC_DBG_BLK_SZ(ptr)); + + /* + * We update table memory stats here as table may already be gone + * when 'free_func' is finally called. + */ + ERTS_DB_ALC_MEM_UPDATE_((DbTable*)tab, size, 0); + + erts_schedule_thr_prgr_later_cleanup_op(free_func, ptr, lop, size); +} + ERTS_GLB_INLINE void erts_db_free_nt(ErtsAlcType_t type, void *ptr, Uint size) { diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 3a5b32606d..71768adcda 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -804,10 +804,15 @@ static DbTableCATreeNode *create_catree_base_node(DbTableCATree *tb) "erl_db_catree_base_node", tb->common.the_name, ERTS_LOCK_FLAGS_CATEGORY_DB); new_base_node->lock_statistics = 0; new_base_node->is_valid = 1; - new_base_node->tab = (DbTable *) tb; return new_base_node_container; } +static ERTS_INLINE Uint sizeof_route_node(Uint key_size) +{ + return (offsetof(DbTableCATreeNode, u.route.key.tpl[1]) + + key_size*sizeof(Eterm)); +} + static DbTableCATreeNode* create_catree_route_node(DbTableCommon * common_table_data, DbTableCATreeNode *left, @@ -832,7 +837,6 @@ create_catree_route_node(DbTableCommon * common_table_data, (DbTableCATreeNode*)new_route_node_container_bytes; DbTableCATreeRouteNode *new_route_node = &new_route_node_container->u.route; - new_route_node->tab = (DbTable *)common_table_data; if (key_size != 0) { newp->size = key_size; top = &newp->tpl[1]; @@ -853,45 +857,42 @@ create_catree_route_node(DbTableCommon * common_table_data, return new_route_node_container; } -static void free_catree_base_node(void* base_node_container_ptr) +static void do_free_base_node(void* vptr) { - DbTableCATreeNode *base_node_container = - (DbTableCATreeNode *)base_node_container_ptr; - DbTableCATreeBaseNode *base_node = - &base_node_container->u.base; - erts_rwmtx_destroy(&base_node->lock); - erts_db_free(ERTS_ALC_T_DB_TABLE, - base_node->tab, base_node_container, - sizeof(DbTableCATreeNode)); -} - -static void free_catree_routing_node(void *route_node_container_ptr) -{ - size_t route_node_container_size; - byte* route_node_container_bytes = route_node_container_ptr; - DbTableCATreeNode *route_node_container = - (DbTableCATreeNode *)route_node_container_bytes; - DbTableCATreeRouteNode *route_node = - &route_node_container->u.route; - int key_size = route_node->key.size; - Uint offset = offsetof(DbTableCATreeNode,u.route.key); - ErlOffHeap tmp_oh; - DbTerm* db_term = (DbTerm*) (route_node_container_bytes + offset); - erts_mtx_destroy(&route_node->lock); - route_node_container_size = - offset + - sizeof(DbTerm) + - sizeof(Eterm)*key_size; - if (key_size != 0) { - tmp_oh.first = db_term->first_oh; + DbTableCATreeNode *p = (DbTableCATreeNode *)vptr; + ASSERT(p->is_base_node); + erts_rwmtx_destroy(&p->u.base.lock); + erts_free(ERTS_ALC_T_DB_TABLE, p); +} + +static void free_catree_base_node(DbTableCATree* tb, DbTableCATreeNode* p) +{ + ASSERT(p->is_base_node); + ERTS_DB_ALC_MEM_UPDATE_(tb, sizeof(DbTableCATreeNode), 0); + do_free_base_node(p); +} + +static void do_free_route_node(void *vptr) +{ + DbTableCATreeNode *p = (DbTableCATreeNode *)vptr; + ASSERT(!p->is_base_node); + erts_mtx_destroy(&p->u.route.lock); + if (p->u.route.key.size != 0) { + ErlOffHeap tmp_oh; + tmp_oh.first = p->u.route.key.first_oh; erts_cleanup_offheap(&tmp_oh); } - erts_db_free(ERTS_ALC_T_DB_TABLE, - route_node->tab, - route_node_container, - route_node_container_size); + erts_free(ERTS_ALC_T_DB_TABLE, p); } +static void free_catree_route_node(DbTableCATree* tb, DbTableCATreeNode* p) +{ + ASSERT(!p->is_base_node); + ERTS_DB_ALC_MEM_UPDATE_(tb, sizeof_route_node(p->u.route.key.size), 0); + do_free_route_node(p); +} + + /* * Returns the parent routing node of the specified * route_node_container if such a routing node exists or NULL if @@ -1272,16 +1273,22 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, wunlock_base_node(base); wunlock_base_node(neighbor_base); /* Free the parent and base */ - erts_schedule_thr_prgr_later_op(free_catree_routing_node, - parent_container, - &parent->free_item); - erts_schedule_thr_prgr_later_op(free_catree_base_node, - base_container, - &base->free_item); - erts_schedule_thr_prgr_later_op(free_catree_base_node, - neighbor_base_container, - &neighbor_base->free_item); - + erts_schedule_db_free(common_table_data, + do_free_route_node, + parent_container, + &parent->free_item, + sizeof_route_node(parent->key.size)); + erts_schedule_db_free(common_table_data, + do_free_base_node, + base_container, + &base->free_item, + sizeof(DbTableCATreeNode)); + erts_schedule_db_free(common_table_data, + do_free_base_node, + neighbor_base_container, + &neighbor_base->free_item, + sizeof(DbTableCATreeNode)); + if (parent_container == neighbor_base_container) { *result_parent_wb = gparent_container; } else { @@ -1470,15 +1477,21 @@ static void join_catree(DbTableCATree *tb, wunlock_base_node(base); wunlock_base_node(neighbor_base); /* Free the parent and base */ - erts_schedule_thr_prgr_later_op(free_catree_routing_node, - parent_container, - &parent->free_item); - erts_schedule_thr_prgr_later_op(free_catree_base_node, - base_container, - &base->free_item); - erts_schedule_thr_prgr_later_op(free_catree_base_node, - neighbor_base_container, - &neighbor_base->free_item); + erts_schedule_db_free(&tb->common, + do_free_route_node, + parent_container, + &parent->free_item, + sizeof_route_node(parent->key.size)); + erts_schedule_db_free(&tb->common, + do_free_base_node, + base_container, + &base->free_item, + sizeof(DbTableCATreeNode)); + erts_schedule_db_free(&tb->common, + do_free_base_node, + neighbor_base_container, + &neighbor_base->free_item, + sizeof(DbTableCATreeNode)); } @@ -1530,9 +1543,11 @@ static void split_catree(DbTableCommon *tb, } base->is_valid = 0; wunlock_base_node(base); - erts_schedule_thr_prgr_later_op(free_catree_base_node, - base_container, - &base->free_item); + erts_schedule_db_free(tb, + do_free_base_node, + base_container, + &base->free_item, + sizeof(DbTableCATreeNode)); } } @@ -1594,7 +1609,7 @@ static SWord do_free_routing_nodes_catree_cont(DbTableCATree *tb, SWord num_left PUSH_NODE(&tb->free_stack_rnodes, root); root = p; } else { - free_catree_routing_node(root); + free_catree_route_node(tb, root); if (--num_left >= 0) { break; } else { @@ -1635,7 +1650,7 @@ static SWord do_free_base_node_cont(DbTableCATree *tb, SWord num_left) } } catree_deque_base_node_from_free_list(tb); - free_catree_base_node(base_node_container); + free_catree_base_node(tb, base_node_container); base_node_container = catree_first_base_node_from_free_list(tb); if (base_node_container != NULL) { PUSH_NODE(&tb->free_stack_elems, base_node_container->u.base.root); diff --git a/erts/emulator/beam/erl_db_catree.h b/erts/emulator/beam/erl_db_catree.h index 406f3bb0f2..4d3e75bfe0 100644 --- a/erts/emulator/beam/erl_db_catree.h +++ b/erts/emulator/beam/erl_db_catree.h @@ -38,14 +38,12 @@ typedef struct { Sint lock_statistics; int is_valid; /* If this base node is still valid */ TreeDbTerm *root; /* The root of the sequential tree */ - DbTable * tab; /* Table ptr, used when freeing using thread progress */ ErtsThrPrgrLaterOp free_item; /* Used when freeing using thread progress */ struct DbTableCATreeNode * next; /* Used when gradually deleting */ } DbTableCATreeBaseNode; typedef struct { ErtsThrPrgrLaterOp free_item; /* Used when freeing using thread progress */ - DbTable* tab; /* Table ptr, used when freeing using thread progress */ erts_mtx_t lock; /* Used when joining route nodes */ int is_valid; /* If this route node is still valid */ erts_atomic_t left; diff --git a/erts/emulator/beam/erl_db_hash.c b/erts/emulator/beam/erl_db_hash.c index 752d3ae3a8..61806876a7 100644 --- a/erts/emulator/beam/erl_db_hash.c +++ b/erts/emulator/beam/erl_db_hash.c @@ -2731,13 +2731,9 @@ static int free_seg(DbTableHash *tb, int free_records) * sure no lingering threads are still hanging in BUCKET macro * with an old segtab pointer. */ - Uint sz = SIZEOF_EXT_SEGTAB(est->nsegs); - ASSERT(sz == ERTS_ALC_DBG_BLK_SZ(est)); - ERTS_DB_ALC_MEM_UPDATE_(tb, sz, 0); - erts_schedule_thr_prgr_later_cleanup_op(dealloc_ext_segtab, - est, - &est->lop, - sz); + erts_schedule_db_free(&tb->common, dealloc_ext_segtab, + est, &est->lop, + SIZEOF_EXT_SEGTAB(est->nsegs)); } else erts_db_free(ERTS_ALC_T_DB_SEG, (DbTable*)tb, est, -- cgit v1.2.3