fix

Author: topointon-jump

src/flamenco/runtime/fd_hashes.c

@@ -859,62 +859,43 @@ typedef struct accounts_hash accounts_hash_t;
 static ulong
 fd_accounts_sorted_subrange_count( fd_funkier_t * funk,
                                    uint           range_idx,
-                                   uint           range_cnt,
-                                   fd_spad_t *    runtime_spad ) {
+                                   uint           range_cnt ) {
 
   fd_wksp_t *           wksp      = fd_funkier_wksp( funk );
   ulong                 num_pairs = 0UL;
   ulong                 range_len = ULONG_MAX/range_cnt;
   ulong                 range_min = range_len*range_idx;
   ulong                 range_max = (range_idx+1U<range_cnt) ? (range_min+range_len-1U) : ULONG_MAX;
 
-  FD_SPAD_FRAME_BEGIN( runtime_spad ) {
+  fd_funkier_all_iter_t iter[1];
+  for( fd_funkier_all_iter_new( funk, iter );
+       !fd_funkier_all_iter_done( iter );
+       fd_funkier_all_iter_next( iter ) ) {
+    fd_funkier_rec_t const * rec = fd_funkier_all_iter_ele_const( iter );
 
-    /* Thread-safe iteration as described in the documentation of fd_map_para.c */
-    fd_funkier_rec_map_t rec_map = fd_funkier_rec_map( funk, wksp );
-    ulong lock_cnt               = fd_funkier_rec_map_chain_cnt( &rec_map );
-    ulong * lock_seq             = fd_spad_alloc( runtime_spad, alignof(ulong), sizeof(ulong) );
-    for( ulong lock_idx=0UL; lock_idx<lock_cnt; lock_idx++ ) lock_seq[ lock_idx ] = lock_idx;
-
-    fd_funkier_rec_map_iter_lock( &rec_map, lock_seq, lock_cnt, FD_MAP_FLAG_BLOCKING );
-
-    for( ulong lock_idx=0UL; lock_idx<lock_cnt; lock_idx++ ) {
-      /* Process chains in the order they were locked */
-      ulong chain_idx = lock_seq[ lock_idx ];
-
-      for( fd_funkier_rec_map_iter_t iter = fd_funkier_rec_map_iter( &rec_map, chain_idx );
-        !fd_funkier_rec_map_iter_done( iter );
-        iter = fd_funkier_rec_map_iter_next( iter ) ) {
-        fd_funkier_rec_t const * rec = fd_funkier_rec_map_iter_ele_const( iter );
-
-        if ( !fd_funkier_key_is_acc( rec->pair.key ) ||         /* not a solana record */
-            (rec->flags & FD_FUNKIER_REC_FLAG_ERASE) ||        /* this is a tombstone */
-            (rec->pair.xid->ul[0] | rec->pair.xid->ul[1]) != 0 /* not root xid */ ) {
-          continue;
-        }
-
-        ulong n = __builtin_bswap64( rec->pair.key->ul[0] );
-        if( n<range_min || n>range_max ) {
-          continue;
-        }
-
-        fd_account_meta_t * metadata = (fd_account_meta_t *)fd_funkier_val_const( rec, wksp );
-        int is_empty = (metadata->info.lamports == 0);
-        if( is_empty ) {
-          continue;
-        }
+    if ( !fd_funkier_key_is_acc( rec->pair.key ) ||         /* not a solana record */
+        (rec->flags & FD_FUNKIER_REC_FLAG_ERASE) ||        /* this is a tombstone */
+        (rec->pair.xid->ul[0] | rec->pair.xid->ul[1]) != 0 /* not root xid */ ) {
+      continue;
+    }
 
-        if( (metadata->info.executable & ~1) != 0 ) {
-          continue;
-        }
+    ulong n = __builtin_bswap64( rec->pair.key->ul[0] );
+    if( n<range_min || n>range_max ) {
+      continue;
+    }
 
-        num_pairs++;
-      }
+    fd_account_meta_t * metadata = (fd_account_meta_t *)fd_funkier_val_const( rec, wksp );
+    int is_empty = (metadata->info.lamports == 0);
+    if( is_empty ) {
+      continue;
+    }
 
-      /* Unlock incrementally */
-      fd_funkier_rec_map_iter_unlock( &rec_map, lock_seq + lock_idx, 1UL );
+    if( (metadata->info.executable & ~1) != 0 ) {
+      continue;
     }
-  } FD_SPAD_FRAME_END;
+
+    num_pairs++;
+  }
 
   return num_pairs;
 }
@@ -927,8 +908,7 @@ fd_accounts_sorted_subrange_gather( fd_funkier_t *             funk,
                                     fd_lthash_value_t *     lthash_values_out,
                                     ulong                   n0,
                                     fd_pubkey_hash_pair_t * pairs,
-                                    fd_features_t          *features,
-                                    fd_spad_t *             runtime_spad ) {
+                                    fd_features_t          *features ) {
 
   fd_wksp_t *     wksp              = fd_funkier_wksp( funk );
   ulong           num_pairs         = 0UL;
@@ -938,69 +918,51 @@ fd_accounts_sorted_subrange_gather( fd_funkier_t *             funk,
 
   fd_lthash_value_t accum = {0};
 
-  FD_SPAD_FRAME_BEGIN( runtime_spad ) {
-
-    /* Thread-safe iteration as described in the documentation of fd_map_para.c */
-    fd_funkier_rec_map_t rec_map = fd_funkier_rec_map( funk, wksp );
-    ulong lock_cnt               = fd_funkier_rec_map_chain_cnt( &rec_map );
-    ulong * lock_seq             = fd_spad_alloc( runtime_spad, alignof(ulong), sizeof(ulong) );
-    for( ulong lock_idx=0UL; lock_idx<lock_cnt; lock_idx++ ) lock_seq[ lock_idx ] = lock_idx;
-
-    fd_funkier_rec_map_iter_lock( &rec_map, lock_seq, lock_cnt, FD_MAP_FLAG_BLOCKING );
-
-    for( ulong lock_idx=0UL; lock_idx<lock_cnt; lock_idx++ ) {
-      /* Process chains in the order they were locked */
-      ulong chain_idx = lock_seq[ lock_idx ];
-
-      for( fd_funkier_rec_map_iter_t iter = fd_funkier_rec_map_iter( &rec_map, chain_idx );
-        !fd_funkier_rec_map_iter_done( iter );
-        iter = fd_funkier_rec_map_iter_next( iter ) ) {
-        fd_funkier_rec_t const * rec = fd_funkier_rec_map_iter_ele_const( iter );
-        if ( !fd_funkier_key_is_acc( rec->pair.key ) ||         /* not a solana record */
-            (rec->flags & FD_FUNKIER_REC_FLAG_ERASE) ||        /* this is a tombstone */
-            (rec->pair.xid->ul[0] | rec->pair.xid->ul[1]) != 0 /* not root xid */ ) {
-          continue;
-        }
-
-        ulong n = __builtin_bswap64( rec->pair.key->ul[0] );
-        if( n<range_min || n>range_max ) {
-          continue;
-        }
-        fd_account_meta_t * metadata = (fd_account_meta_t *)fd_funkier_val_const( rec, wksp );
-        int is_empty = (metadata->info.lamports == 0);
-        if( is_empty ) {
-          continue;
-        }
-
-        /* FIXME: remove magic number */
-        uchar hash[32];
-        fd_lthash_value_t new_lthash_value = {0};
+  fd_funkier_all_iter_t iter[1];
+  for( fd_funkier_all_iter_new( funk, iter );
+       !fd_funkier_all_iter_done( iter );
+       fd_funkier_all_iter_next( iter ) ) {
+    fd_funkier_rec_t const * rec = fd_funkier_all_iter_ele_const( iter );
+    if ( !fd_funkier_key_is_acc( rec->pair.key ) ||         /* not a solana record */
+        (rec->flags & FD_FUNKIER_REC_FLAG_ERASE) ||        /* this is a tombstone */
+        (rec->pair.xid->ul[0] | rec->pair.xid->ul[1]) != 0 /* not root xid */ ) {
+      continue;
+    }
 
-        fd_hash_account_current( (uchar *)hash, &new_lthash_value, metadata, rec->pair.key->uc, fd_account_meta_get_data( metadata ), FD_HASH_BOTH_HASHES, features  );
-        fd_lthash_add( &accum, &new_lthash_value );
+    ulong n = __builtin_bswap64( rec->pair.key->ul[0] );
+    if( n<range_min || n>range_max ) {
+      continue;
+    }
+    fd_account_meta_t * metadata = (fd_account_meta_t *)fd_funkier_val_const( rec, wksp );
+    int is_empty = (metadata->info.lamports == 0);
+    if( is_empty ) {
+      continue;
+    }
 
-        fd_hash_t * h = (fd_hash_t *)metadata->hash;
-        if( FD_LIKELY( (h->ul[0] | h->ul[1] | h->ul[2] | h->ul[3]) != 0 ) ) {
-          if( FD_UNLIKELY( fd_acc_exists( metadata ) && memcmp( metadata->hash, &hash, 32 ) != 0 ) ) {
-            FD_LOG_WARNING(( "snapshot hash (%s) doesn't match calculated hash (%s)", FD_BASE58_ENC_32_ALLOCA( metadata->hash ), FD_BASE58_ENC_32_ALLOCA( &hash ) ));
-          }
-        } else {
-          fd_memcpy( metadata->hash, &hash, sizeof(fd_hash_t) );
-        }
+    /* FIXME: remove magic number */
+    uchar hash[32];
+    fd_lthash_value_t new_lthash_value = {0};
 
-        if( (metadata->info.executable & ~1) != 0 ) {
-          continue;
-        }
+    fd_hash_account_current( (uchar *)hash, &new_lthash_value, metadata, rec->pair.key->uc, fd_account_meta_get_data( metadata ), FD_HASH_BOTH_HASHES, features  );
+    fd_lthash_add( &accum, &new_lthash_value );
 
-        fd_pubkey_hash_pair_t * pair = &pairs[num_pairs++];
-        pair->rec                    = rec;
-        pair->hash                   = (const fd_hash_t *)metadata->hash;
+    fd_hash_t * h = (fd_hash_t *)metadata->hash;
+    if( FD_LIKELY( (h->ul[0] | h->ul[1] | h->ul[2] | h->ul[3]) != 0 ) ) {
+      if( FD_UNLIKELY( fd_acc_exists( metadata ) && memcmp( metadata->hash, &hash, 32 ) != 0 ) ) {
+        FD_LOG_WARNING(( "snapshot hash (%s) doesn't match calculated hash (%s)", FD_BASE58_ENC_32_ALLOCA( metadata->hash ), FD_BASE58_ENC_32_ALLOCA( &hash ) ));
       }
+    } else {
+      fd_memcpy( metadata->hash, &hash, sizeof(fd_hash_t) );
+    }
 
-      /* Unlock incrementally */
-      fd_funkier_rec_map_iter_unlock( &rec_map, lock_seq + lock_idx, 1UL );
+    if( (metadata->info.executable & ~1) != 0 ) {
+      continue;
     }
-  } FD_SPAD_FRAME_END;
+
+    fd_pubkey_hash_pair_t * pair = &pairs[num_pairs++];
+    pair->rec                    = rec;
+    pair->hash                   = (const fd_hash_t *)metadata->hash;
+  }
 
   sort_pubkey_hash_pair_inplace( pairs, num_pairs );
 
@@ -1015,7 +977,6 @@ struct fd_subrange_task_info {
   ulong                        num_lists;
   fd_pubkey_hash_pair_list_t * lists;
   fd_lthash_value_t *          lthash_values;
-  fd_spad_t *                  runtime_spad;
 };
 typedef struct fd_subrange_task_info fd_subrange_task_info_t;
 
@@ -1028,7 +989,7 @@ fd_accounts_sorted_subrange_count_task( void *tpool,
                                         ulong m0, ulong m1 FD_PARAM_UNUSED,
                                         ulong n0 FD_PARAM_UNUSED, ulong n1 FD_PARAM_UNUSED) {
   fd_subrange_task_info_t * task_info = (fd_subrange_task_info_t *)tpool;
-  ulong num_pairs = fd_accounts_sorted_subrange_count( task_info->funk, (uint)m0, (uint)task_info->num_lists, task_info->runtime_spad );
+  ulong num_pairs = fd_accounts_sorted_subrange_count( task_info->funk, (uint)m0, (uint)task_info->num_lists );
   task_info->lists[m0].pairs_len = num_pairs;
 }
 
@@ -1043,7 +1004,7 @@ fd_accounts_sorted_subrange_gather_task( void *tpool,
   fd_subrange_task_info_t *    task_info = (fd_subrange_task_info_t *)tpool;
   fd_pubkey_hash_pair_list_t * list      = task_info->lists + m0;
   fd_accounts_sorted_subrange_gather( task_info->funk, (uint)m0, (uint)task_info->num_lists,
-                                      &list->pairs_len, task_info->lthash_values, n0, list->pairs, task_info->features, task_info->runtime_spad );
+                                      &list->pairs_len, task_info->lthash_values, n0, list->pairs, task_info->features );
 }
 
 int
@@ -1057,6 +1018,14 @@ fd_accounts_hash( fd_funkier_t *      funk,
 
   FD_LOG_NOTICE(("accounts_hash start"));
 
+  /* FIXME: this is not the correct lock to use, although in reality this is fine as we never modify
+     accounts at the same time as hashing them. Once the hashing has been moved into tiles, we need to
+     change it so that we partition by hash chains, and acquire the lock on the individual hash chains. */
+  fd_wksp_t * wksp = fd_funkier_wksp( funk );
+  fd_funkier_rec_pool_t rec_pool = fd_funkier_rec_pool( funk, wksp );
+  fd_funkier_rec_pool_lock( &rec_pool, 1 );
+  fd_funkier_txn_start_read( funk );
+
   if( tpool == NULL || fd_tpool_worker_cnt( tpool ) <= 1U ) {
     ulong               num_pairs     = 0UL;
     fd_lthash_value_t * lthash_values = fd_spad_alloc( runtime_spad, FD_LTHASH_VALUE_ALIGN, FD_LTHASH_VALUE_FOOTPRINT );
@@ -1066,7 +1035,7 @@ fd_accounts_hash( fd_funkier_t *      funk,
                                                                FD_PUBKEY_HASH_PAIR_ALIGN,
                                                                funk->rec_max * sizeof(fd_pubkey_hash_pair_t) );
 
-    fd_accounts_sorted_subrange_gather( funk, 0, 1, &num_pairs, lthash_values, 0, pairs, features, runtime_spad );
+    fd_accounts_sorted_subrange_gather( funk, 0, 1, &num_pairs, lthash_values, 0, pairs, features );
     if( FD_UNLIKELY( !pairs ) ) {
       FD_LOG_ERR(( "failed to allocate memory for account hash" ));
     }
@@ -1097,7 +1066,6 @@ fd_accounts_hash( fd_funkier_t *      funk,
         .num_lists     = num_lists,
         .lists         = lists,
         .lthash_values = lthash_values,
-        .runtime_spad  = runtime_spad,
       };
 
       fd_tpool_exec_all_rrobin( tpool, 0UL, num_lists, fd_accounts_sorted_subrange_count_task, &task_info,
@@ -1126,6 +1094,9 @@ fd_accounts_hash( fd_funkier_t *      funk,
   } else
     FD_LOG_NOTICE(( "accounts_hash %s", FD_BASE58_ENC_32_ALLOCA( accounts_hash->hash ) ));
 
+  fd_funkier_rec_pool_unlock( &rec_pool );
+  fd_funkier_txn_end_read( funk );
+
   return 0;
 }
 

src/funkier/fd_funkier.c

@@ -207,7 +207,7 @@ fd_funkier_delete( void * shfunk ) {
   /* Thread-safe iteration as described in the documentation of fd_map_para.c */
   fd_funkier_rec_map_t rec_map = fd_funkier_rec_map( funk, wksp );
   ulong lock_cnt               = fd_funkier_rec_map_chain_cnt( &rec_map );
-  ulong * lock_seq             = (ulong *)fd_alloc_malloc( alloc, alignof(ulong), 1UL );
+  ulong * lock_seq             = (ulong *)fd_alloc_malloc( alloc, alignof(ulong), sizeof(ulong) * lock_cnt );
   for( ulong lock_idx=0UL; lock_idx<lock_cnt; lock_idx++ ) {
     lock_seq[ lock_idx ] = lock_idx;
   }