From 1c4443d3e01ecea0c4e59dbbf055a704e30cd672 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Thu, 27 Sep 2018 15:54:51 +0200 Subject: erts: Fix compiler warning in erl_bif_binary.c --- erts/emulator/beam/erl_bif_binary.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_bif_binary.c b/erts/emulator/beam/erl_bif_binary.c index ff919082c3..5b3f091ccc 100644 --- a/erts/emulator/beam/erl_bif_binary.c +++ b/erts/emulator/beam/erl_bif_binary.c @@ -471,6 +471,9 @@ static BMData *create_bmdata(MyAllocator *my, byte *x, Uint len, Binary **the_bin /* out */) { Uint datasize; + BMData *bmd; + Binary *mb; + byte *data; if(len > 1) { datasize = BM_SIZE_MULTI(len); @@ -478,9 +481,8 @@ static BMData *create_bmdata(MyAllocator *my, byte *x, Uint len, datasize = BM_SIZE_SINGLE(); } - BMData *bmd; - Binary *mb = erts_create_magic_binary(datasize,cleanup_my_data_bm); - byte *data = ERTS_MAGIC_BIN_DATA(mb); + mb = erts_create_magic_binary(datasize,cleanup_my_data_bm); + data = ERTS_MAGIC_BIN_DATA(mb); init_my_allocator(my, datasize, data); bmd = my_alloc(my, sizeof(BMData)); bmd->x = my_alloc(my,len); -- cgit v1.2.3 From 4a67d1a104193ca0f5a0dc3f3dfc0e2b6df61f2f Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 26 Sep 2018 20:42:30 +0200 Subject: erts: Refactor rename union in DbTableCATreeNode u as in union --- erts/emulator/beam/erl_db_catree.c | 160 ++++++++++++++++++------------------- erts/emulator/beam/erl_db_catree.h | 2 +- 2 files changed, 80 insertions(+), 82 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 37a299df35..3a5b32606d 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -210,29 +210,29 @@ DbTableMethod db_catree = * Internal CA tree related helper functions and macros */ -#define GET_ROUTE_NODE_KEY(node) (node->baseOrRoute.route.key.tpl[0]) -#define GET_BASE_NODE_LOCK(node) (&(node->baseOrRoute.base.lock)) -#define GET_ROUTE_NODE_LOCK(node) (&(node->baseOrRoute.route.lock)) +#define GET_ROUTE_NODE_KEY(node) (node->u.route.key.tpl[0]) +#define GET_BASE_NODE_LOCK(node) (&(node->u.base.lock)) +#define GET_ROUTE_NODE_LOCK(node) (&(node->u.route.lock)) /* Helpers for reading and writing shared atomic variables */ /* No memory barrier */ #define GET_ROOT(tb) ((DbTableCATreeNode*)erts_atomic_read_nob(&(tb->root))) -#define GET_LEFT(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_nob(&(ca_tree_route_node->baseOrRoute.route.left))) -#define GET_RIGHT(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_nob(&(ca_tree_route_node->baseOrRoute.route.right))) +#define GET_LEFT(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_nob(&(ca_tree_route_node->u.route.left))) +#define GET_RIGHT(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_nob(&(ca_tree_route_node->u.route.right))) #define SET_ROOT(tb, v) erts_atomic_set_nob(&((tb)->root), (erts_aint_t)(v)) -#define SET_LEFT(ca_tree_route_node, v) erts_atomic_set_nob(&(ca_tree_route_node->baseOrRoute.route.left), (erts_aint_t)(v)); -#define SET_RIGHT(ca_tree_route_node, v) erts_atomic_set_nob(&(ca_tree_route_node->baseOrRoute.route.right), (erts_aint_t)(v)); +#define SET_LEFT(ca_tree_route_node, v) erts_atomic_set_nob(&(ca_tree_route_node->u.route.left), (erts_aint_t)(v)); +#define SET_RIGHT(ca_tree_route_node, v) erts_atomic_set_nob(&(ca_tree_route_node->u.route.right), (erts_aint_t)(v)); /* Release or acquire barriers */ #define GET_ROOT_ACQB(tb) ((DbTableCATreeNode*)erts_atomic_read_acqb(&(tb->root))) -#define GET_LEFT_ACQB(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_acqb(&(ca_tree_route_node->baseOrRoute.route.left))) -#define GET_RIGHT_ACQB(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_acqb(&(ca_tree_route_node->baseOrRoute.route.right))) +#define GET_LEFT_ACQB(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_acqb(&(ca_tree_route_node->u.route.left))) +#define GET_RIGHT_ACQB(ca_tree_route_node) ((DbTableCATreeNode*)erts_atomic_read_acqb(&(ca_tree_route_node->u.route.right))) #define SET_ROOT_RELB(tb, v) erts_atomic_set_relb(&((tb)->root), (erts_aint_t)(v)) -#define SET_LEFT_RELB(ca_tree_route_node, v) erts_atomic_set_relb(&(ca_tree_route_node->baseOrRoute.route.left), (erts_aint_t)(v)); -#define SET_RIGHT_RELB(ca_tree_route_node, v) erts_atomic_set_relb(&(ca_tree_route_node->baseOrRoute.route.right), (erts_aint_t)(v)); +#define SET_LEFT_RELB(ca_tree_route_node, v) erts_atomic_set_relb(&(ca_tree_route_node->u.route.left), (erts_aint_t)(v)); +#define SET_RIGHT_RELB(ca_tree_route_node, v) erts_atomic_set_relb(&(ca_tree_route_node->u.route.right), (erts_aint_t)(v)); /* Compares a key to the key in a route node */ static ERTS_INLINE Sint cmp_key_route(DbTableCommon * tb, @@ -740,7 +740,7 @@ void unlock_route_node(DbTableCATreeRouteNode *route_node) current_node = GET_RIGHT_ACQB(current_node); \ } \ } \ - base_node = ¤t_node->baseOrRoute.base; \ + base_node = ¤t_node->u.base; \ LOCK(base_node); \ if ( ! base_node->is_valid ) { \ /* Retry */ \ @@ -792,7 +792,7 @@ static DbTableCATreeNode *create_catree_base_node(DbTableCATree *tb) (DbTable *) tb, sizeof(DbTableCATreeNode)); DbTableCATreeBaseNode *new_base_node = - &new_base_node_container->baseOrRoute.base; + &new_base_node_container->u.base; erts_rwmtx_opt_t rwmtx_opt = ERTS_RWMTX_OPT_DEFAULT_INITER; new_base_node_container->is_base_node = 1; new_base_node->root = NULL; @@ -817,8 +817,7 @@ create_catree_route_node(DbTableCommon * common_table_data, Eterm* top; Eterm key = GETKEY(common_table_data,keyTerm->tpl); int key_size = size_object(key); - Uint offset = offsetof(DbTableCATreeNode,baseOrRoute) + - offsetof(DbTableCATreeRouteNode,key); + Uint offset = offsetof(DbTableCATreeNode,u.route.key); size_t route_node_container_size = offset + sizeof(DbTerm) + @@ -832,7 +831,7 @@ create_catree_route_node(DbTableCommon * common_table_data, DbTableCATreeNode *new_route_node_container = (DbTableCATreeNode*)new_route_node_container_bytes; DbTableCATreeRouteNode *new_route_node = - &new_route_node_container->baseOrRoute.route; + &new_route_node_container->u.route; new_route_node->tab = (DbTable *)common_table_data; if (key_size != 0) { newp->size = key_size; @@ -859,7 +858,7 @@ static void free_catree_base_node(void* base_node_container_ptr) DbTableCATreeNode *base_node_container = (DbTableCATreeNode *)base_node_container_ptr; DbTableCATreeBaseNode *base_node = - &base_node_container->baseOrRoute.base; + &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, @@ -873,10 +872,9 @@ static void free_catree_routing_node(void *route_node_container_ptr) DbTableCATreeNode *route_node_container = (DbTableCATreeNode *)route_node_container_bytes; DbTableCATreeRouteNode *route_node = - &route_node_container->baseOrRoute.route; + &route_node_container->u.route; int key_size = route_node->key.size; - Uint offset = offsetof(DbTableCATreeNode,baseOrRoute) + - offsetof(DbTableCATreeRouteNode,key); + 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); @@ -1001,10 +999,10 @@ get_next_base_node_and_path(DbTableCommon *common_table_data, stack); } else { Eterm pkey = - TOP_NODE(stack)->baseOrRoute.route.key.tpl[0]; /* pKey = key of parent */ + TOP_NODE(stack)->u.route.key.tpl[0]; /* pKey = key of parent */ POP_NODE(stack); while (!EMPTY_NODE(stack)) { - if (TOP_NODE(stack)->baseOrRoute.route.is_valid && + if (TOP_NODE(stack)->u.route.is_valid && cmp_key_route(common_table_data, pkey, TOP_NODE(stack)) <= 0) { return leftmost_base_node_and_path(GET_RIGHT_ACQB(TOP_NODE(stack)), stack); } else { @@ -1045,7 +1043,7 @@ lock_first_base_node(DbTable *tbl, PUSH_NODE(search_stack_ptr, current_node); current_node = GET_LEFT_ACQB(current_node); } - base_node = ¤t_node->baseOrRoute.base; + base_node = ¤t_node->u.base; rlock_base_node(base_node); if ( ! base_node->is_valid ) { /* Retry */ @@ -1076,7 +1074,7 @@ find_and_lock_next_base_node_and_path(DbTable *tbl, if (current_node == NULL) { return NULL; } - base_node = ¤t_node->baseOrRoute.base; + base_node = ¤t_node->u.base; rlock_base_node(base_node); if ( ! base_node->is_valid ) { /* Retry */ @@ -1102,7 +1100,7 @@ void unlock_and_release_locked_base_node_stack(DbTable *tbl, int i; for (i = 0; i < locked_base_nodes_stack_ptr->pos; i++) { current_node = locked_base_nodes_stack_ptr->array[i]; - base_node = ¤t_node->baseOrRoute.base; + base_node = ¤t_node->u.base; if (locked_base_nodes_stack_ptr->pos > 1) { base_node->lock_statistics = /* This is not atomic which is fine as */ base_node->lock_statistics + /* correctness does not depend on that. */ @@ -1178,7 +1176,7 @@ lock_base_node_with_key(DbTable *tbl, current_node = GET_RIGHT_ACQB(current_node); } } - base_node = ¤t_node->baseOrRoute.base; + base_node = ¤t_node->u.base; rlock_base_node(base_node); if ( ! base_node->is_valid ) { /* Retry */ @@ -1204,7 +1202,7 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, DbTableCATreeRouteNode *parent; DbTableCATreeNode *gparent_container; DbTableCATreeRouteNode *gparent; - DbTableCATreeBaseNode *base = &base_container->baseOrRoute.base; + DbTableCATreeBaseNode *base = &base_container->u.base; DbTableCATree *tb = (DbTableCATree *)common_table_data; DbTableCATreeNode *neighbor_base_container; DbTableCATreeBaseNode *neighbor_base; @@ -1214,10 +1212,10 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, if (parent_container == NULL) { return NULL; } - parent = &parent_container->baseOrRoute.route; + parent = &parent_container->u.route; do { neighbor_base_container = leftmost_base_node(GET_RIGHT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->baseOrRoute.base; + neighbor_base = &neighbor_base_container->u.base; wlock_base_node_no_stats(neighbor_base); neighbour_not_valid = !neighbor_base->is_valid; if (neighbour_not_valid) { @@ -1236,7 +1234,7 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, } gparent_container = parent_of(tb, parent_container); if (gparent_container != NULL) { - gparent = &gparent_container->baseOrRoute.route; + gparent = &gparent_container->u.route; lock_route_node(gparent); } else { gparent = NULL; @@ -1254,9 +1252,9 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, unlock_route_node(gparent); } new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->baseOrRoute.base.root = + new_neighbor_base->u.base.root = join_trees(base->root, neighbor_base->root); - wlock_base_node_no_stats(&(new_neighbor_base->baseOrRoute.base)); + wlock_base_node_no_stats(&(new_neighbor_base->u.base)); neighbor_base_parent = NULL; if (GET_RIGHT(parent_container) == neighbor_base_container) { neighbor_base_parent = gparent_container; @@ -1314,10 +1312,10 @@ merge_to_one_locked_base_node(DbTableCommon * common_table_data) parent_container = base_container; base_container = GET_LEFT_ACQB(base_container); } - wlock_base_node_no_stats(&(base_container->baseOrRoute.base)); - is_not_valid = ! base_container->baseOrRoute.base.is_valid; + wlock_base_node_no_stats(&(base_container->u.base)); + is_not_valid = ! base_container->u.base.is_valid; if (is_not_valid) { - wunlock_base_node(&(base_container->baseOrRoute.base)); + wunlock_base_node(&(base_container->u.base)); } } while(is_not_valid); do { @@ -1341,7 +1339,7 @@ static void join_catree(DbTableCATree *tb, DbTableCATreeRouteNode *parent; DbTableCATreeNode *gparent_container; DbTableCATreeRouteNode *gparent; - DbTableCATreeBaseNode *base = &base_container->baseOrRoute.base; + DbTableCATreeBaseNode *base = &base_container->u.base; DbTableCATreeNode *neighbor_base_container; DbTableCATreeBaseNode *neighbor_base; DbTableCATreeNode *new_neighbor_base; @@ -1351,10 +1349,10 @@ static void join_catree(DbTableCATree *tb, wunlock_base_node(base); return; } - parent = &parent_container->baseOrRoute.route; + parent = &parent_container->u.route; if (GET_LEFT(parent_container) == base_container) { neighbor_base_container = leftmost_base_node(GET_RIGHT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->baseOrRoute.base; + neighbor_base = &neighbor_base_container->u.base; if (try_wlock_base_node(neighbor_base)) { /* Failed to acquire lock */ base->lock_statistics = 0; @@ -1378,7 +1376,7 @@ static void join_catree(DbTableCATree *tb, } gparent_container = parent_of(tb, parent_container); if (gparent_container != NULL) { - gparent = &gparent_container->baseOrRoute.route; + gparent = &gparent_container->u.route; lock_route_node(gparent); } else { gparent = NULL; @@ -1396,7 +1394,7 @@ static void join_catree(DbTableCATree *tb, unlock_route_node(gparent); } new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->baseOrRoute.base.root = + new_neighbor_base->u.base.root = join_trees(base->root, neighbor_base->root); neighbor_base_parent = NULL; if (GET_RIGHT(parent_container) == neighbor_base_container) { @@ -1408,7 +1406,7 @@ static void join_catree(DbTableCATree *tb, } } else { /* Symetric case */ neighbor_base_container = rightmost_base_node(GET_LEFT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->baseOrRoute.base; + neighbor_base = &neighbor_base_container->u.base; if (try_wlock_base_node(neighbor_base)) { /* Failed to acquire lock */ base->lock_statistics = 0; @@ -1432,7 +1430,7 @@ static void join_catree(DbTableCATree *tb, } gparent_container = parent_of(tb, parent_container); if (gparent_container != NULL) { - gparent = &gparent_container->baseOrRoute.route; + gparent = &gparent_container->u.route; lock_route_node(gparent); } else { gparent = NULL; @@ -1450,7 +1448,7 @@ static void join_catree(DbTableCATree *tb, unlock_route_node(gparent); } new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->baseOrRoute.base.root = + new_neighbor_base->u.base.root = join_trees(neighbor_base->root, base->root); neighbor_base_parent = NULL; if (GET_LEFT(parent_container) == neighbor_base_container) { @@ -1493,12 +1491,12 @@ static void split_catree(DbTableCommon *tb, DbTableCATreeNode *left_base_node; DbTableCATreeNode *right_base_node; DbTableCATreeNode *routing_node_container; - DbTableCATreeBaseNode *base = &base_container->baseOrRoute.base; + DbTableCATreeBaseNode *base = &base_container->u.base; DbTableCATreeRouteNode *parent; if (parent_container == NULL) { parent = NULL; } else { - parent = &parent_container->baseOrRoute.route; + parent = &parent_container->u.route; } if (less_than_two_elements(base->root)) { @@ -1517,8 +1515,8 @@ static void split_catree(DbTableCommon *tb, create_catree_base_node((DbTableCATree*)tb); right_base_node = create_catree_base_node((DbTableCATree*)tb); - left_base_node->baseOrRoute.base.root = leftWriteBack; - right_base_node->baseOrRoute.base.root = rightWriteBack; + left_base_node->u.base.root = leftWriteBack; + right_base_node->u.base.root = rightWriteBack; routing_node_container = create_catree_route_node(tb, left_base_node, right_base_node, @@ -1546,7 +1544,7 @@ static void catree_add_base_node_to_free_list( DbTableCATree *tb, DbTableCATreeNode *base_node_container) { - base_node_container->baseOrRoute.base.next = + base_node_container->u.base.next = tb->base_nodes_to_free_list; tb->base_nodes_to_free_list = base_node_container; } @@ -1557,7 +1555,7 @@ static void catree_deque_base_node_from_free_list(DbTableCATree *tb) return; /* List empty */ } else { DbTableCATreeNode *first = tb->base_nodes_to_free_list; - tb->base_nodes_to_free_list = first->baseOrRoute.base.next; + tb->base_nodes_to_free_list = first->u.base.next; } } @@ -1640,7 +1638,7 @@ static SWord do_free_base_node_cont(DbTableCATree *tb, SWord num_left) free_catree_base_node(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->baseOrRoute.base.root); + PUSH_NODE(&tb->free_stack_elems, base_node_container->u.base.root); } return num_left; } @@ -1675,7 +1673,7 @@ static int db_first_catree(Process *p, DbTable *tbl, Eterm *ret) int result; DECLARE_AND_INIT_BASE_NODE_SEARCH_STACKS; /* Find first base node */ - base_node = &(lock_first_base_node(tbl, &search_stack, &locked_base_nodes_stack)->baseOrRoute.base); + base_node = &(lock_first_base_node(tbl, &search_stack, &locked_base_nodes_stack)->u.base); /* Find next base node until non-empty base node is found */ while (base_node != NULL && base_node->root == NULL) { base_node = find_and_lock_next_base_node_and_path(tbl, &search_stack_ptr, &search_stack_copy_ptr, locked_base_nodes_stack_ptr); @@ -1718,8 +1716,8 @@ static int db_last_catree(Process *p, DbTable *tbl, Eterm *ret) { DbTableCATree *tb = &tbl->catree; DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); - int result = db_last_tree_common(p, tbl, base->baseOrRoute.base.root, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + int result = db_last_tree_common(p, tbl, base->u.base.root, ret, NULL); + wunlock_base_node(&(base->u.base)); return result; } @@ -1733,8 +1731,8 @@ static int db_prev_catree(Process *p, DbTable *tbl, Eterm key, Eterm *ret) DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); base = merge_to_one_locked_base_node(&tb->common); - result = db_prev_tree_common(p, tbl, base->baseOrRoute.base.root, key, ret, &stack); - wunlock_base_node(&(base->baseOrRoute.base)); + result = db_prev_tree_common(p, tbl, base->u.base.root, key, ret, &stack); + wunlock_base_node(&(base->u.base)); return result; } @@ -1839,9 +1837,9 @@ static int db_slot_catree(Process *p, DbTable *tbl, int result; DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); - result = db_slot_tree_common(p, tbl, base->baseOrRoute.base.root, + result = db_slot_tree_common(p, tbl, base->u.base.root, slot_term, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1854,9 +1852,9 @@ static int db_select_continue_catree(Process *p, int result; DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); - result = db_select_continue_tree_common(p, &tb->common, &base->baseOrRoute.base.root, + result = db_select_continue_tree_common(p, &tb->common, &base->u.base.root, continuation, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1868,10 +1866,10 @@ static int db_select_catree(Process *p, DbTable *tbl, Eterm tid, int result; DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); - result = db_select_tree_common(p, &tb->common, &base->baseOrRoute.base.root, + result = db_select_tree_common(p, &tb->common, &base->u.base.root, tid, pattern, reverse, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1885,9 +1883,9 @@ static int db_select_count_continue_catree(Process *p, DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); result = db_select_count_continue_tree_common(p, &tb->common, - &base->baseOrRoute.base.root, + &base->u.base.root, continuation, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1898,9 +1896,9 @@ static int db_select_count_catree(Process *p, DbTable *tbl, Eterm tid, int result; DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); - result = db_select_count_tree_common(p, &tb->common, &base->baseOrRoute.base.root, + result = db_select_count_tree_common(p, &tb->common, &base->u.base.root, tid, pattern, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1912,9 +1910,9 @@ static int db_select_chunk_catree(Process *p, DbTable *tbl, Eterm tid, int result; DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); - result = db_select_chunk_tree_common(p, &tb->common, &base->baseOrRoute.base.root, + result = db_select_chunk_tree_common(p, &tb->common, &base->u.base.root, tid, pattern, chunk_size, reversed, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1930,9 +1928,9 @@ static int db_select_delete_continue_catree(Process *p, DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); base = merge_to_one_locked_base_node(&tb->common); - result = db_select_delete_continue_tree_common(p, tbl, &base->baseOrRoute.base.root, + result = db_select_delete_continue_tree_common(p, tbl, &base->u.base.root, continuation, ret, &stack); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1946,9 +1944,9 @@ static int db_select_delete_catree(Process *p, DbTable *tbl, Eterm tid, DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); base = merge_to_one_locked_base_node(&tb->common); - result = db_select_delete_tree_common(p, tbl, &base->baseOrRoute.base.root, + result = db_select_delete_tree_common(p, tbl, &base->u.base.root, tid, pattern, ret, &stack); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1960,9 +1958,9 @@ static int db_select_replace_catree(Process *p, DbTable *tbl, Eterm tid, DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); result = db_select_replace_tree_common(p, &tb->common, - &base->baseOrRoute.base.root, + &base->u.base.root, tid, pattern, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -1974,9 +1972,9 @@ static int db_select_replace_continue_catree(Process *p, DbTable *tbl, DbTableCATreeNode *base; base = merge_to_one_locked_base_node(&tb->common); result = db_select_replace_continue_tree_common(p, &tb->common, - &base->baseOrRoute.base.root, + &base->u.base.root, continuation, ret, NULL); - wunlock_base_node(&(base->baseOrRoute.base)); + wunlock_base_node(&(base->u.base)); return result; } @@ -2004,8 +2002,8 @@ static void db_print_catree(fmtfn_t to, void *to_arg, { DbTableCATree *tb = &tbl->catree; DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); - db_print_tree_common(to, to_arg, show, base->baseOrRoute.base.root, tbl); - wunlock_base_node(&(base->baseOrRoute.base)); + db_print_tree_common(to, to_arg, show, base->u.base.root, tbl); + wunlock_base_node(&(base->u.base)); } /* Release all memory occupied by a single table */ @@ -2045,7 +2043,7 @@ static SWord db_free_table_continue_catree(DbTable *tbl, SWord reds) } else { tb->is_routing_nodes_freed = 1; /* Ready with the routing nodes */ first_base_node = catree_first_base_node_from_free_list(tb); - PUSH_NODE(&tb->free_stack_elems, first_base_node->baseOrRoute.base.root); + PUSH_NODE(&tb->free_stack_elems, first_base_node->u.base.root); } } while (catree_first_base_node_from_free_list(tb) != NULL) { @@ -2083,8 +2081,8 @@ static void db_foreach_offheap_catree(DbTable *tbl, { DbTableCATree *tb = &tbl->catree; DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); - db_foreach_offheap_tree_common(base->baseOrRoute.base.root, func, arg); - wunlock_base_node(&(base->baseOrRoute.base)); + db_foreach_offheap_tree_common(base->u.base.root, func, arg); + wunlock_base_node(&(base->u.base)); } static int db_lookup_dbterm_catree(Process *p, DbTable *tbl, Eterm key, Eterm obj, @@ -2111,7 +2109,7 @@ static void db_finalize_dbterm_catree(int cret, DbUpdateHandle *handle) DbTableCATreeNode *prev_node = handle->lck; DbTableCATreeNode *current_node = handle->lck2; int current_level = handle->current_level; - DbTableCATreeBaseNode *base_node = ¤t_node->baseOrRoute.base; + DbTableCATreeBaseNode *base_node = ¤t_node->u.base; db_finalize_dbterm_tree_common(cret, handle, NULL); ERL_DB_CATREE_CREATE_DO_OPERATION_FUNCTION_ADAPT_AND_UNLOCK_PART; return; diff --git a/erts/emulator/beam/erl_db_catree.h b/erts/emulator/beam/erl_db_catree.h index 1f2c7da091..406f3bb0f2 100644 --- a/erts/emulator/beam/erl_db_catree.h +++ b/erts/emulator/beam/erl_db_catree.h @@ -58,7 +58,7 @@ typedef struct DbTableCATreeNode { union { DbTableCATreeRouteNode route; DbTableCATreeBaseNode base; - } baseOrRoute; + } u; } DbTableCATreeNode; typedef struct { -- cgit v1.2.3 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') 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 From a875b2991bebe68cbb1ce1d0cb0b338fc3248cc9 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Fri, 28 Sep 2018 14:37:22 +0200 Subject: erts: Do some refactoring in erl_db_catree.c Fewer variables with shorter names and prefer DbTableCATree over DbTableCommon. --- erts/emulator/beam/erl_db_catree.c | 653 ++++++++++++++++--------------------- 1 file changed, 285 insertions(+), 368 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 71768adcda..65ed7ab81d 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -235,8 +235,7 @@ DbTableMethod db_catree = #define SET_RIGHT_RELB(ca_tree_route_node, v) erts_atomic_set_relb(&(ca_tree_route_node->u.route.right), (erts_aint_t)(v)); /* Compares a key to the key in a route node */ -static ERTS_INLINE Sint cmp_key_route(DbTableCommon * tb, - Eterm key, +static ERTS_INLINE Sint cmp_key_route(Eterm key, DbTableCATreeNode *obj) { return CMP(key, GET_ROUTE_NODE_KEY(obj)); @@ -279,7 +278,7 @@ int less_than_two_elements(TreeDbTerm *root) * Inserts a TreeDbTerm into a tree. Returns the new root. */ static ERTS_INLINE -TreeDbTerm* insert_TreeDbTerm(DbTableCommon *common_table_data, +TreeDbTerm* insert_TreeDbTerm(DbTableCATree *tb, TreeDbTerm *insert_to_root, TreeDbTerm *value_to_insert) { /* Non recursive insertion in AVL tree, building our own stack */ @@ -295,7 +294,7 @@ TreeDbTerm* insert_TreeDbTerm(DbTableCommon *common_table_data, int dir; TreeDbTerm *p1, *p2, *p; - key = GETKEY(common_table_data, value_to_insert->dbterm.tpl); + key = GETKEY(tb, value_to_insert->dbterm.tpl); dstack[dpos++] = DIR_END; for (;;) @@ -305,7 +304,7 @@ TreeDbTerm* insert_TreeDbTerm(DbTableCommon *common_table_data, (*this)->balance = 0; (*this)->left = (*this)->right = NULL; break; - } else if ((c = cmp_key(common_table_data, key, *this)) < 0) { + } else if ((c = cmp_key(&tb->common, key, *this)) < 0) { /* go lefts */ dstack[dpos++] = DIR_LEFT; tstack[tpos++] = this; @@ -392,7 +391,7 @@ TreeDbTerm* insert_TreeDbTerm(DbTableCommon *common_table_data, * left_wb and the tree containing the rest of the keys in the write * back parameter right_wb. */ -static void split_tree(DbTableCommon *tb, +static void split_tree(DbTableCATree *tb, TreeDbTerm *root, TreeDbTerm **split_key_node_wb, TreeDbTerm **left_wb, @@ -413,9 +412,7 @@ static void split_tree(DbTableCommon *tb, split_node->left = NULL; right_root = split_node->right; split_node->right = NULL; - right_root = insert_TreeDbTerm(tb, - right_root, - split_node); + right_root = insert_TreeDbTerm(tb, right_root, split_node); *split_key_node_wb = split_node; *left_wb = left_root; *right_wb = right_root; @@ -700,15 +697,17 @@ void runlock_base_node(DbTableCATreeBaseNode *base_node) } static ERTS_INLINE -void lock_route_node(DbTableCATreeRouteNode *route_node) +void lock_route_node(DbTableCATreeNode *route_node) { - erts_mtx_lock(&route_node->lock); + ASSERT(!route_node->is_base_node); + erts_mtx_lock(&route_node->u.route.lock); } static ERTS_INLINE -void unlock_route_node(DbTableCATreeRouteNode *route_node) +void unlock_route_node(DbTableCATreeNode *route_node) { - erts_mtx_unlock(&route_node->lock); + ASSERT(!route_node->is_base_node); + erts_mtx_unlock(&route_node->u.route.lock); } @@ -722,7 +721,6 @@ void unlock_route_node(DbTableCATreeRouteNode *route_node) int retry; \ DbTableCATreeNode *current_node; \ DbTableCATreeNode *prev_node; \ - DbTableCommon* common_table_data = &tb->common; \ DbTableCATreeBaseNode *base_node; \ int current_level; \ (void)prev_node; \ @@ -734,7 +732,7 @@ void unlock_route_node(DbTableCATreeRouteNode *route_node) while ( ! current_node->is_base_node ) { \ current_level = current_level + 1; \ prev_node = current_node; \ - if (cmp_key_route(common_table_data,key,current_node) < 0) { \ + if (cmp_key_route(key,current_node) < 0) { \ current_node = GET_LEFT_ACQB(current_node); \ } else { \ current_node = GET_RIGHT_ACQB(current_node); \ @@ -753,7 +751,7 @@ void unlock_route_node(DbTableCATreeRouteNode *route_node) #define ERL_DB_CATREE_CREATE_DO_OPERATION_FUNCTION_ADAPT_AND_UNLOCK_PART \ if (base_node->lock_statistics > ERL_DB_CATREE_HIGH_CONTENTION_LIMIT \ && current_level < ERL_DB_CATREE_MAX_ROUTE_NODE_LAYER_HEIGHT) { \ - split_catree(&tb->common, prev_node, current_node); \ + split_catree(tb, prev_node, current_node); \ } else if (base_node->lock_statistics < ERL_DB_CATREE_LOW_CONTENTION_LIMIT) { \ join_catree(tb, prev_node, current_node); \ } else { \ @@ -787,24 +785,22 @@ void unlock_route_node(DbTableCATreeRouteNode *route_node) static DbTableCATreeNode *create_catree_base_node(DbTableCATree *tb) { - DbTableCATreeNode *new_base_node_container = - erts_db_alloc(ERTS_ALC_T_DB_TABLE, - (DbTable *) tb, - sizeof(DbTableCATreeNode)); - DbTableCATreeBaseNode *new_base_node = - &new_base_node_container->u.base; + DbTableCATreeNode *p = erts_db_alloc(ERTS_ALC_T_DB_TABLE, + (DbTable *) tb, + sizeof(DbTableCATreeNode)); erts_rwmtx_opt_t rwmtx_opt = ERTS_RWMTX_OPT_DEFAULT_INITER; - new_base_node_container->is_base_node = 1; - new_base_node->root = NULL; + p->is_base_node = 1; + p->u.base.root = NULL; if (tb->common.type & DB_FREQ_READ) rwmtx_opt.type = ERTS_RWMTX_TYPE_FREQUENT_READ; if (erts_ets_rwmtx_spin_count >= 0) rwmtx_opt.main_spincount = erts_ets_rwmtx_spin_count; - erts_rwmtx_init_opt(&new_base_node->lock, &rwmtx_opt, - "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; - return new_base_node_container; + erts_rwmtx_init_opt(&p->u.base.lock, &rwmtx_opt, + "erl_db_catree_base_node", tb->common.the_name, + ERTS_LOCK_FLAGS_CATEGORY_DB); + p->u.base.lock_statistics = 0; + p->u.base.is_valid = 1; + return p; } static ERTS_INLINE Uint sizeof_route_node(Uint key_size) @@ -814,47 +810,37 @@ static ERTS_INLINE Uint sizeof_route_node(Uint key_size) } static DbTableCATreeNode* -create_catree_route_node(DbTableCommon * common_table_data, +create_catree_route_node(DbTableCATree *tb, DbTableCATreeNode *left, DbTableCATreeNode *right, DbTerm * keyTerm) { Eterm* top; - Eterm key = GETKEY(common_table_data,keyTerm->tpl); + Eterm key = GETKEY(tb,keyTerm->tpl); int key_size = size_object(key); - Uint offset = offsetof(DbTableCATreeNode,u.route.key); - size_t route_node_container_size = - offset + - sizeof(DbTerm) + - sizeof(Eterm)*key_size; ErlOffHeap tmp_offheap; - byte* new_route_node_container_bytes = - erts_db_alloc(ERTS_ALC_T_DB_TABLE, - (DbTable *) common_table_data, - route_node_container_size); - DbTerm* newp = (DbTerm*) (new_route_node_container_bytes + offset); - DbTableCATreeNode *new_route_node_container = - (DbTableCATreeNode*)new_route_node_container_bytes; - DbTableCATreeRouteNode *new_route_node = - &new_route_node_container->u.route; + DbTableCATreeNode* p = erts_db_alloc(ERTS_ALC_T_DB_TABLE, + (DbTable *) tb, + sizeof_route_node(key_size)); + if (key_size != 0) { - newp->size = key_size; - top = &newp->tpl[1]; + p->u.route.key.size = key_size; + top = &p->u.route.key.tpl[1]; tmp_offheap.first = NULL; - newp->tpl[0] = copy_struct(key, key_size, &top, &tmp_offheap); - newp->first_oh = tmp_offheap.first; + p->u.route.key.tpl[0] = copy_struct(key, key_size, &top, &tmp_offheap); + p->u.route.key.first_oh = tmp_offheap.first; } else { - newp->size = key_size; - newp->first_oh = NULL; - newp->tpl[0] = key; + p->u.route.key.size = key_size; + p->u.route.key.first_oh = NULL; + p->u.route.key.tpl[0] = key; } - new_route_node_container->is_base_node = 0; - new_route_node->is_valid = 1; - erts_atomic_init_nob(&(new_route_node->left), (erts_aint_t)left); - erts_atomic_init_nob(&(new_route_node->right), (erts_aint_t)right); - erts_mtx_init(&new_route_node->lock, "erl_db_catree_route_node", + p->is_base_node = 0; + p->u.route.is_valid = 1; + erts_atomic_init_nob(&p->u.route.left, (erts_aint_t)left); + erts_atomic_init_nob(&p->u.route.right, (erts_aint_t)right); + erts_mtx_init(&p->u.route.lock, "erl_db_catree_route_node", NIL, ERTS_LOCK_FLAGS_CATEGORY_DB); - return new_route_node_container; + return p; } static void do_free_base_node(void* vptr) @@ -895,29 +881,26 @@ static void free_catree_route_node(DbTableCATree* tb, DbTableCATreeNode* p) /* * Returns the parent routing node of the specified - * route_node_container if such a routing node exists or NULL if - * route_node_container is attached to the root + * route node 'child' if such a parent exists + * or NULL if 'child' is attached to the root. */ static ERTS_INLINE DbTableCATreeNode * parent_of(DbTableCATree *tb, - DbTableCATreeNode *route_node_container) + DbTableCATreeNode *child) { + Eterm key = GET_ROUTE_NODE_KEY(child); + DbTableCATreeNode *current = GET_ROOT_ACQB(tb); + DbTableCATreeNode *prev = NULL; - Eterm key = GET_ROUTE_NODE_KEY(route_node_container); - DbTableCATreeNode *current_node = GET_ROOT_ACQB(tb); - DbTableCATreeNode *prev_node = NULL; - if (current_node == route_node_container) { - return NULL; - } - while (current_node != route_node_container) { - prev_node = current_node; - if (cmp_key_route((DbTableCommon *)tb, key, current_node) < 0) { - current_node = GET_LEFT_ACQB(current_node); + while (current != child) { + prev = current; + if (cmp_key_route(key, current) < 0) { + current = GET_LEFT_ACQB(current); } else { - current_node = GET_RIGHT_ACQB(current_node); + current = GET_RIGHT_ACQB(current); } } - return prev_node; + return prev; } @@ -952,11 +935,7 @@ leftmost_route_node(DbTableCATreeNode *root) prev_node = node; node = GET_LEFT_ACQB(node); } - if (prev_node == NULL) { - return NULL; - } else { - return prev_node; - } + return prev_node; } static ERTS_INLINE DbTableCATreeNode* @@ -968,11 +947,7 @@ rightmost_route_node(DbTableCATreeNode *root) prev_node = node; node = GET_RIGHT_ACQB(node); } - if (prev_node == NULL) { - return NULL; - } else { - return prev_node; - } + return prev_node; } static ERTS_INLINE DbTableCATreeNode* @@ -987,8 +962,7 @@ leftmost_base_node_and_path(DbTableCATreeNode *root, CATreeNodeStack * stack) } static ERTS_INLINE DbTableCATreeNode* -get_next_base_node_and_path(DbTableCommon *common_table_data, - DbTableCATreeNode *base_node, +get_next_base_node_and_path(DbTableCATreeNode *base_node, CATreeNodeStack *stack) { if (EMPTY_NODE(stack)) { /* The parent of b is the root */ @@ -1004,7 +978,7 @@ get_next_base_node_and_path(DbTableCommon *common_table_data, POP_NODE(stack); while (!EMPTY_NODE(stack)) { if (TOP_NODE(stack)->u.route.is_valid && - cmp_key_route(common_table_data, pkey, TOP_NODE(stack)) <= 0) { + cmp_key_route(pkey, TOP_NODE(stack)) <= 0) { return leftmost_base_node_and_path(GET_RIGHT_ACQB(TOP_NODE(stack)), stack); } else { POP_NODE(stack); @@ -1033,12 +1007,10 @@ lock_first_base_node(DbTable *tbl, CATreeNodeStack *search_stack_ptr, CATreeNodeStack *locked_base_nodes_stack_ptr) { - int retry; DbTableCATreeNode *current_node; DbTableCATreeBaseNode *base_node; DbTableCATree* tb = &tbl->catree; - do { - retry = 0; + while (1) { current_node = GET_ROOT_ACQB(tb); while ( ! current_node->is_base_node ) { PUSH_NODE(search_stack_ptr, current_node); @@ -1046,12 +1018,11 @@ lock_first_base_node(DbTable *tbl, } base_node = ¤t_node->u.base; rlock_base_node(base_node); - if ( ! base_node->is_valid ) { - /* Retry */ - runlock_base_node(base_node); - retry = 1; - } - } while(retry); + if (base_node->is_valid) + break; + /* Retry */ + runlock_base_node(base_node); + } push_node_dyn_array(tbl, locked_base_nodes_stack_ptr, current_node); return current_node; } @@ -1065,19 +1036,20 @@ find_and_lock_next_base_node_and_path(DbTable *tbl, DbTableCATreeNode *current_node; DbTableCATreeBaseNode *base_node; CATreeNodeStack * tmp_stack_ptr; - DbTableCommon* common_table_data; - retry_find_and_lock_next_base_node: - current_node = TOP_NODE(locked_base_nodes_stack_ptr); - common_table_data = &tbl->common; - clone_stack(*search_stack_ptr_ptr, *search_stack_copy_ptr_ptr); - current_node = - get_next_base_node_and_path(common_table_data, current_node, *search_stack_ptr_ptr); - if (current_node == NULL) { - return NULL; - } - base_node = ¤t_node->u.base; - rlock_base_node(base_node); - if ( ! base_node->is_valid ) { + + while (1) { + current_node = TOP_NODE(locked_base_nodes_stack_ptr); + clone_stack(*search_stack_ptr_ptr, *search_stack_copy_ptr_ptr); + current_node = + get_next_base_node_and_path(current_node, *search_stack_ptr_ptr); + if (current_node == NULL) { + return NULL; + } + base_node = ¤t_node->u.base; + rlock_base_node(base_node); + if (base_node->is_valid) + break; + /* Retry */ runlock_base_node(base_node); /* Revert to previous state */ @@ -1085,10 +1057,9 @@ find_and_lock_next_base_node_and_path(DbTable *tbl, tmp_stack_ptr = *search_stack_ptr_ptr; *search_stack_ptr_ptr = *search_stack_copy_ptr_ptr; *search_stack_copy_ptr_ptr = tmp_stack_ptr; - goto retry_find_and_lock_next_base_node; - } else { - push_node_dyn_array(tbl, locked_base_nodes_stack_ptr, current_node); } + + push_node_dyn_array(tbl, locked_base_nodes_stack_ptr, current_node); return base_node; } @@ -1165,13 +1136,12 @@ lock_base_node_with_key(DbTable *tbl, DbTableCATreeNode *current_node; DbTableCATreeBaseNode *base_node; DbTableCATree* tb = &tbl->catree; - DbTableCommon* common_table_data = &tbl->common; do { retry = 0; current_node = GET_ROOT_ACQB(tb); while ( ! current_node->is_base_node ) { PUSH_NODE(search_stack_ptr, current_node); - if( cmp_key_route(common_table_data,key,current_node) < 0 ) { + if( cmp_key_route(key,current_node) < 0 ) { current_node = GET_LEFT_ACQB(current_node); } else { current_node = GET_RIGHT_ACQB(current_node); @@ -1195,106 +1165,93 @@ lock_base_node_with_key(DbTable *tbl, * node to join with. */ static DbTableCATreeNode* -erl_db_catree_force_join_right(DbTableCommon *common_table_data, - DbTableCATreeNode *parent_container, - DbTableCATreeNode *base_container, +erl_db_catree_force_join_right(DbTableCATree *tb, + DbTableCATreeNode *parent, + DbTableCATreeNode *thiz, DbTableCATreeNode **result_parent_wb) { - DbTableCATreeRouteNode *parent; - DbTableCATreeNode *gparent_container; - DbTableCATreeRouteNode *gparent; - DbTableCATreeBaseNode *base = &base_container->u.base; - DbTableCATree *tb = (DbTableCATree *)common_table_data; - DbTableCATreeNode *neighbor_base_container; - DbTableCATreeBaseNode *neighbor_base; - DbTableCATreeNode *new_neighbor_base; - DbTableCATreeNode *neighbor_base_parent; + DbTableCATreeNode *gparent; + DbTableCATreeNode *neighbor; + DbTableCATreeNode *new_neighbor; + DbTableCATreeNode *neighbor_parent; int neighbour_not_valid; - if (parent_container == NULL) { + if (parent == NULL) { return NULL; } - parent = &parent_container->u.route; do { - neighbor_base_container = leftmost_base_node(GET_RIGHT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->u.base; - wlock_base_node_no_stats(neighbor_base); - neighbour_not_valid = !neighbor_base->is_valid; + neighbor = leftmost_base_node(GET_RIGHT_ACQB(parent)); + wlock_base_node_no_stats(&neighbor->u.base); + neighbour_not_valid = !neighbor->u.base.is_valid; if (neighbour_not_valid) { - wunlock_base_node(neighbor_base); + wunlock_base_node(&neighbor->u.base); } } while (neighbour_not_valid); lock_route_node(parent); - parent->is_valid = 0; - neighbor_base->is_valid = 0; - base->is_valid = 0; + parent->u.route.is_valid = 0; + neighbor->u.base.is_valid = 0; + thiz->u.base.is_valid = 0; gparent = NULL; - gparent_container = NULL; do { if (gparent != NULL) { unlock_route_node(gparent); } - gparent_container = parent_of(tb, parent_container); - if (gparent_container != NULL) { - gparent = &gparent_container->u.route; + gparent = parent_of(tb, parent); + if (gparent != NULL) { lock_route_node(gparent); - } else { - gparent = NULL; } - } while (gparent != NULL && !gparent->is_valid); + } while (gparent != NULL && !gparent->u.route.is_valid); if (gparent == NULL) { - SET_ROOT_RELB(tb, GET_RIGHT(parent_container)); - } else if (GET_LEFT(gparent_container) == parent_container) { - SET_LEFT_RELB(gparent_container, GET_RIGHT(parent_container)); + SET_ROOT_RELB(tb, GET_RIGHT(parent)); + } else if (GET_LEFT(gparent) == parent) { + SET_LEFT_RELB(gparent, GET_RIGHT(parent)); } else { - SET_RIGHT_RELB(gparent_container, GET_RIGHT(parent_container)); + SET_RIGHT_RELB(gparent, GET_RIGHT(parent)); } unlock_route_node(parent); if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->u.base.root = - join_trees(base->root, neighbor_base->root); - wlock_base_node_no_stats(&(new_neighbor_base->u.base)); - neighbor_base_parent = NULL; - if (GET_RIGHT(parent_container) == neighbor_base_container) { - neighbor_base_parent = gparent_container; + new_neighbor = create_catree_base_node(tb); + new_neighbor->u.base.root = + join_trees(thiz->u.base.root, neighbor->u.base.root); + wlock_base_node_no_stats(&(new_neighbor->u.base)); + if (GET_RIGHT(parent) == neighbor) { + neighbor_parent = gparent; } else { - neighbor_base_parent = - leftmost_route_node(GET_RIGHT(parent_container)); + neighbor_parent = leftmost_route_node(GET_RIGHT(parent)); } - if(neighbor_base_parent == NULL) { - SET_ROOT_RELB(tb, new_neighbor_base); - } else if (GET_LEFT(neighbor_base_parent) == neighbor_base_container) { - SET_LEFT_RELB(neighbor_base_parent, new_neighbor_base); + if(neighbor_parent == NULL) { + SET_ROOT_RELB(tb, new_neighbor); + } else if (GET_LEFT(neighbor_parent) == neighbor) { + SET_LEFT_RELB(neighbor_parent, new_neighbor); } else { - SET_RIGHT_RELB(neighbor_base_parent, new_neighbor_base); + SET_RIGHT_RELB(neighbor_parent, new_neighbor); } - wunlock_base_node(base); - wunlock_base_node(neighbor_base); + wunlock_base_node(&thiz->u.base); + wunlock_base_node(&neighbor->u.base); /* Free the parent and base */ - erts_schedule_db_free(common_table_data, + 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(common_table_data, + parent, + &parent->u.route.free_item, + sizeof_route_node(parent->u.route.key.size)); + erts_schedule_db_free(&tb->common, do_free_base_node, - base_container, - &base->free_item, + thiz, + &thiz->u.base.free_item, sizeof(DbTableCATreeNode)); - erts_schedule_db_free(common_table_data, + erts_schedule_db_free(&tb->common, do_free_base_node, - neighbor_base_container, - &neighbor_base->free_item, + neighbor, + &neighbor->u.base.free_item, sizeof(DbTableCATreeNode)); - if (parent_container == neighbor_base_container) { - *result_parent_wb = gparent_container; + if (parent == neighbor) { + *result_parent_wb = gparent; } else { - *result_parent_wb = neighbor_base_parent; + *result_parent_wb = neighbor_parent; } - return new_neighbor_base; + return new_neighbor; } /* @@ -1303,250 +1260,224 @@ erl_db_catree_force_join_right(DbTableCommon *common_table_data, * locked state. */ static DbTableCATreeNode * -merge_to_one_locked_base_node(DbTableCommon * common_table_data) +merge_to_one_locked_base_node(DbTableCATree* tb) { - DbTableCATreeNode *parent_container; - DbTableCATreeNode *new_parent_container; - DbTableCATree *tb = (DbTableCATree *)common_table_data; - DbTableCATreeNode *base_container; - DbTableCATreeNode *new_base_container; + DbTableCATreeNode *parent; + DbTableCATreeNode *new_parent; + DbTableCATreeNode *base; + DbTableCATreeNode *new_base; int is_not_valid; /* Find first base node */ do { - parent_container = NULL; - base_container = GET_ROOT_ACQB(tb); - while ( ! base_container->is_base_node ) { - parent_container = base_container; - base_container = GET_LEFT_ACQB(base_container); + parent = NULL; + base = GET_ROOT_ACQB(tb); + while ( ! base->is_base_node ) { + parent = base; + base = GET_LEFT_ACQB(base); } - wlock_base_node_no_stats(&(base_container->u.base)); - is_not_valid = ! base_container->u.base.is_valid; + wlock_base_node_no_stats(&(base->u.base)); + is_not_valid = ! base->u.base.is_valid; if (is_not_valid) { - wunlock_base_node(&(base_container->u.base)); + wunlock_base_node(&(base->u.base)); } } while(is_not_valid); do { - new_base_container = erl_db_catree_force_join_right(common_table_data, - parent_container, - base_container, - &new_parent_container); - if (new_base_container != NULL) { - base_container = new_base_container; - parent_container = new_parent_container; + new_base = erl_db_catree_force_join_right(tb, + parent, + base, + &new_parent); + if (new_base != NULL) { + base = new_base; + parent = new_parent; } - } while(new_base_container != NULL); - return base_container; + } while(new_base != NULL); + return base; } static void join_catree(DbTableCATree *tb, - DbTableCATreeNode *parent_container, - DbTableCATreeNode *base_container) -{ - DbTableCATreeRouteNode *parent; - DbTableCATreeNode *gparent_container; - DbTableCATreeRouteNode *gparent; - DbTableCATreeBaseNode *base = &base_container->u.base; - DbTableCATreeNode *neighbor_base_container; - DbTableCATreeBaseNode *neighbor_base; - DbTableCATreeNode *new_neighbor_base; - DbTableCATreeNode *neighbor_base_parent; - if (parent_container == NULL) { - base->lock_statistics = 0; - wunlock_base_node(base); + DbTableCATreeNode *parent, + DbTableCATreeNode *thiz) +{ + DbTableCATreeNode *gparent; + DbTableCATreeNode *neighbor; + DbTableCATreeNode *new_neighbor; + DbTableCATreeNode *neighbor_parent; + + ASSERT(thiz->is_base_node); + if (parent == NULL) { + thiz->u.base.lock_statistics = 0; + wunlock_base_node(&thiz->u.base); return; } - parent = &parent_container->u.route; - if (GET_LEFT(parent_container) == base_container) { - neighbor_base_container = leftmost_base_node(GET_RIGHT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->u.base; - if (try_wlock_base_node(neighbor_base)) { + ASSERT(!parent->is_base_node); + if (GET_LEFT(parent) == thiz) { + neighbor = leftmost_base_node(GET_RIGHT_ACQB(parent)); + if (try_wlock_base_node(&neighbor->u.base)) { /* Failed to acquire lock */ - base->lock_statistics = 0; - wunlock_base_node(base); + thiz->u.base.lock_statistics = 0; + wunlock_base_node(&thiz->u.base); return; - } else if (!neighbor_base->is_valid) { - base->lock_statistics = 0; - wunlock_base_node(base); - wunlock_base_node(neighbor_base); + } else if (!neighbor->u.base.is_valid) { + thiz->u.base.lock_statistics = 0; + wunlock_base_node(&thiz->u.base); + wunlock_base_node(&neighbor->u.base); return; } else { lock_route_node(parent); - parent->is_valid = 0; - neighbor_base->is_valid = 0; - base->is_valid = 0; + parent->u.route.is_valid = 0; + neighbor->u.base.is_valid = 0; + thiz->u.base.is_valid = 0; gparent = NULL; - gparent_container = NULL; do { if (gparent != NULL) { unlock_route_node(gparent); } - gparent_container = parent_of(tb, parent_container); - if (gparent_container != NULL) { - gparent = &gparent_container->u.route; + gparent = parent_of(tb, parent); + if (gparent != NULL) lock_route_node(gparent); - } else { - gparent = NULL; - } - } while (gparent != NULL && !gparent->is_valid); + } while (gparent != NULL && !gparent->u.route.is_valid); + if (gparent == NULL) { - SET_ROOT_RELB(tb, GET_RIGHT(parent_container)); - } else if (GET_LEFT(gparent_container) == parent_container) { - SET_LEFT_RELB(gparent_container, GET_RIGHT(parent_container)); + SET_ROOT_RELB(tb, GET_RIGHT(parent)); + } else if (GET_LEFT(gparent) == parent) { + SET_LEFT_RELB(gparent, GET_RIGHT(parent)); } else { - SET_RIGHT_RELB(gparent_container, GET_RIGHT(parent_container)); + SET_RIGHT_RELB(gparent, GET_RIGHT(parent)); } unlock_route_node(parent); if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->u.base.root = - join_trees(base->root, neighbor_base->root); - neighbor_base_parent = NULL; - if (GET_RIGHT(parent_container) == neighbor_base_container) { - neighbor_base_parent = gparent_container; + new_neighbor = create_catree_base_node(tb); + new_neighbor->u.base.root = join_trees(thiz->u.base.root, + neighbor->u.base.root); + neighbor_parent = NULL; + if (GET_RIGHT(parent) == neighbor) { + neighbor_parent = gparent; } else { - neighbor_base_parent = - leftmost_route_node(GET_RIGHT(parent_container)); + neighbor_parent = leftmost_route_node(GET_RIGHT(parent)); } } } else { /* Symetric case */ - neighbor_base_container = rightmost_base_node(GET_LEFT_ACQB(parent_container)); - neighbor_base = &neighbor_base_container->u.base; - if (try_wlock_base_node(neighbor_base)) { + ASSERT(GET_RIGHT(parent) == thiz); + neighbor = rightmost_base_node(GET_LEFT_ACQB(parent)); + if (try_wlock_base_node(&neighbor->u.base)) { /* Failed to acquire lock */ - base->lock_statistics = 0; - wunlock_base_node(base); + thiz->u.base.lock_statistics = 0; + wunlock_base_node(&thiz->u.base); return; - } else if (!neighbor_base->is_valid) { - base->lock_statistics = 0; - wunlock_base_node(base); - wunlock_base_node(neighbor_base); + } else if (!neighbor->u.base.is_valid) { + thiz->u.base.lock_statistics = 0; + wunlock_base_node(&thiz->u.base); + wunlock_base_node(&neighbor->u.base); return; } else { lock_route_node(parent); - parent->is_valid = 0; - neighbor_base->is_valid = 0; - base->is_valid = 0; + parent->u.route.is_valid = 0; + neighbor->u.base.is_valid = 0; + thiz->u.base.is_valid = 0; gparent = NULL; - gparent_container = NULL; do { if (gparent != NULL) { unlock_route_node(gparent); } - gparent_container = parent_of(tb, parent_container); - if (gparent_container != NULL) { - gparent = &gparent_container->u.route; + gparent = parent_of(tb, parent); + if (gparent != NULL) { lock_route_node(gparent); } else { gparent = NULL; } - } while (gparent != NULL && !gparent->is_valid); + } while (gparent != NULL && !gparent->u.route.is_valid); if (gparent == NULL) { - SET_ROOT_RELB(tb, GET_LEFT(parent_container)); - } else if (GET_RIGHT(gparent_container) == parent_container) { - SET_RIGHT_RELB(gparent_container, GET_LEFT(parent_container)); + SET_ROOT_RELB(tb, GET_LEFT(parent)); + } else if (GET_RIGHT(gparent) == parent) { + SET_RIGHT_RELB(gparent, GET_LEFT(parent)); } else { - SET_LEFT_RELB(gparent_container, GET_LEFT(parent_container)); + SET_LEFT_RELB(gparent, GET_LEFT(parent)); } unlock_route_node(parent); if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor_base = create_catree_base_node(tb); - new_neighbor_base->u.base.root = - join_trees(neighbor_base->root, base->root); - neighbor_base_parent = NULL; - if (GET_LEFT(parent_container) == neighbor_base_container) { - neighbor_base_parent = gparent_container; + new_neighbor = create_catree_base_node(tb); + new_neighbor->u.base.root = join_trees(neighbor->u.base.root, + thiz->u.base.root); + neighbor_parent = NULL; + if (GET_LEFT(parent) == neighbor) { + neighbor_parent = gparent; } else { - neighbor_base_parent = - rightmost_route_node(GET_LEFT(parent_container)); + neighbor_parent = + rightmost_route_node(GET_LEFT(parent)); } } } /* Link in new neighbor and free nodes that are no longer in the tree */ - if (neighbor_base_parent == NULL) { - SET_ROOT_RELB(tb, new_neighbor_base); - } else if (GET_LEFT(neighbor_base_parent) == neighbor_base_container) { - SET_LEFT_RELB(neighbor_base_parent, new_neighbor_base); + if (neighbor_parent == NULL) { + SET_ROOT_RELB(tb, new_neighbor); + } else if (GET_LEFT(neighbor_parent) == neighbor) { + SET_LEFT_RELB(neighbor_parent, new_neighbor); } else { - SET_RIGHT_RELB(neighbor_base_parent, new_neighbor_base); + SET_RIGHT_RELB(neighbor_parent, new_neighbor); } - wunlock_base_node(base); - wunlock_base_node(neighbor_base); + wunlock_base_node(&thiz->u.base); + wunlock_base_node(&neighbor->u.base); /* Free the parent and base */ erts_schedule_db_free(&tb->common, do_free_route_node, - parent_container, - &parent->free_item, - sizeof_route_node(parent->key.size)); + parent, + &parent->u.route.free_item, + sizeof_route_node(parent->u.route.key.size)); erts_schedule_db_free(&tb->common, do_free_base_node, - base_container, - &base->free_item, + thiz, + &thiz->u.base.free_item, sizeof(DbTableCATreeNode)); erts_schedule_db_free(&tb->common, do_free_base_node, - neighbor_base_container, - &neighbor_base->free_item, + neighbor, + &neighbor->u.base.free_item, sizeof(DbTableCATreeNode)); } - -static void split_catree(DbTableCommon *tb, - DbTableCATreeNode *parent_container, - DbTableCATreeNode *base_container) { +static void split_catree(DbTableCATree *tb, + DbTableCATreeNode *parent, + DbTableCATreeNode *base) { TreeDbTerm *splitOutWriteBack; - TreeDbTerm *leftWriteBack; - TreeDbTerm *rightWriteBack; - DbTableCATreeNode *left_base_node; - DbTableCATreeNode *right_base_node; - DbTableCATreeNode *routing_node_container; - DbTableCATreeBaseNode *base = &base_container->u.base; - DbTableCATreeRouteNode *parent; - if (parent_container == NULL) { - parent = NULL; - } else { - parent = &parent_container->u.route; - } + DbTableCATreeNode *new_left; + DbTableCATreeNode *new_right; + DbTableCATreeNode *new_route; - if (less_than_two_elements(base->root)) { - base->lock_statistics = 0; - wunlock_base_node(base); + if (less_than_two_elements(base->u.base.root)) { + base->u.base.lock_statistics = 0; + wunlock_base_node(&base->u.base); return; } else { - /* Split the tree */ + new_left = create_catree_base_node(tb); + new_right = create_catree_base_node(tb); + split_tree(tb, - base->root, + base->u.base.root, &splitOutWriteBack, - &leftWriteBack, - &rightWriteBack); - /* Create new base nodes */ - left_base_node = - create_catree_base_node((DbTableCATree*)tb); - right_base_node = - create_catree_base_node((DbTableCATree*)tb); - left_base_node->u.base.root = leftWriteBack; - right_base_node->u.base.root = rightWriteBack; - routing_node_container = create_catree_route_node(tb, - left_base_node, - right_base_node, - &splitOutWriteBack->dbterm); + &new_left->u.base.root, + &new_right->u.base.root); + new_route = create_catree_route_node(tb, + new_left, + new_right, + &splitOutWriteBack->dbterm); if (parent == NULL) { - SET_ROOT_RELB((DbTableCATree*)tb, routing_node_container); - } else if(GET_LEFT(parent_container) == base_container) { - SET_LEFT_RELB(parent_container, routing_node_container); + SET_ROOT_RELB(tb, new_route); + } else if(GET_LEFT(parent) == base) { + SET_LEFT_RELB(parent, new_route); } else { - SET_RIGHT_RELB(parent_container, routing_node_container); + SET_RIGHT_RELB(parent, new_route); } - base->is_valid = 0; - wunlock_base_node(base); - erts_schedule_db_free(tb, + base->u.base.is_valid = 0; + wunlock_base_node(&base->u.base); + erts_schedule_db_free(&tb->common, do_free_base_node, - base_container, - &base->free_item, + base, + &base->u.base.free_item, sizeof(DbTableCATreeNode)); } } @@ -1729,8 +1660,7 @@ static int db_next_catree(Process *p, DbTable *tbl, Eterm key, Eterm *ret) static int db_last_catree(Process *p, DbTable *tbl, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; - DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); + DbTableCATreeNode *base = merge_to_one_locked_base_node(&tbl->catree); int result = db_last_tree_common(p, tbl, base->u.base.root, ret, NULL); wunlock_base_node(&(base->u.base)); return result; @@ -1741,11 +1671,10 @@ static int db_prev_catree(Process *p, DbTable *tbl, Eterm key, Eterm *ret) { DbTreeStack stack; TreeDbTerm * stack_array[STACK_NEED]; - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); - base = merge_to_one_locked_base_node(&tb->common); + base = merge_to_one_locked_base_node(&tbl->catree); result = db_prev_tree_common(p, tbl, base->u.base.root, key, ret, &stack); wunlock_base_node(&(base->u.base)); return result; @@ -1848,10 +1777,9 @@ static int db_erase_object_catree(DbTable *tbl, Eterm object, Eterm *ret) static int db_slot_catree(Process *p, DbTable *tbl, Eterm slot_term, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); + base = merge_to_one_locked_base_node(&tbl->catree); result = db_slot_tree_common(p, tbl, base->u.base.root, slot_term, ret, NULL); wunlock_base_node(&(base->u.base)); @@ -1863,11 +1791,10 @@ static int db_select_continue_catree(Process *p, Eterm continuation, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_continue_tree_common(p, &tb->common, &base->u.base.root, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_continue_tree_common(p, &tbl->common, &base->u.base.root, continuation, ret, NULL); wunlock_base_node(&(base->u.base)); return result; @@ -1877,11 +1804,10 @@ static int db_select_continue_catree(Process *p, static int db_select_catree(Process *p, DbTable *tbl, Eterm tid, Eterm pattern, int reverse, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_tree_common(p, &tb->common, &base->u.base.root, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_tree_common(p, &tbl->common, &base->u.base.root, tid, pattern, reverse, ret, NULL); wunlock_base_node(&(base->u.base)); @@ -1893,11 +1819,10 @@ static int db_select_count_continue_catree(Process *p, Eterm continuation, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_count_continue_tree_common(p, &tb->common, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_count_continue_tree_common(p, &tbl->common, &base->u.base.root, continuation, ret, NULL); wunlock_base_node(&(base->u.base)); @@ -1907,11 +1832,10 @@ static int db_select_count_continue_catree(Process *p, static int db_select_count_catree(Process *p, DbTable *tbl, Eterm tid, Eterm pattern, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_count_tree_common(p, &tb->common, &base->u.base.root, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_count_tree_common(p, &tbl->common, &base->u.base.root, tid, pattern, ret, NULL); wunlock_base_node(&(base->u.base)); return result; @@ -1921,11 +1845,10 @@ static int db_select_chunk_catree(Process *p, DbTable *tbl, Eterm tid, Eterm pattern, Sint chunk_size, int reversed, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_chunk_tree_common(p, &tb->common, &base->u.base.root, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_chunk_tree_common(p, &tbl->common, &base->u.base.root, tid, pattern, chunk_size, reversed, ret, NULL); wunlock_base_node(&(base->u.base)); return result; @@ -1936,13 +1859,12 @@ static int db_select_delete_continue_catree(Process *p, Eterm continuation, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; DbTreeStack stack; TreeDbTerm * stack_array[STACK_NEED]; int result; DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); - base = merge_to_one_locked_base_node(&tb->common); + base = merge_to_one_locked_base_node(&tbl->catree); result = db_select_delete_continue_tree_common(p, tbl, &base->u.base.root, continuation, ret, &stack); wunlock_base_node(&(base->u.base)); @@ -1952,13 +1874,12 @@ static int db_select_delete_continue_catree(Process *p, static int db_select_delete_catree(Process *p, DbTable *tbl, Eterm tid, Eterm pattern, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; DbTreeStack stack; TreeDbTerm * stack_array[STACK_NEED]; int result; DbTableCATreeNode *base; init_tree_stack(&stack, stack_array, 0); - base = merge_to_one_locked_base_node(&tb->common); + base = merge_to_one_locked_base_node(&tbl->catree); result = db_select_delete_tree_common(p, tbl, &base->u.base.root, tid, pattern, ret, &stack); wunlock_base_node(&(base->u.base)); @@ -1968,11 +1889,10 @@ static int db_select_delete_catree(Process *p, DbTable *tbl, Eterm tid, static int db_select_replace_catree(Process *p, DbTable *tbl, Eterm tid, Eterm pattern, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_replace_tree_common(p, &tb->common, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_replace_tree_common(p, &tbl->common, &base->u.base.root, tid, pattern, ret, NULL); wunlock_base_node(&(base->u.base)); @@ -1982,11 +1902,10 @@ static int db_select_replace_catree(Process *p, DbTable *tbl, Eterm tid, static int db_select_replace_continue_catree(Process *p, DbTable *tbl, Eterm continuation, Eterm *ret) { - DbTableCATree *tb = &tbl->catree; int result; DbTableCATreeNode *base; - base = merge_to_one_locked_base_node(&tb->common); - result = db_select_replace_continue_tree_common(p, &tb->common, + base = merge_to_one_locked_base_node(&tbl->catree); + result = db_select_replace_continue_tree_common(p, &tbl->common, &base->u.base.root, continuation, ret, NULL); wunlock_base_node(&(base->u.base)); @@ -2015,8 +1934,7 @@ static int db_take_catree(Process *p, DbTable *tbl, Eterm key, Eterm *ret) static void db_print_catree(fmtfn_t to, void *to_arg, int show, DbTable *tbl) { - DbTableCATree *tb = &tbl->catree; - DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); + DbTableCATreeNode *base = merge_to_one_locked_base_node(&tbl->catree); db_print_tree_common(to, to_arg, show, base->u.base.root, tbl); wunlock_base_node(&(base->u.base)); } @@ -2094,8 +2012,7 @@ static void db_foreach_offheap_catree(DbTable *tbl, void (*func)(ErlOffHeap *, void *), void *arg) { - DbTableCATree *tb = &tbl->catree; - DbTableCATreeNode *base = merge_to_one_locked_base_node(&tb->common); + DbTableCATreeNode *base = merge_to_one_locked_base_node(&tbl->catree); db_foreach_offheap_tree_common(base->u.base.root, func, arg); wunlock_base_node(&(base->u.base)); } -- cgit v1.2.3 From 3ed92c67b957df55762ab837a7e3d9c8d562083b Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Thu, 27 Sep 2018 12:21:55 +0200 Subject: erts: Add some ERTS_RESTRICT pointers --- erts/emulator/beam/erl_db_catree.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 65ed7ab81d..7ce5d79acf 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -1441,12 +1441,12 @@ static void join_catree(DbTableCATree *tb, } static void split_catree(DbTableCATree *tb, - DbTableCATreeNode *parent, - DbTableCATreeNode *base) { + DbTableCATreeNode* ERTS_RESTRICT parent, + DbTableCATreeNode* ERTS_RESTRICT base) { TreeDbTerm *splitOutWriteBack; - DbTableCATreeNode *new_left; - DbTableCATreeNode *new_right; - DbTableCATreeNode *new_route; + DbTableCATreeNode* ERTS_RESTRICT new_left; + DbTableCATreeNode* ERTS_RESTRICT new_right; + DbTableCATreeNode* ERTS_RESTRICT new_route; if (less_than_two_elements(base->u.base.root)) { base->u.base.lock_statistics = 0; -- cgit v1.2.3 From b0fea529419514d9b173b84aae0f188f2e38c931 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Mon, 1 Oct 2018 20:37:38 +0200 Subject: erts: Add Erlang term order to lock checker --- erts/emulator/beam/erl_lock_check.c | 72 +++++++++++++++++++++++-------------- erts/emulator/beam/erl_lock_flags.h | 6 ++-- 2 files changed, 50 insertions(+), 28 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_lock_check.c b/erts/emulator/beam/erl_lock_check.c index 3b10ef8c25..43edff8914 100644 --- a/erts/emulator/beam/erl_lock_check.c +++ b/erts/emulator/beam/erl_lock_check.c @@ -42,6 +42,7 @@ #include "erl_term.h" #include "erl_threads.h" #include "erl_atom_table.h" +#include "erl_utils.h" typedef struct { char *name; @@ -729,6 +730,14 @@ erts_lc_is_check_order(char *name) return 1; } +static int +lc_is_term_order(Sint16 id) +{ + return erts_lock_order[id].internal_order != NULL + && sys_strcmp(erts_lock_order[id].internal_order, "term") == 0; +} + + static int compare_locked_by_id(lc_locked_lock_t *locked_lock, erts_lc_lock_t *comparand) { if(locked_lock->id < comparand->id) { @@ -740,18 +749,23 @@ static int compare_locked_by_id(lc_locked_lock_t *locked_lock, erts_lc_lock_t *c return 0; } -static int compare_locked_by_id_extra(lc_locked_lock_t *locked_lock, erts_lc_lock_t *comparand) +static int compare_locked_by_id_extra(lc_locked_lock_t *ll, erts_lc_lock_t *comparand) { - int order = compare_locked_by_id(locked_lock, comparand); + int order = compare_locked_by_id(ll, comparand); if(order) { return order; - } else if(locked_lock->extra < comparand->extra) { + } + if (ll->flags & ERTS_LOCK_FLAGS_PROPERTY_TERM_ORDER) { + ASSERT(!is_header(ll->extra) && !is_header(comparand->extra)); + return CMP(ll->extra, comparand->extra); + } + + if(ll->extra < comparand->extra) { return -1; - } else if(locked_lock->extra > comparand->extra) { + } else if(ll->extra > comparand->extra) { return 1; } - return 0; } @@ -990,7 +1004,8 @@ erts_lc_trylock_force_busy_flg(erts_lc_lock_t *lck, erts_lock_options_t options) return 0; } else { - lc_locked_lock_t *tl_lck; + lc_locked_lock_t *ll; + int order; ASSERT(thr->locked.last); @@ -999,25 +1014,28 @@ erts_lc_trylock_force_busy_flg(erts_lc_lock_t *lck, erts_lock_options_t options) type_order_violation("trylocking ", thr, lck); #endif - if (thr->locked.last->id < lck->id - || (thr->locked.last->id == lck->id - && thr->locked.last->extra < lck->extra)) - return 0; + ll = thr->locked.last; + order = compare_locked_by_id_extra(ll, lck); + + if (order < 0) + return 0; /* - * Lock order violation + * TryLock order violation */ - if (lck->check_order) { /* Check that we are not trying to lock this lock twice */ - for (tl_lck = thr->locked.last; tl_lck; tl_lck = tl_lck->prev) { - if (tl_lck->id < lck->id - || (tl_lck->id == lck->id && tl_lck->extra <= lck->extra)) { - if (tl_lck->id == lck->id && tl_lck->extra == lck->extra) + while (1) { + if (order <= 0) { + if (order == 0) lock_twice("Trylocking", thr, lck, options); break; } + ll = ll->prev; + if (!ll) + break; + order = compare_locked_by_id_extra(ll, lck); } } @@ -1066,9 +1084,9 @@ void erts_lc_trylock_flg_x(int locked, erts_lc_lock_t *lck, erts_lock_options_t #endif for (tl_lck = thr->locked.last; tl_lck; tl_lck = tl_lck->prev) { - if (tl_lck->id < lck->id - || (tl_lck->id == lck->id && tl_lck->extra <= lck->extra)) { - if (tl_lck->id == lck->id && tl_lck->extra == lck->extra && lck->check_order) + int order = compare_locked_by_id_extra(tl_lck, lck); + if (order <= 0) { + if (order == 0 && lck->check_order) lock_twice("Trylocking", thr, lck, options); if (locked) { ll->next = tl_lck->next; @@ -1111,10 +1129,10 @@ void erts_lc_require_lock_flg(erts_lc_lock_t *lck, erts_lock_options_t options, for (l_lck2 = thr->required.last; l_lck2; l_lck2 = l_lck2->prev) { - if (l_lck2->id < lck->id - || (l_lck2->id == lck->id && l_lck2->extra < lck->extra)) + int order = compare_locked_by_id_extra(l_lck2, lck); + if (order < 0) break; - else if (l_lck2->id == lck->id && l_lck2->extra == lck->extra) + if (order == 0) require_twice(thr, lck); } if (!l_lck2) { @@ -1172,6 +1190,7 @@ void erts_lc_lock_flg_x(erts_lc_lock_t *lck, erts_lock_options_t options, { lc_thread_t *thr; lc_locked_lock_t *new_ll; + int order; if (lck->inited != ERTS_LC_INITITALIZED) uninitialized_lock(); @@ -1187,11 +1206,10 @@ void erts_lc_lock_flg_x(erts_lc_lock_t *lck, erts_lock_options_t options, thr->locked.last = thr->locked.first = new_ll; ASSERT(0 < lck->id && lck->id < ERTS_LOCK_ORDER_SIZE); thr->matrix.m[lck->id][0] = 1; + return; } - else if (( ! lck->check_order && thr->locked.last->id == lck->id) || - (thr->locked.last->id < lck->id - || (thr->locked.last->id == lck->id - && thr->locked.last->extra < lck->extra))) { + order = compare_locked_by_id_extra(thr->locked.last, lck); + if (order < 0 || (!lck->check_order && order == 0)) { lc_locked_lock_t* ll; if (LOCK_IS_TYPE_ORDER_VIOLATION(lck->flags, thr->locked.last->flags)) { type_order_violation("locking ", thr, lck); @@ -1337,6 +1355,8 @@ erts_lc_init_lock_x(erts_lc_lock_t *lck, char *name, erts_lock_flags_t flags, Et lck->extra = extra; ASSERT(is_immed(lck->extra)); lck->flags = flags; + if (lc_is_term_order(lck->id)) + lck->flags |= ERTS_LOCK_FLAGS_PROPERTY_TERM_ORDER; lck->taken_options = 0; lck->inited = ERTS_LC_INITITALIZED; } diff --git a/erts/emulator/beam/erl_lock_flags.h b/erts/emulator/beam/erl_lock_flags.h index d711f69456..2db133b598 100644 --- a/erts/emulator/beam/erl_lock_flags.h +++ b/erts/emulator/beam/erl_lock_flags.h @@ -28,15 +28,17 @@ /* Property/category are bitfields to simplify their use in masks. */ #define ERTS_LOCK_FLAGS_MASK_CATEGORY (0xFFC0) -#define ERTS_LOCK_FLAGS_MASK_PROPERTY (0x0030) +#define ERTS_LOCK_FLAGS_MASK_PROPERTY (0x0038) /* Type is a plain number. */ -#define ERTS_LOCK_FLAGS_MASK_TYPE (0x000F) +#define ERTS_LOCK_FLAGS_MASK_TYPE (0x0007) #define ERTS_LOCK_FLAGS_TYPE_SPINLOCK (1) #define ERTS_LOCK_FLAGS_TYPE_MUTEX (2) #define ERTS_LOCK_FLAGS_TYPE_PROCLOCK (3) +/* Lock checker use real term order instead of raw word compare */ +#define ERTS_LOCK_FLAGS_PROPERTY_TERM_ORDER (1 << 3) /* "Static" guarantees that the lock will never be destroyed once created. */ #define ERTS_LOCK_FLAGS_PROPERTY_STATIC (1 << 4) #define ERTS_LOCK_FLAGS_PROPERTY_READ_WRITE (1 << 5) -- cgit v1.2.3 From d988f91307b2922de79f92d3b9aa160ac947b44c Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 18:04:38 +0200 Subject: erts: Add lock order check of erl_db_catree base nodes Lock order is key term order, so each base node needs its own key if lock check is enabled. --- erts/emulator/beam/erl_db_catree.c | 174 ++++++++++++++++++++++++------------ erts/emulator/beam/erl_db_catree.h | 10 +++ erts/emulator/beam/erl_lock_check.c | 16 +++- erts/emulator/beam/erl_lock_check.h | 4 +- 4 files changed, 141 insertions(+), 63 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 7ce5d79acf..cadfc3b405 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -782,27 +782,71 @@ void unlock_route_node(DbTableCATreeNode *route_node) } +#ifdef ERTS_ENABLE_LOCK_CHECK +# define sizeof_base_node(KEY_SZ) \ + (offsetof(DbTableCATreeNode, u.base.lc.key_heap) \ + + (KEY_SZ)*sizeof(Eterm)) +# define LC_KEY(KEY) KEY +#else +# define sizeof_base_node(KEY_SZ) \ + offsetof(DbTableCATreeNode, u.base.end_of_struct__) +# define LC_KEY(KEY) NIL +#endif -static DbTableCATreeNode *create_catree_base_node(DbTableCATree *tb) +static DbTableCATreeNode *create_base_node(DbTableCATree *tb, + TreeDbTerm* root, + Eterm lc_key) { - DbTableCATreeNode *p = erts_db_alloc(ERTS_ALC_T_DB_TABLE, - (DbTable *) tb, - sizeof(DbTableCATreeNode)); + DbTableCATreeNode *p; erts_rwmtx_opt_t rwmtx_opt = ERTS_RWMTX_OPT_DEFAULT_INITER; +#ifdef ERTS_ENABLE_LOCK_CHECK + Eterm lc_key_size = size_object(lc_key); +#endif + p = erts_db_alloc(ERTS_ALC_T_DB_TABLE, (DbTable *) tb, + sizeof_base_node(lc_key_size)); + p->is_base_node = 1; - p->u.base.root = NULL; + p->u.base.root = root; if (tb->common.type & DB_FREQ_READ) rwmtx_opt.type = ERTS_RWMTX_TYPE_FREQUENT_READ; if (erts_ets_rwmtx_spin_count >= 0) rwmtx_opt.main_spincount = erts_ets_rwmtx_spin_count; + +#ifdef ERTS_ENABLE_LOCK_CHECK + { + Eterm* hp = &p->u.base.lc.key_heap[0]; + ErlOffHeap oh; + + oh.first = NULL; + lc_key = copy_struct(lc_key, lc_key_size, &hp, &oh); + p->u.base.lc.key = lc_key; + p->u.base.lc.key_size = lc_key_size; + p->u.base.lc.key_oh = oh.first; + } +#endif erts_rwmtx_init_opt(&p->u.base.lock, &rwmtx_opt, - "erl_db_catree_base_node", tb->common.the_name, + "erl_db_catree_base_node", + lc_key, ERTS_LOCK_FLAGS_CATEGORY_DB); p->u.base.lock_statistics = 0; p->u.base.is_valid = 1; return p; } +static ERTS_INLINE +DbTableCATreeNode *create_wlocked_base_node(DbTableCATree *tb, + TreeDbTerm* root, + Eterm lc_key) +{ + DbTableCATreeNode* p = create_base_node(tb, root, lc_key); + ethr_rwmutex_rwlock(&p->u.base.lock.rwmtx); +#ifdef ERTS_ENABLE_LOCK_CHECK + erts_lc_trylock_flg(-1, &p->u.base.lock.lc, ERTS_LOCK_OPTIONS_RDWR); +#endif + return p; +} + + static ERTS_INLINE Uint sizeof_route_node(Uint key_size) { return (offsetof(DbTableCATreeNode, u.route.key.tpl[1]) @@ -810,10 +854,10 @@ static ERTS_INLINE Uint sizeof_route_node(Uint key_size) } static DbTableCATreeNode* -create_catree_route_node(DbTableCATree *tb, - DbTableCATreeNode *left, - DbTableCATreeNode *right, - DbTerm * keyTerm) +create_route_node(DbTableCATree *tb, + DbTableCATreeNode *left, + DbTableCATreeNode *right, + DbTerm * keyTerm) { Eterm* top; Eterm key = GETKEY(tb,keyTerm->tpl); @@ -848,13 +892,20 @@ static void do_free_base_node(void* vptr) DbTableCATreeNode *p = (DbTableCATreeNode *)vptr; ASSERT(p->is_base_node); erts_rwmtx_destroy(&p->u.base.lock); +#ifdef ERTS_ENABLE_LOCK_CHECK + if (p->u.base.lc.key_size != 0) { + ErlOffHeap oh; + oh.first = p->u.base.lc.key_oh; + erts_cleanup_offheap(&oh); + } +#endif 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); + ERTS_DB_ALC_MEM_UPDATE_(tb, sizeof_base_node(p->u.base.lc.key_size), 0); do_free_base_node(p); } @@ -1174,32 +1225,31 @@ erl_db_catree_force_join_right(DbTableCATree *tb, DbTableCATreeNode *neighbor; DbTableCATreeNode *new_neighbor; DbTableCATreeNode *neighbor_parent; - int neighbour_not_valid; + if (parent == NULL) { return NULL; } - do { + ASSERT(thiz == GET_LEFT(parent)); + while (1) { neighbor = leftmost_base_node(GET_RIGHT_ACQB(parent)); wlock_base_node_no_stats(&neighbor->u.base); - neighbour_not_valid = !neighbor->u.base.is_valid; - if (neighbour_not_valid) { - wunlock_base_node(&neighbor->u.base); - } - } while (neighbour_not_valid); + if (neighbor->u.base.is_valid) + break; + wunlock_base_node(&neighbor->u.base); + } lock_route_node(parent); parent->u.route.is_valid = 0; neighbor->u.base.is_valid = 0; thiz->u.base.is_valid = 0; - gparent = NULL; - do { - if (gparent != NULL) { - unlock_route_node(gparent); - } + while (1) { gparent = parent_of(tb, parent); - if (gparent != NULL) { - lock_route_node(gparent); - } - } while (gparent != NULL && !gparent->u.route.is_valid); + if (gparent == NULL) + break; + lock_route_node(gparent); + if (gparent->u.route.is_valid) + break; + unlock_route_node(gparent); + } if (gparent == NULL) { SET_ROOT_RELB(tb, GET_RIGHT(parent)); } else if (GET_LEFT(gparent) == parent) { @@ -1211,10 +1261,14 @@ erl_db_catree_force_join_right(DbTableCATree *tb, if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor = create_catree_base_node(tb); - new_neighbor->u.base.root = - join_trees(thiz->u.base.root, neighbor->u.base.root); - wlock_base_node_no_stats(&(new_neighbor->u.base)); + + { + TreeDbTerm* new_root = join_trees(thiz->u.base.root, + neighbor->u.base.root); + new_neighbor = create_wlocked_base_node(tb, new_root, + LC_KEY(thiz->u.base.lc.key)); + } + if (GET_RIGHT(parent) == neighbor) { neighbor_parent = gparent; } else { @@ -1239,12 +1293,12 @@ erl_db_catree_force_join_right(DbTableCATree *tb, do_free_base_node, thiz, &thiz->u.base.free_item, - sizeof(DbTableCATreeNode)); + sizeof_base_node(thiz->u.base.lc.key_size)); erts_schedule_db_free(&tb->common, do_free_base_node, neighbor, &neighbor->u.base.free_item, - sizeof(DbTableCATreeNode)); + sizeof_base_node(neighbor->u.base.lc.key_size)); if (parent == neighbor) { *result_parent_wb = gparent; @@ -1349,10 +1403,12 @@ static void join_catree(DbTableCATree *tb, if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor = create_catree_base_node(tb); - new_neighbor->u.base.root = join_trees(thiz->u.base.root, - neighbor->u.base.root); - neighbor_parent = NULL; + { + TreeDbTerm* new_root = join_trees(thiz->u.base.root, + neighbor->u.base.root); + new_neighbor = create_base_node(tb, new_root, + LC_KEY(thiz->u.base.lc.key)); + } if (GET_RIGHT(parent) == neighbor) { neighbor_parent = gparent; } else { @@ -1400,10 +1456,12 @@ static void join_catree(DbTableCATree *tb, if (gparent != NULL) { unlock_route_node(gparent); } - new_neighbor = create_catree_base_node(tb); - new_neighbor->u.base.root = join_trees(neighbor->u.base.root, - thiz->u.base.root); - neighbor_parent = NULL; + { + TreeDbTerm* new_root = join_trees(neighbor->u.base.root, + thiz->u.base.root); + new_neighbor = create_base_node(tb, new_root, + LC_KEY(thiz->u.base.lc.key)); + } if (GET_LEFT(parent) == neighbor) { neighbor_parent = gparent; } else { @@ -1432,12 +1490,12 @@ static void join_catree(DbTableCATree *tb, do_free_base_node, thiz, &thiz->u.base.free_item, - sizeof(DbTableCATreeNode)); + sizeof_base_node(thiz->u.base.lc.key_size)); erts_schedule_db_free(&tb->common, do_free_base_node, neighbor, &neighbor->u.base.free_item, - sizeof(DbTableCATreeNode)); + sizeof_base_node(neighbor->u.base.lc.key_size)); } static void split_catree(DbTableCATree *tb, @@ -1453,18 +1511,20 @@ static void split_catree(DbTableCATree *tb, wunlock_base_node(&base->u.base); return; } else { - new_left = create_catree_base_node(tb); - new_right = create_catree_base_node(tb); - - split_tree(tb, - base->u.base.root, - &splitOutWriteBack, - &new_left->u.base.root, - &new_right->u.base.root); - new_route = create_catree_route_node(tb, - new_left, - new_right, - &splitOutWriteBack->dbterm); + TreeDbTerm *left_tree; + TreeDbTerm *right_tree; + + split_tree(tb, base->u.base.root, &splitOutWriteBack, + &left_tree, &right_tree); + + new_left = create_base_node(tb, left_tree, + LC_KEY(GETKEY(tb, left_tree->dbterm.tpl))); + new_right = create_base_node(tb, right_tree, + LC_KEY(GETKEY(tb, right_tree->dbterm.tpl))); + new_route = create_route_node(tb, + new_left, + new_right, + &splitOutWriteBack->dbterm); if (parent == NULL) { SET_ROOT_RELB(tb, new_route); } else if(GET_LEFT(parent) == base) { @@ -1478,7 +1538,7 @@ static void split_catree(DbTableCATree *tb, do_free_base_node, base, &base->u.base.free_item, - sizeof(DbTableCATreeNode)); + sizeof_base_node(base->u.base.lc.key_size)); } } @@ -1606,7 +1666,7 @@ void db_initialize_catree(void) int db_create_catree(Process *p, DbTable *tbl) { DbTableCATree *tb = &tbl->catree; - DbTableCATreeNode *root = create_catree_base_node(tb); + DbTableCATreeNode *root = create_base_node(tb, NULL, NIL); tb->deletion = 0; tb->base_nodes_to_free_list = NULL; erts_atomic_init_relb(&(tb->root), (erts_aint_t)root); diff --git a/erts/emulator/beam/erl_db_catree.h b/erts/emulator/beam/erl_db_catree.h index 4d3e75bfe0..07a48d5bd3 100644 --- a/erts/emulator/beam/erl_db_catree.h +++ b/erts/emulator/beam/erl_db_catree.h @@ -40,6 +40,16 @@ typedef struct { TreeDbTerm *root; /* The root of the sequential tree */ ErtsThrPrgrLaterOp free_item; /* Used when freeing using thread progress */ struct DbTableCATreeNode * next; /* Used when gradually deleting */ + +#ifdef ERTS_ENABLE_LOCK_CHECK + struct { + Eterm key; + struct erl_off_heap_header* key_oh; + Uint key_size; + Eterm key_heap[1]; + } lc; +#endif + char end_of_struct__; } DbTableCATreeBaseNode; typedef struct { diff --git a/erts/emulator/beam/erl_lock_check.c b/erts/emulator/beam/erl_lock_check.c index 43edff8914..7cc347f810 100644 --- a/erts/emulator/beam/erl_lock_check.c +++ b/erts/emulator/beam/erl_lock_check.c @@ -92,7 +92,7 @@ static erts_lc_lock_order_t erts_lock_order[] = { { "db_tab", "address" }, { "db_tab_fix", "address" }, { "db_hash_slot", "address" }, - { "erl_db_catree_base_node", "dynamic" }, + { "erl_db_catree_base_node", "term" }, { "erl_db_catree_route_node", "dynamic" }, { "resource_monitors", "address" }, { "driver_list", NULL }, @@ -1056,6 +1056,11 @@ erts_lc_trylock_force_busy_flg(erts_lc_lock_t *lck, erts_lock_options_t options) #endif } +/* + * locked = 0 trylock failed + * locked > 0 trylock succeeded + * locked < 0 prelocking of newly created lock (no lock order check) + */ void erts_lc_trylock_flg_x(int locked, erts_lc_lock_t *lck, erts_lock_options_t options, char *file, unsigned int line) { @@ -1086,7 +1091,7 @@ void erts_lc_trylock_flg_x(int locked, erts_lc_lock_t *lck, erts_lock_options_t for (tl_lck = thr->locked.last; tl_lck; tl_lck = tl_lck->prev) { int order = compare_locked_by_id_extra(tl_lck, lck); if (order <= 0) { - if (order == 0 && lck->check_order) + if (order == 0 && lck->check_order && locked >= 0) lock_twice("Trylocking", thr, lck, options); if (locked) { ll->next = tl_lck->next; @@ -1353,10 +1358,13 @@ erts_lc_init_lock_x(erts_lc_lock_t *lck, char *name, erts_lock_flags_t flags, Et lck->id = erts_lc_get_lock_order_id(name); lck->check_order = erts_lc_is_check_order(name); lck->extra = extra; - ASSERT(is_immed(lck->extra)); lck->flags = flags; - if (lc_is_term_order(lck->id)) + if (lc_is_term_order(lck->id)) { lck->flags |= ERTS_LOCK_FLAGS_PROPERTY_TERM_ORDER; + ASSERT(!is_header(lck->extra)); + } + else + ASSERT(is_immed(lck->extra)); lck->taken_options = 0; lck->inited = ERTS_LC_INITITALIZED; } diff --git a/erts/emulator/beam/erl_lock_check.h b/erts/emulator/beam/erl_lock_check.h index 472e88ad65..965b73e27c 100644 --- a/erts/emulator/beam/erl_lock_check.h +++ b/erts/emulator/beam/erl_lock_check.h @@ -106,7 +106,7 @@ Eterm erts_lc_dump_graph(void); #define erts_lc_lock(lck) erts_lc_lock_x(lck,__FILE__,__LINE__) #define erts_lc_trylock(res,lck) erts_lc_trylock_x(res,lck,__FILE__,__LINE__) -#define erts_lc_lock_flg(lck) erts_lc_lock_flg_x(lck,__FILE__,__LINE__) -#define erts_lc_trylock_flg(res,lck) erts_lc_trylock_flg_x(res,lck,__FILE__,__LINE__) +#define erts_lc_lock_flg(lck,flg) erts_lc_lock_flg_x(lck,flg,__FILE__,__LINE__) +#define erts_lc_trylock_flg(res,lck,flg) erts_lc_trylock_flg_x(res,lck,flg,__FILE__,__LINE__) #endif /* #ifndef ERTS_LOCK_CHECK_H__ */ -- cgit v1.2.3 From 4324d0a42124a9e9be42c8bb2879db4660acb9e9 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 17:17:57 +0200 Subject: erts: Add lock order check for route nodes Lock order is reverse tree depth, from leafs toward root. This solution may eventually fail if running too long as route nodes do not increase their 'lc_order' in a join operation when they move up in the tree. But who runs a VM with lock-checker for such a long time? --- erts/emulator/beam/erl_db_catree.c | 34 ++++++++++++++++++++++++---------- erts/emulator/beam/erl_db_catree.h | 3 +++ erts/emulator/beam/erl_lock_check.c | 2 +- 3 files changed, 28 insertions(+), 11 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index cadfc3b405..77fd07277f 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -786,11 +786,11 @@ void unlock_route_node(DbTableCATreeNode *route_node) # define sizeof_base_node(KEY_SZ) \ (offsetof(DbTableCATreeNode, u.base.lc.key_heap) \ + (KEY_SZ)*sizeof(Eterm)) -# define LC_KEY(KEY) KEY +# define LC_ORDER(ORDER) ORDER #else # define sizeof_base_node(KEY_SZ) \ offsetof(DbTableCATreeNode, u.base.end_of_struct__) -# define LC_KEY(KEY) NIL +# define LC_ORDER(ORDER) NIL #endif static DbTableCATreeNode *create_base_node(DbTableCATree *tb, @@ -857,7 +857,8 @@ static DbTableCATreeNode* create_route_node(DbTableCATree *tb, DbTableCATreeNode *left, DbTableCATreeNode *right, - DbTerm * keyTerm) + DbTerm * keyTerm, + DbTableCATreeNode* lc_parent) { Eterm* top; Eterm key = GETKEY(tb,keyTerm->tpl); @@ -882,8 +883,20 @@ create_route_node(DbTableCATree *tb, p->u.route.is_valid = 1; erts_atomic_init_nob(&p->u.route.left, (erts_aint_t)left); erts_atomic_init_nob(&p->u.route.right, (erts_aint_t)right); +#ifdef ERTS_ENABLE_LOCK_CHECK + /* Route node lock order is inverse tree depth (from leafs toward root) */ + p->u.route.lc_order = (lc_parent == NULL ? MAX_SMALL : + lc_parent->u.route.lc_order - 1); + /* + * This assert may eventually fail as we don't increase 'lc_order' in join + * operations when route nodes move up in the tree. + * Tough luck if you run a lock-checking VM for such a long time on 32-bit. + */ + ERTS_LC_ASSERT(p->u.route.lc_order >= 0); +#endif erts_mtx_init(&p->u.route.lock, "erl_db_catree_route_node", - NIL, ERTS_LOCK_FLAGS_CATEGORY_DB); + LC_ORDER(make_small(p->u.route.lc_order)), + ERTS_LOCK_FLAGS_CATEGORY_DB); return p; } @@ -1266,7 +1279,7 @@ erl_db_catree_force_join_right(DbTableCATree *tb, TreeDbTerm* new_root = join_trees(thiz->u.base.root, neighbor->u.base.root); new_neighbor = create_wlocked_base_node(tb, new_root, - LC_KEY(thiz->u.base.lc.key)); + LC_ORDER(thiz->u.base.lc.key)); } if (GET_RIGHT(parent) == neighbor) { @@ -1407,7 +1420,7 @@ static void join_catree(DbTableCATree *tb, TreeDbTerm* new_root = join_trees(thiz->u.base.root, neighbor->u.base.root); new_neighbor = create_base_node(tb, new_root, - LC_KEY(thiz->u.base.lc.key)); + LC_ORDER(thiz->u.base.lc.key)); } if (GET_RIGHT(parent) == neighbor) { neighbor_parent = gparent; @@ -1460,7 +1473,7 @@ static void join_catree(DbTableCATree *tb, TreeDbTerm* new_root = join_trees(neighbor->u.base.root, thiz->u.base.root); new_neighbor = create_base_node(tb, new_root, - LC_KEY(thiz->u.base.lc.key)); + LC_ORDER(thiz->u.base.lc.key)); } if (GET_LEFT(parent) == neighbor) { neighbor_parent = gparent; @@ -1518,13 +1531,14 @@ static void split_catree(DbTableCATree *tb, &left_tree, &right_tree); new_left = create_base_node(tb, left_tree, - LC_KEY(GETKEY(tb, left_tree->dbterm.tpl))); + LC_ORDER(GETKEY(tb, left_tree->dbterm.tpl))); new_right = create_base_node(tb, right_tree, - LC_KEY(GETKEY(tb, right_tree->dbterm.tpl))); + LC_ORDER(GETKEY(tb, right_tree->dbterm.tpl))); new_route = create_route_node(tb, new_left, new_right, - &splitOutWriteBack->dbterm); + &splitOutWriteBack->dbterm, + parent); if (parent == NULL) { SET_ROOT_RELB(tb, new_route); } else if(GET_LEFT(parent) == base) { diff --git a/erts/emulator/beam/erl_db_catree.h b/erts/emulator/beam/erl_db_catree.h index 07a48d5bd3..0ad921d880 100644 --- a/erts/emulator/beam/erl_db_catree.h +++ b/erts/emulator/beam/erl_db_catree.h @@ -53,6 +53,9 @@ typedef struct { } DbTableCATreeBaseNode; typedef struct { +#ifdef ERTS_ENABLE_LOCK_CHECK + Sint lc_order; +#endif ErtsThrPrgrLaterOp free_item; /* 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 */ diff --git a/erts/emulator/beam/erl_lock_check.c b/erts/emulator/beam/erl_lock_check.c index 7cc347f810..d68ddcc748 100644 --- a/erts/emulator/beam/erl_lock_check.c +++ b/erts/emulator/beam/erl_lock_check.c @@ -93,7 +93,7 @@ static erts_lc_lock_order_t erts_lock_order[] = { { "db_tab_fix", "address" }, { "db_hash_slot", "address" }, { "erl_db_catree_base_node", "term" }, - { "erl_db_catree_route_node", "dynamic" }, + { "erl_db_catree_route_node", "index" }, { "resource_monitors", "address" }, { "driver_list", NULL }, { "proc_msgq", "pid" }, -- cgit v1.2.3 From 6fefb29c3cef9ff3e07f61d88eb0eb62bcde5d03 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 17:27:12 +0200 Subject: erts: Remove "dynamic" lock order support --- erts/emulator/beam/erl_lock_check.c | 48 ++++++++++--------------------------- erts/emulator/beam/erl_lock_check.h | 4 +--- 2 files changed, 13 insertions(+), 39 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_lock_check.c b/erts/emulator/beam/erl_lock_check.c index d68ddcc748..987e370341 100644 --- a/erts/emulator/beam/erl_lock_check.c +++ b/erts/emulator/beam/erl_lock_check.c @@ -710,26 +710,6 @@ erts_lc_get_lock_order_id(char *name) return (Sint16) -1; } -int -erts_lc_is_check_order(char *name) -{ - int i; - if (!name || name[0] == '\0') - erts_fprintf(stderr, "Missing lock name\n"); - - for (i = 0; i < ERTS_LOCK_ORDER_SIZE; i++) { - if (sys_strcmp(erts_lock_order[i].name, name) == 0) { - if (erts_lock_order[i].internal_order != NULL && - sys_strcmp(erts_lock_order[i].internal_order, "dynamic") == 0) { - return 0; - }else{ - return 1; - } - } - } - return 1; -} - static int lc_is_term_order(Sint16 id) { @@ -1024,19 +1004,17 @@ erts_lc_trylock_force_busy_flg(erts_lc_lock_t *lck, erts_lock_options_t options) * TryLock order violation */ - if (lck->check_order) { - /* Check that we are not trying to lock this lock twice */ - while (1) { - if (order <= 0) { - if (order == 0) - lock_twice("Trylocking", thr, lck, options); - break; - } - ll = ll->prev; - if (!ll) - break; - order = compare_locked_by_id_extra(ll, lck); + /* Check that we are not trying to lock this lock twice */ + while (1) { + if (order <= 0) { + if (order == 0) + lock_twice("Trylocking", thr, lck, options); + break; } + ll = ll->prev; + if (!ll) + break; + order = compare_locked_by_id_extra(ll, lck); } #ifndef ERTS_LC_ALLWAYS_FORCE_BUSY_TRYLOCK_ON_LOCK_ORDER_VIOLATION @@ -1091,7 +1069,7 @@ void erts_lc_trylock_flg_x(int locked, erts_lc_lock_t *lck, erts_lock_options_t for (tl_lck = thr->locked.last; tl_lck; tl_lck = tl_lck->prev) { int order = compare_locked_by_id_extra(tl_lck, lck); if (order <= 0) { - if (order == 0 && lck->check_order && locked >= 0) + if (order == 0 && locked >= 0) lock_twice("Trylocking", thr, lck, options); if (locked) { ll->next = tl_lck->next; @@ -1214,7 +1192,7 @@ void erts_lc_lock_flg_x(erts_lc_lock_t *lck, erts_lock_options_t options, return; } order = compare_locked_by_id_extra(thr->locked.last, lck); - if (order < 0 || (!lck->check_order && order == 0)) { + if (order < 0) { lc_locked_lock_t* ll; if (LOCK_IS_TYPE_ORDER_VIOLATION(lck->flags, thr->locked.last->flags)) { type_order_violation("locking ", thr, lck); @@ -1344,7 +1322,6 @@ void erts_lc_init_lock(erts_lc_lock_t *lck, char *name, erts_lock_flags_t flags) { lck->id = erts_lc_get_lock_order_id(name); - lck->check_order = erts_lc_is_check_order(name); lck->extra = (UWord) &lck->extra; ASSERT(is_not_immed(lck->extra)); lck->flags = flags; @@ -1356,7 +1333,6 @@ void erts_lc_init_lock_x(erts_lc_lock_t *lck, char *name, erts_lock_flags_t flags, Eterm extra) { lck->id = erts_lc_get_lock_order_id(name); - lck->check_order = erts_lc_is_check_order(name); lck->extra = extra; lck->flags = flags; if (lc_is_term_order(lck->id)) { diff --git a/erts/emulator/beam/erl_lock_check.h b/erts/emulator/beam/erl_lock_check.h index 965b73e27c..b32f27d9f9 100644 --- a/erts/emulator/beam/erl_lock_check.h +++ b/erts/emulator/beam/erl_lock_check.h @@ -46,7 +46,6 @@ typedef struct { int inited; Sint16 id; - int check_order; erts_lock_flags_t flags; erts_lock_options_t taken_options; UWord extra; @@ -54,12 +53,11 @@ typedef struct { #define ERTS_LC_INITITALIZED 0x7f7f7f7f -#define ERTS_LC_LOCK_INIT(ID, X, F) {ERTS_LC_INITITALIZED, (ID), 1, (F), 0, (X)} +#define ERTS_LC_LOCK_INIT(ID, X, F) {ERTS_LC_INITITALIZED, (ID), (F), 0, (X)} void erts_lc_init(void); void erts_lc_late_init(void); Sint16 erts_lc_get_lock_order_id(char *name); -int erts_lc_is_check_order(char *name); void erts_lc_check(erts_lc_lock_t *have, int have_len, erts_lc_lock_t *have_not, int have_not_len); void erts_lc_check_exact(erts_lc_lock_t *have, int have_len); -- cgit v1.2.3 From e3592b63ed0c1d563c6b9d146f00b07fcc272c24 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 18:06:21 +0200 Subject: erts: Fix bug in erl_db_catree Search stack must be cleared before retry. --- erts/emulator/beam/erl_db_catree.c | 1 + 1 file changed, 1 insertion(+) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 77fd07277f..61667202c0 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -1086,6 +1086,7 @@ lock_first_base_node(DbTable *tbl, break; /* Retry */ runlock_base_node(base_node); + search_stack_ptr->pos = 0; } push_node_dyn_array(tbl, locked_base_nodes_stack_ptr, current_node); return current_node; -- cgit v1.2.3 From b276b28f467cadcf4255b37114c323598b551138 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 17:46:36 +0200 Subject: erts: Remove dead code in erl_db_catree 'parent' is route node and 'neighbor' is base node, they can never be equal. 'neighbor_parent' has already been set correctly for all cases. --- erts/emulator/beam/erl_db_catree.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index 61667202c0..f2baf50cbc 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -1314,11 +1314,7 @@ erl_db_catree_force_join_right(DbTableCATree *tb, &neighbor->u.base.free_item, sizeof_base_node(neighbor->u.base.lc.key_size)); - if (parent == neighbor) { - *result_parent_wb = gparent; - } else { - *result_parent_wb = neighbor_parent; - } + *result_parent_wb = neighbor_parent; return new_neighbor; } -- cgit v1.2.3 From 7bdb8e08f7f6206617a5c5b83b15e3453e5e27d2 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 3 Oct 2018 20:06:16 +0200 Subject: erts: Refactor out DbRouteKey struct to not abuse DbTerm. --- erts/emulator/beam/erl_db_catree.c | 100 ++++++++++++++++++------------------- erts/emulator/beam/erl_db_catree.h | 16 +++--- 2 files changed, 58 insertions(+), 58 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c index f2baf50cbc..a8e48bce1b 100644 --- a/erts/emulator/beam/erl_db_catree.c +++ b/erts/emulator/beam/erl_db_catree.c @@ -210,7 +210,7 @@ DbTableMethod db_catree = * Internal CA tree related helper functions and macros */ -#define GET_ROUTE_NODE_KEY(node) (node->u.route.key.tpl[0]) +#define GET_ROUTE_NODE_KEY(node) (node->u.route.key.term) #define GET_BASE_NODE_LOCK(node) (&(node->u.base.lock)) #define GET_ROUTE_NODE_LOCK(node) (&(node->u.route.lock)) @@ -782,9 +782,38 @@ void unlock_route_node(DbTableCATreeNode *route_node) } +static ERTS_INLINE +void copy_route_key(DbRouteKey* dst, Eterm key, Uint key_size) +{ + dst->size = key_size; + if (key_size != 0) { + Eterm* hp = &dst->heap[0]; + ErlOffHeap tmp_offheap; + tmp_offheap.first = NULL; + dst->term = copy_struct(key, key_size, &hp, &tmp_offheap); + dst->oh = tmp_offheap.first; + } + else { + ASSERT(is_immed(key)); + dst->term = key; + dst->oh = NULL; + } +} + +static ERTS_INLINE +void destroy_route_key(DbRouteKey* key) +{ + if (key->oh) { + ErlOffHeap oh; + oh.first = key->oh; + erts_cleanup_offheap(&oh); + } +} + + #ifdef ERTS_ENABLE_LOCK_CHECK # define sizeof_base_node(KEY_SZ) \ - (offsetof(DbTableCATreeNode, u.base.lc.key_heap) \ + (offsetof(DbTableCATreeNode, u.base.lc_key.heap) \ + (KEY_SZ)*sizeof(Eterm)) # define LC_ORDER(ORDER) ORDER #else @@ -813,16 +842,7 @@ static DbTableCATreeNode *create_base_node(DbTableCATree *tb, rwmtx_opt.main_spincount = erts_ets_rwmtx_spin_count; #ifdef ERTS_ENABLE_LOCK_CHECK - { - Eterm* hp = &p->u.base.lc.key_heap[0]; - ErlOffHeap oh; - - oh.first = NULL; - lc_key = copy_struct(lc_key, lc_key_size, &hp, &oh); - p->u.base.lc.key = lc_key; - p->u.base.lc.key_size = lc_key_size; - p->u.base.lc.key_oh = oh.first; - } + copy_route_key(&p->u.base.lc_key, lc_key, lc_key_size); #endif erts_rwmtx_init_opt(&p->u.base.lock, &rwmtx_opt, "erl_db_catree_base_node", @@ -849,7 +869,7 @@ DbTableCATreeNode *create_wlocked_base_node(DbTableCATree *tb, static ERTS_INLINE Uint sizeof_route_node(Uint key_size) { - return (offsetof(DbTableCATreeNode, u.route.key.tpl[1]) + return (offsetof(DbTableCATreeNode, u.route.key.heap) + key_size*sizeof(Eterm)); } @@ -860,25 +880,13 @@ create_route_node(DbTableCATree *tb, DbTerm * keyTerm, DbTableCATreeNode* lc_parent) { - Eterm* top; Eterm key = GETKEY(tb,keyTerm->tpl); int key_size = size_object(key); - ErlOffHeap tmp_offheap; DbTableCATreeNode* p = erts_db_alloc(ERTS_ALC_T_DB_TABLE, (DbTable *) tb, sizeof_route_node(key_size)); - if (key_size != 0) { - p->u.route.key.size = key_size; - top = &p->u.route.key.tpl[1]; - tmp_offheap.first = NULL; - p->u.route.key.tpl[0] = copy_struct(key, key_size, &top, &tmp_offheap); - p->u.route.key.first_oh = tmp_offheap.first; - } else { - p->u.route.key.size = key_size; - p->u.route.key.first_oh = NULL; - p->u.route.key.tpl[0] = key; - } + copy_route_key(&p->u.route.key, key, key_size); p->is_base_node = 0; p->u.route.is_valid = 1; erts_atomic_init_nob(&p->u.route.left, (erts_aint_t)left); @@ -906,11 +914,7 @@ static void do_free_base_node(void* vptr) ASSERT(p->is_base_node); erts_rwmtx_destroy(&p->u.base.lock); #ifdef ERTS_ENABLE_LOCK_CHECK - if (p->u.base.lc.key_size != 0) { - ErlOffHeap oh; - oh.first = p->u.base.lc.key_oh; - erts_cleanup_offheap(&oh); - } + destroy_route_key(&p->u.base.lc_key); #endif erts_free(ERTS_ALC_T_DB_TABLE, p); } @@ -918,7 +922,7 @@ static void do_free_base_node(void* vptr) static void free_catree_base_node(DbTableCATree* tb, DbTableCATreeNode* p) { ASSERT(p->is_base_node); - ERTS_DB_ALC_MEM_UPDATE_(tb, sizeof_base_node(p->u.base.lc.key_size), 0); + ERTS_DB_ALC_MEM_UPDATE_(tb, sizeof_base_node(p->u.base.lc_key.size), 0); do_free_base_node(p); } @@ -927,11 +931,7 @@ 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); - } + destroy_route_key(&p->u.route.key); erts_free(ERTS_ALC_T_DB_TABLE, p); } @@ -1038,7 +1038,7 @@ get_next_base_node_and_path(DbTableCATreeNode *base_node, stack); } else { Eterm pkey = - TOP_NODE(stack)->u.route.key.tpl[0]; /* pKey = key of parent */ + TOP_NODE(stack)->u.route.key.term; /* pKey = key of parent */ POP_NODE(stack); while (!EMPTY_NODE(stack)) { if (TOP_NODE(stack)->u.route.is_valid && @@ -1239,6 +1239,7 @@ erl_db_catree_force_join_right(DbTableCATree *tb, DbTableCATreeNode *neighbor; DbTableCATreeNode *new_neighbor; DbTableCATreeNode *neighbor_parent; + TreeDbTerm* new_root; if (parent == NULL) { return NULL; @@ -1276,12 +1277,9 @@ erl_db_catree_force_join_right(DbTableCATree *tb, unlock_route_node(gparent); } - { - TreeDbTerm* new_root = join_trees(thiz->u.base.root, - neighbor->u.base.root); - new_neighbor = create_wlocked_base_node(tb, new_root, - LC_ORDER(thiz->u.base.lc.key)); - } + new_root = join_trees(thiz->u.base.root, neighbor->u.base.root); + new_neighbor = create_wlocked_base_node(tb, new_root, + LC_ORDER(thiz->u.base.lc_key.term)); if (GET_RIGHT(parent) == neighbor) { neighbor_parent = gparent; @@ -1307,12 +1305,12 @@ erl_db_catree_force_join_right(DbTableCATree *tb, do_free_base_node, thiz, &thiz->u.base.free_item, - sizeof_base_node(thiz->u.base.lc.key_size)); + sizeof_base_node(thiz->u.base.lc_key.size)); erts_schedule_db_free(&tb->common, do_free_base_node, neighbor, &neighbor->u.base.free_item, - sizeof_base_node(neighbor->u.base.lc.key_size)); + sizeof_base_node(neighbor->u.base.lc_key.size)); *result_parent_wb = neighbor_parent; return new_neighbor; @@ -1417,7 +1415,7 @@ static void join_catree(DbTableCATree *tb, TreeDbTerm* new_root = join_trees(thiz->u.base.root, neighbor->u.base.root); new_neighbor = create_base_node(tb, new_root, - LC_ORDER(thiz->u.base.lc.key)); + LC_ORDER(thiz->u.base.lc_key.term)); } if (GET_RIGHT(parent) == neighbor) { neighbor_parent = gparent; @@ -1470,7 +1468,7 @@ static void join_catree(DbTableCATree *tb, TreeDbTerm* new_root = join_trees(neighbor->u.base.root, thiz->u.base.root); new_neighbor = create_base_node(tb, new_root, - LC_ORDER(thiz->u.base.lc.key)); + LC_ORDER(thiz->u.base.lc_key.term)); } if (GET_LEFT(parent) == neighbor) { neighbor_parent = gparent; @@ -1500,12 +1498,12 @@ static void join_catree(DbTableCATree *tb, do_free_base_node, thiz, &thiz->u.base.free_item, - sizeof_base_node(thiz->u.base.lc.key_size)); + sizeof_base_node(thiz->u.base.lc_key.size)); erts_schedule_db_free(&tb->common, do_free_base_node, neighbor, &neighbor->u.base.free_item, - sizeof_base_node(neighbor->u.base.lc.key_size)); + sizeof_base_node(neighbor->u.base.lc_key.size)); } static void split_catree(DbTableCATree *tb, @@ -1549,7 +1547,7 @@ static void split_catree(DbTableCATree *tb, do_free_base_node, base, &base->u.base.free_item, - sizeof_base_node(base->u.base.lc.key_size)); + sizeof_base_node(base->u.base.lc_key.size)); } } diff --git a/erts/emulator/beam/erl_db_catree.h b/erts/emulator/beam/erl_db_catree.h index 0ad921d880..f9c0784289 100644 --- a/erts/emulator/beam/erl_db_catree.h +++ b/erts/emulator/beam/erl_db_catree.h @@ -33,6 +33,13 @@ struct DbTableCATreeNode; +typedef struct { + Eterm term; + struct erl_off_heap_header* oh; + Uint size; + Eterm heap[1]; +} DbRouteKey; + typedef struct { erts_rwmtx_t lock; /* The lock for this base node */ Sint lock_statistics; @@ -42,12 +49,7 @@ typedef struct { struct DbTableCATreeNode * next; /* Used when gradually deleting */ #ifdef ERTS_ENABLE_LOCK_CHECK - struct { - Eterm key; - struct erl_off_heap_header* key_oh; - Uint key_size; - Eterm key_heap[1]; - } lc; + DbRouteKey lc_key; #endif char end_of_struct__; } DbTableCATreeBaseNode; @@ -61,7 +63,7 @@ typedef struct { int is_valid; /* If this route node is still valid */ erts_atomic_t left; erts_atomic_t right; - DbTerm key; + DbRouteKey key; } DbTableCATreeRouteNode; typedef struct DbTableCATreeNode { -- cgit v1.2.3