funk: remove root-level record linked list

Funk maintained a LRU-like linked list weaving through all records
in the root txn (fd_funk_rec_t).  This linked list list served no
purpose other than to iterate through records at the root txn.

However, it created painful DRAM traffic for record-level
operations, particularly when rooting.  It also inhibited some
optimizations for doing fast parallel account loading into funk.

Removing this linked list at the root-level simplifies code,
provides an immediate speedup, and possibilities for future
optimization.

Author: riptl

src/funk/fd_funk.c

@@ -119,8 +119,6 @@ fd_funk_new( void * shmem,
   fd_funk_rec_pool_reset( rec_join, 0UL );
   funk->rec_ele_gaddr = fd_wksp_gaddr_fast( wksp, rec_ele );
   funk->rec_max = (uint)rec_max;
-  funk->rec_head_idx  = FD_FUNK_REC_IDX_NULL;
-  funk->rec_tail_idx  = FD_FUNK_REC_IDX_NULL;
 
   funk->alloc_gaddr = fd_wksp_gaddr_fast( wksp, fd_alloc_join( fd_alloc_new( alloc, wksp_tag ), 0UL ) );
 
@@ -402,23 +400,6 @@ fd_funk_verify( fd_funk_t * join ) {
   TEST( rec_chain_cnt==fd_funk_rec_map_chain_cnt( rec_map ) );
   TEST( seed==fd_funk_rec_map_seed( rec_map ) );
 
-  uint rec_head_idx = funk->rec_head_idx;
-  uint rec_tail_idx = funk->rec_tail_idx;
-
-  int null_rec_head = fd_funk_rec_idx_is_null( rec_head_idx );
-  int null_rec_tail = fd_funk_rec_idx_is_null( rec_tail_idx );
-
-  if( !rec_max ) TEST( null_rec_head & null_rec_tail );
-  else {
-    if( null_rec_head ) TEST( null_rec_tail );
-    else                TEST( rec_head_idx<rec_max );
-
-    if( null_rec_tail ) TEST( null_rec_head );
-    else                TEST( rec_tail_idx<rec_max );
-  }
-
-  if( !rec_max ) TEST( fd_funk_rec_idx_is_null( rec_tail_idx ) );
-
   TEST( !fd_funk_rec_verify( join ) );
 
   /* Test values */

src/funk/fd_funk.h

@@ -239,8 +239,6 @@ struct __attribute__((aligned(FD_FUNK_ALIGN))) fd_funk_shmem_private {
                           rec_max==fd_funk_rec_map_key_max(rec_map) */
   ulong rec_pool_gaddr;
   ulong rec_ele_gaddr;
-  uint rec_head_idx;  /* Record map index of the first record, FD_FUNK_REC_IDX_NULL if none (from oldest to youngest) */
-  uint rec_tail_idx;  /* "                       last          " */
 
   /* The funk alloc is used for allocating wksp resources for record
      values.  This is a fd_alloc and more details are given in

src/funk/fd_funk_rec.c

@@ -1,4 +1,5 @@
 #include "fd_funk.h"
+#include "fd_funk_txn.h"
 
 /* Provide the actual record map implementation */
 
@@ -222,6 +223,7 @@ fd_funk_rec_prepare( fd_funk_t *               funk,
     return NULL;
   }
 #endif
+  memset( prepare, 0, sizeof(fd_funk_rec_prepare_t) );
 
   if( !txn ) { /* Modifying last published */
     if( FD_UNLIKELY( fd_funk_last_publish_is_frozen( funk ) ) ) {
@@ -239,30 +241,28 @@ fd_funk_rec_prepare( fd_funk_t *               funk,
   if( opt_err && *opt_err == FD_POOL_ERR_CORRUPT ) {
     FD_LOG_ERR(( "corrupt element returned from funk rec pool" ));
   }
-
-  if( rec != NULL ) {
-    fd_funk_val_init( rec );
-    if( txn == NULL ) {
-      fd_funk_txn_xid_set_root( rec->pair.xid );
-      rec->txn_cidx = fd_funk_txn_cidx( FD_FUNK_TXN_IDX_NULL );
-      prepare->rec_head_idx = &funk->shmem->rec_head_idx;
-      prepare->rec_tail_idx = &funk->shmem->rec_tail_idx;
-      prepare->txn_lock     = &funk->shmem->lock;
-    } else {
-      fd_funk_txn_xid_copy( rec->pair.xid, &txn->xid );
-      rec->txn_cidx = fd_funk_txn_cidx( (ulong)( txn - funk->txn_pool->ele ) );
-      prepare->rec_head_idx = &txn->rec_head_idx;
-      prepare->rec_tail_idx = &txn->rec_tail_idx;
-      prepare->txn_lock     = &txn->lock;
-    }
-    fd_funk_rec_key_copy( rec->pair.key, key );
-    rec->tag = 0;
-    rec->flags = 0;
-    rec->prev_idx = FD_FUNK_REC_IDX_NULL;
-    rec->next_idx = FD_FUNK_REC_IDX_NULL;
-  } else {
+  if( FD_UNLIKELY( !rec ) ) {
     fd_int_store_if( !!opt_err, opt_err, FD_FUNK_ERR_REC );
+    return rec;
   }
+
+  fd_funk_val_init( rec );
+  if( txn == NULL ) {
+    fd_funk_txn_xid_set_root( rec->pair.xid );
+    rec->txn_cidx = fd_funk_txn_cidx( FD_FUNK_TXN_IDX_NULL );
+    prepare->txn_lock     = &funk->shmem->lock;
+  } else {
+    fd_funk_txn_xid_copy( rec->pair.xid, &txn->xid );
+    rec->txn_cidx = fd_funk_txn_cidx( (ulong)( txn - funk->txn_pool->ele ) );
+    prepare->rec_head_idx = &txn->rec_head_idx;
+    prepare->rec_tail_idx = &txn->rec_tail_idx;
+    prepare->txn_lock     = &txn->lock;
+  }
+  fd_funk_rec_key_copy( rec->pair.key, key );
+  rec->tag = 0;
+  rec->flags = 0;
+  rec->prev_idx = FD_FUNK_REC_IDX_NULL;
+  rec->next_idx = FD_FUNK_REC_IDX_NULL;
   return rec;
 }
 
@@ -276,16 +276,17 @@ fd_funk_rec_publish( fd_funk_t *             funk,
   /* Lock the txn */
   while( FD_ATOMIC_CAS( prepare->txn_lock, 0, 1 ) ) FD_SPIN_PAUSE();
 
-  uint rec_prev_idx;
-  uint rec_idx = (uint)( rec - funk->rec_pool->ele );
-  rec_prev_idx = *rec_tail_idx;
-  *rec_tail_idx = rec_idx;
-  rec->prev_idx = rec_prev_idx;
-  rec->next_idx = FD_FUNK_REC_IDX_NULL;
-  if( fd_funk_rec_idx_is_null( rec_prev_idx ) ) {
-    *rec_head_idx = rec_idx;
-  } else {
-    funk->rec_pool->ele[ rec_prev_idx ].next_idx = rec_idx;
+  if( rec_head_idx ) {
+    uint rec_idx      = (uint)( rec - funk->rec_pool->ele );
+    uint rec_prev_idx = *rec_tail_idx;
+    *rec_tail_idx = rec_idx;
+    rec->prev_idx = rec_prev_idx;
+    rec->next_idx = FD_FUNK_REC_IDX_NULL;
+    if( fd_funk_rec_idx_is_null( rec_prev_idx ) ) {
+      *rec_head_idx = rec_idx;
+    } else {
+      funk->rec_pool->ele[ rec_prev_idx ].next_idx = rec_idx;
+    }
   }
 
   if( fd_funk_rec_map_insert( funk->rec_map, rec, FD_MAP_FLAG_BLOCKING ) ) {
@@ -601,7 +602,6 @@ fd_funk_rec_purify( fd_funk_t * funk ) {
   fd_funk_rec_map_reset( rec_map );
   rec_pool->pool->ver_top = fd_funk_rec_pool_idx_null();;
 
-  uint prev_idx = FD_FUNK_REC_IDX_NULL;
   for( ulong i = 0; i < rec_max; ++i ) {
     fd_funk_rec_t * rec = rec_pool->ele + i;
     if( fd_funk_rec_pool_is_in_pool( rec ) ||
@@ -619,21 +619,6 @@ fd_funk_rec_purify( fd_funk_t * funk ) {
     rec_used_cnt++;
 
     fd_funk_rec_map_insert( rec_map, rec, FD_MAP_FLAG_BLOCKING );
-
-    rec->prev_idx = prev_idx;
-    if( prev_idx != FD_FUNK_REC_IDX_NULL ) {
-      (rec_pool->ele + prev_idx)->next_idx = fd_funk_rec_map_private_cidx( i );
-    } else {
-      funk->shmem->rec_head_idx = fd_funk_rec_map_private_cidx( i );
-    }
-    prev_idx = fd_funk_rec_map_private_cidx( i );
-  }
-
-  funk->shmem->rec_tail_idx = prev_idx;
-  if( prev_idx != FD_FUNK_REC_IDX_NULL ) {
-    (rec_pool->ele + prev_idx)->next_idx = FD_FUNK_REC_IDX_NULL;
-  } else {
-    funk->shmem->rec_head_idx = FD_FUNK_REC_IDX_NULL;
   }
 
   FD_LOG_NOTICE(( "funk records used after purify: %u, free: %u", rec_used_cnt, rec_free_cnt ));
@@ -675,6 +660,9 @@ fd_funk_rec_verify( fd_funk_t * funk ) {
       if( fd_funk_txn_idx_is_null( txn_idx ) ) { /* This is a record from the last published transaction */
 
         TEST( fd_funk_txn_xid_eq_root( txn_xid ) );
+        /* No record linked list at the root txn */
+        TEST( fd_funk_rec_idx_is_null( rec->prev_idx ) );
+        TEST( fd_funk_rec_idx_is_null( rec->next_idx ) );
 
       } else { /* This is a record from an in-prep transaction */
 
@@ -687,26 +675,20 @@ fd_funk_rec_verify( fd_funk_t * funk ) {
     }
   }
 
-  /* Clear record tags and then verify the forward and reverse linkage */
+  /* Clear record tags and then verify membership */
 
   for( ulong rec_idx=0UL; rec_idx<rec_max; rec_idx++ ) rec_pool->ele[ rec_idx ].tag = 0U;
 
   do {
-    ulong txn_idx = FD_FUNK_TXN_IDX_NULL;
-    uint  rec_idx = funk->shmem->rec_head_idx;
-    while( !fd_funk_rec_idx_is_null( rec_idx ) ) {
-      TEST( (rec_idx<rec_max) && (fd_funk_txn_idx( rec_pool->ele[ rec_idx ].txn_cidx )==txn_idx) && rec_pool->ele[ rec_idx ].tag==0U );
-      rec_pool->ele[ rec_idx ].tag = 1U;
-      fd_funk_rec_query_t query[1];
-      fd_funk_rec_t const * rec2 = fd_funk_rec_query_try_global( funk, NULL, rec_pool->ele[ rec_idx ].pair.key, NULL, query );
-      if( FD_UNLIKELY( rec_pool->ele[ rec_idx ].flags & FD_FUNK_REC_FLAG_ERASE ) )
-        TEST( rec2 == NULL );
-      else
-        TEST( rec2 == rec_pool->ele + rec_idx );
-      uint next_idx = rec_pool->ele[ rec_idx ].next_idx;
-      if( !fd_funk_rec_idx_is_null( next_idx ) ) TEST( rec_pool->ele[ next_idx ].prev_idx==rec_idx );
-      rec_idx = next_idx;
+    fd_funk_all_iter_t iter[1];
+    for( fd_funk_all_iter_new( funk, iter ); !fd_funk_all_iter_done( iter ); fd_funk_all_iter_next( iter ) ) {
+      fd_funk_rec_t * rec = fd_funk_all_iter_ele( iter );
+      if( fd_funk_txn_xid_eq_root( rec->pair.xid ) ) {
+        TEST( rec->tag==0U );
+        rec->tag = 1U;
+      }
     }
+
     fd_funk_txn_all_iter_t txn_iter[1];
     for( fd_funk_txn_all_iter_new( funk, txn_iter ); !fd_funk_txn_all_iter_done( txn_iter ); fd_funk_txn_all_iter_next( txn_iter ) ) {
       fd_funk_txn_t const * txn = fd_funk_txn_all_iter_ele_const( txn_iter );
@@ -735,16 +717,6 @@ fd_funk_rec_verify( fd_funk_t * funk ) {
   }
 
   do {
-    ulong txn_idx = FD_FUNK_TXN_IDX_NULL;
-    uint  rec_idx = funk->shmem->rec_tail_idx;
-    while( !fd_funk_rec_idx_is_null( rec_idx ) ) {
-      TEST( (rec_idx<rec_max) && (fd_funk_txn_idx( rec_pool->ele[ rec_idx ].txn_cidx )==txn_idx) && rec_pool->ele[ rec_idx ].tag==1U );
-      rec_pool->ele[ rec_idx ].tag = 2U;
-      uint prev_idx = rec_pool->ele[ rec_idx ].prev_idx;
-      if( !fd_funk_rec_idx_is_null( prev_idx ) ) TEST( rec_pool->ele[ prev_idx ].next_idx==rec_idx );
-      rec_idx = prev_idx;
-    }
-
     fd_funk_txn_all_iter_t txn_iter[1];
     for( fd_funk_txn_all_iter_new( funk, txn_iter ); !fd_funk_txn_all_iter_done( txn_iter ); fd_funk_txn_all_iter_next( txn_iter ) ) {
       fd_funk_txn_t const * txn = fd_funk_txn_all_iter_ele_const( txn_iter );

src/funk/fd_funk_txn.c

@@ -478,23 +478,23 @@ fd_funk_txn_cancel_all( fd_funk_t *     funk,
    changing the order of existing values. */
 
 static void
-fd_funk_txn_update( fd_funk_t *                  funk,
-                       uint *                   _dst_rec_head_idx, /* Pointer to the dst list head */
-                       uint *                   _dst_rec_tail_idx, /* Pointer to the dst list tail */
-                       ulong                     dst_txn_idx,       /* Transaction index of the merge destination */
-                       fd_funk_txn_xid_t const * dst_xid,        /* dst xid */
-                       ulong                     txn_idx ) {        /* Transaction index of the records to merge */
+fd_funk_txn_update( fd_funk_t *               funk,
+                    uint *                    _dst_rec_head_idx, /* Pointer to the dst list head */
+                    uint *                    _dst_rec_tail_idx, /* Pointer to the dst list tail */
+                    ulong                     dst_txn_idx,       /* Transaction index of the merge destination */
+                    fd_funk_txn_xid_t const * dst_xid,           /* dst xid */
+                    ulong                     txn_idx ) {        /* Transaction index of the records to merge */
   fd_wksp_t *          wksp     = funk->wksp;
   fd_alloc_t *         alloc    = funk->alloc;
   fd_funk_rec_map_t *  rec_map  = funk->rec_map;
   fd_funk_rec_pool_t * rec_pool = funk->rec_pool;
   fd_funk_txn_pool_t * txn_pool = funk->txn_pool;
 
-  int critical = (_dst_rec_head_idx == &funk->shmem->rec_head_idx);
-  if (critical) {
+  int const critical = _dst_rec_head_idx==NULL;
+  if( critical ) {
     /* If the root transaction is being updated, we need to mark
        the beginning of a critical section becuase fd_funk_purify can't fix it */
-    fd_begin_crit(funk);
+    fd_begin_crit( funk );
   }
 
   fd_funk_txn_t * txn = &txn_pool->ele[ txn_idx ];
@@ -518,12 +518,12 @@ fd_funk_txn_update( fd_funk_t *                  funk,
       uint prev_idx = rec2->prev_idx;
       uint next_idx = rec2->next_idx;
       if( fd_funk_rec_idx_is_null( prev_idx ) ) {
-        *_dst_rec_head_idx = next_idx;
+        if( _dst_rec_head_idx ) *_dst_rec_head_idx = next_idx;
       } else {
         rec_pool->ele[ prev_idx ].next_idx = next_idx;
       }
       if( fd_funk_rec_idx_is_null( next_idx ) ) {
-        *_dst_rec_tail_idx = prev_idx;
+        if( _dst_rec_tail_idx ) *_dst_rec_tail_idx = prev_idx;
       } else {
         rec_pool->ele[ next_idx ].prev_idx = prev_idx;
       }
@@ -544,14 +544,16 @@ fd_funk_txn_update( fd_funk_t *                  funk,
     rec->pair.xid[0] = *dst_xid;
     rec->txn_cidx = fd_funk_txn_cidx( dst_txn_idx );
 
-    if( fd_funk_rec_idx_is_null( *_dst_rec_head_idx ) ) {
-      *_dst_rec_head_idx = rec_idx;
-      rec->prev_idx = FD_FUNK_REC_IDX_NULL;
-    } else {
-      rec_pool->ele[ *_dst_rec_tail_idx ].next_idx = rec_idx;
-      rec->prev_idx = *_dst_rec_tail_idx;
+    rec->prev_idx = FD_FUNK_REC_IDX_NULL;
+    if( _dst_rec_head_idx ) {
+      if( fd_funk_rec_idx_is_null( *_dst_rec_head_idx ) ) {
+        *_dst_rec_head_idx = rec_idx;
+      } else {
+        rec_pool->ele[ *_dst_rec_tail_idx ].next_idx = rec_idx;
+        rec->prev_idx = *_dst_rec_tail_idx;
+      }
+      *_dst_rec_tail_idx = rec_idx;
     }
-    *_dst_rec_tail_idx = rec_idx;
     rec->next_idx = FD_FUNK_REC_IDX_NULL;
 
     rec_idx = next_rec_idx;
@@ -560,8 +562,8 @@ fd_funk_txn_update( fd_funk_t *                  funk,
   txn_pool->ele[ txn_idx ].rec_head_idx = FD_FUNK_REC_IDX_NULL;
   txn_pool->ele[ txn_idx ].rec_tail_idx = FD_FUNK_REC_IDX_NULL;
 
-  if (critical) {
-    fd_end_crit(funk);
+  if( critical ) {
+    fd_end_crit( funk );
   }
 }
 
@@ -578,7 +580,7 @@ fd_funk_txn_publish_funk_child( fd_funk_t * const funk,
 
   /* Apply the updates in txn to the last published transactions */
 
-  fd_funk_txn_update( funk, &funk->shmem->rec_head_idx, &funk->shmem->rec_tail_idx, FD_FUNK_TXN_IDX_NULL, fd_funk_root( funk ), txn_idx );
+  fd_funk_txn_update( funk, NULL, NULL, FD_FUNK_TXN_IDX_NULL, fd_funk_root( funk ), txn_idx );
 
   /* Cancel all competing transaction histories */
 
@@ -706,7 +708,7 @@ fd_funk_txn_publish_into_parent( fd_funk_t *     funk,
   ulong parent_idx = fd_funk_txn_idx( txn->parent_cidx );
   if( fd_funk_txn_idx_is_null( parent_idx ) ) {
     /* Publish to root */
-    fd_funk_txn_update( funk, &funk->shmem->rec_head_idx, &funk->shmem->rec_tail_idx, FD_FUNK_TXN_IDX_NULL, fd_funk_root( funk ), txn_idx );
+    fd_funk_txn_update( funk, NULL, NULL, FD_FUNK_TXN_IDX_NULL, fd_funk_root( funk ), txn_idx );
     /* Inherit the children */
     funk->shmem->child_head_cidx = txn->child_head_cidx;
     funk->shmem->child_tail_cidx = txn->child_tail_cidx;
@@ -733,29 +735,20 @@ fd_funk_txn_publish_into_parent( fd_funk_t *     funk,
   return FD_FUNK_SUCCESS;
 }
 
-/* Return the first record in a transaction. Returns NULL if the
-   transaction has no records yet. */
-
 fd_funk_rec_t const *
 fd_funk_txn_first_rec( fd_funk_t *           funk,
                        fd_funk_txn_t const * txn ) {
-  uint rec_idx;
-  if( FD_UNLIKELY( NULL == txn ))
-    rec_idx = funk->shmem->rec_head_idx;
-  else
-    rec_idx = txn->rec_head_idx;
+  if( FD_UNLIKELY( !txn ) ) return NULL;
+  uint rec_idx = txn->rec_head_idx;
   if( fd_funk_rec_idx_is_null( rec_idx ) ) return NULL;
   return funk->rec_pool->ele + rec_idx;
 }
 
 fd_funk_rec_t const *
 fd_funk_txn_last_rec( fd_funk_t *           funk,
                       fd_funk_txn_t const * txn ) {
-  uint rec_idx;
-  if( FD_UNLIKELY( NULL == txn ))
-    rec_idx = funk->shmem->rec_tail_idx;
-  else
-    rec_idx = txn->rec_tail_idx;
+  if( FD_UNLIKELY( !txn ) ) return NULL;
+  uint rec_idx = txn->rec_tail_idx;
   if( fd_funk_rec_idx_is_null( rec_idx ) ) return NULL;
   return funk->rec_pool->ele + rec_idx;
 }

src/funk/fd_funk_txn.h

@@ -224,16 +224,14 @@ fd_funk_txn_is_only_child( fd_funk_txn_t const * txn ) {
 
 typedef struct fd_funk_rec fd_funk_rec_t;
 
-/* Return the first (oldest) record in a transaction. Returns NULL if the
-   transaction has no records yet. */
+/* fd_funk_txn_{first,last}_rec return the {first,last} record in a
+   transaction.  Returns NULL if the transaction has no records yet, or
+   if txn is NULL (root txn). */
 
 fd_funk_rec_t const *
 fd_funk_txn_first_rec( fd_funk_t *           funk,
                        fd_funk_txn_t const * txn );
 
-/* Return the last (newest) record in a transaction. Returns NULL if the
-   transaction has no records yet. */
-
 fd_funk_rec_t const *
 fd_funk_txn_last_rec( fd_funk_t *           funk,
                       fd_funk_txn_t const * txn );