erts: Improve deallocation of CATree nodes

Update table memory stats before scheduling free
to not be dependent on deallocation order with main table struct.

5 files changed, 113 insertions, 91 deletions

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);
@@ -306,6 +312,26 @@ erts_db_free(ErtsAlcType_t type, DbTable *tab, void *ptr, Uint size)
 }
 
 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)
 {
     ASSERT(ptr != 0);
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,