funk: simplify 'try_clone_safe' into 'insert_para'

Remove record allocation and copy from try_clone_safe.

Existing callers effectively only use this API as 'update existing
record'.

Author: riptl

src/flamenco/runtime/program/fd_program_cache.c

@@ -573,18 +573,13 @@ FD_SPAD_FRAME_BEGIN( runtime_spad ) {
     record_sz = fd_program_cache_entry_footprint( &elf_info );
   }
 
-  /* Copy the record (if needed) down into the current funk txn from one
-     of its ancestors.
-
-     TODO: We pass in a `min_sz` of 0 because this API does not resize
-     the record if it already exists in the current funk transaction.
-     This maybe needs to change. */
-  fd_funk_rec_try_clone_safe( slot_ctx->funk, slot_ctx->funk_txn, &id, 0UL, 0UL );
+  /* Insert a new funk record, replacing the existing one if needed.
+     min_sz==0 since the actual allocation happens below. */
+  fd_funk_rec_insert_para( slot_ctx->funk, slot_ctx->funk_txn, &id );
 
   /* Modify the record within the current funk txn */
   fd_funk_rec_query_t query[1];
   fd_funk_rec_t * rec = fd_funk_rec_modify( slot_ctx->funk, slot_ctx->funk_txn, &id, query );
-
   if( FD_UNLIKELY( !rec ) ) {
     /* The record does not exist (somehow). Ideally this should never
        happen as this function is called in a single-threaded context. */
@@ -648,24 +643,34 @@ fd_program_cache_queue_program_for_reverification( fd_funk_t *         funk,
 
   /* Ensure the record is in the current funk transaction */
   fd_funk_rec_key_t id = fd_program_cache_key( program_key );
-  fd_funk_rec_try_clone_safe( funk, funk_txn, &id, 0UL, 0UL );
+  fd_funk_rec_insert_para( funk, funk_txn, &id );
 
   /* Modify the record within the current funk txn */
   fd_funk_rec_query_t query[1];
   fd_funk_rec_t * rec = fd_funk_rec_modify( funk, funk_txn, &id, query );
-
   if( FD_UNLIKELY( !rec ) ) {
     /* The record does not exist (somehow). Ideally this should never
        happen since this function is called in a single-threaded
        context. */
     FD_LOG_CRIT(( "Failed to modify the BPF program cache record. Perhaps there is a race condition?" ));
   }
 
-  void *                     data           = fd_funk_val( rec, fd_funk_wksp( funk ) );
-  fd_program_cache_entry_t * writable_entry = fd_type_pun( data );
+  /* Insert a tombstone */
+  if( FD_UNLIKELY( !fd_funk_val_truncate(
+      rec,
+      fd_funk_alloc( funk ),
+      fd_funk_wksp( funk ),
+      alignof(fd_program_cache_entry_t),
+      sizeof(fd_program_cache_entry_t),
+      NULL ) ) ) {
+    FD_LOG_ERR(( "fd_funk_val_truncate() failed (out of memory?)" ));
+  }
 
-  /* Set the last modified slot to the current slot */
-  writable_entry->last_slot_modified = current_slot;
+  fd_program_cache_entry_t * entry = fd_funk_val( rec, fd_funk_wksp( funk ) );
+  *entry = (fd_program_cache_entry_t) {
+    .magic              = FD_PROGRAM_CACHE_ENTRY_MAGIC,
+    .last_slot_modified = current_slot
+  };
 
   /* Finish modifying and release lock */
   fd_funk_rec_modify_publish( query );

src/flamenco/runtime/program/test_program_cache.c

@@ -315,7 +315,7 @@ test_program_in_cache_queued_for_reverification( void ) {
   FD_TEST( valid_prog->magic==FD_PROGRAM_CACHE_ENTRY_MAGIC );
   FD_TEST( !valid_prog->failed_verification );
   FD_TEST( valid_prog->last_slot_modified==future_slot );
-  FD_TEST( valid_prog->last_slot_verified==original_slot );
+  FD_TEST( valid_prog->last_slot_verified==0UL );
   FD_TEST( valid_prog->last_slot_modified>original_slot );
 
   /* Reverify the cache entry at the future slot */
@@ -421,7 +421,7 @@ test_program_in_cache_queued_for_reverification_and_processed( void ) {
   FD_TEST( valid_prog->magic==FD_PROGRAM_CACHE_ENTRY_MAGIC );
   FD_TEST( !valid_prog->failed_verification );
   FD_TEST( valid_prog->last_slot_modified==future_slot );
-  FD_TEST( valid_prog->last_slot_verified==original_slot );
+  FD_TEST( valid_prog->last_slot_verified==0UL );
   FD_TEST( valid_prog->last_slot_modified>original_slot );
 
   /* Fast forward to a future slot */

src/funk/fd_funk_rec.c

@@ -328,11 +328,9 @@ fd_funk_rec_txn_publish( fd_funk_t *             funk,
 }
 
 void
-fd_funk_rec_try_clone_safe( fd_funk_t *               funk,
-                            fd_funk_txn_t *           txn,
-                            fd_funk_rec_key_t const * key,
-                            ulong                     align,
-                            ulong                     min_sz ) {
+fd_funk_rec_insert_para( fd_funk_t *               funk,
+                         fd_funk_txn_t *           txn,
+                         fd_funk_rec_key_t const * key ) {
 
   /* TODO: There is probably a cleaner way to allocate the txn memory. */
 
@@ -414,36 +412,10 @@ fd_funk_rec_try_clone_safe( fd_funk_t *               funk,
      (if one exists). */
 
   fd_funk_rec_prepare_t prepare[1];
-  fd_funk_rec_t *       new_rec = fd_funk_rec_prepare( funk, txn, key, prepare, &err );
+  fd_funk_rec_prepare( funk, txn, key, prepare, &err );
   if( FD_UNLIKELY( err ) ) {
     FD_LOG_CRIT(( "fd_funk_rec_prepare returned err=%d", err ));
   }
-
-  /* It is fine to use the old version of the record because we can
-     assume that it comes from a frozen txn. */
-  ulong old_val_sz = !!rec_glob ? rec_glob->val_sz : 0UL;
-  ulong new_val_sz = fd_ulong_max( old_val_sz, min_sz );
-
-  uchar * new_val = fd_funk_val_truncate(
-      new_rec,
-      fd_funk_alloc( funk ),
-      fd_funk_wksp( funk ),
-      align,
-      new_val_sz,
-      &err );
-  if( FD_UNLIKELY( err ) ) {
-    FD_LOG_CRIT(( "fd_funk_val_truncate returned err=%d", err ));
-  }
-  if( FD_UNLIKELY( !new_val ) ) {
-    FD_LOG_CRIT(( "fd_funk_val_truncate returned NULL" ));
-  }
-
-  if( rec_glob ) {
-    /* If we have a global copy of the record, copy it in. */
-    uchar * old_data = fd_funk_val( rec_glob, fd_funk_wksp( funk ) );
-    memcpy( new_val, old_data, old_val_sz );
-  }
-
   fd_funk_rec_txn_publish( funk, prepare );
 
   err = fd_funk_rec_map_txn_test( map_txn );

src/funk/fd_funk_rec.h

@@ -310,10 +310,7 @@ fd_funk_rec_cancel( fd_funk_t *             funk,
    modified afterward and must then be published.
 
    NOTE: fd_funk_rec_clone is NOT thread safe and should not be used
-   concurrently with other funk read/write operations.
-
-   FIXME: This function should be removed in favor of
-   fd_funk_rec_try_clone_safe. */
+   concurrently with other funk read/write operations. */
 
 fd_funk_rec_t *
 fd_funk_rec_clone( fd_funk_t *               funk,
@@ -322,11 +319,7 @@ fd_funk_rec_clone( fd_funk_t *               funk,
                    fd_funk_rec_prepare_t *   prepare,
                    int *                     opt_err );
 
-/* fd_funk_rec_try_clone_safe is the thread-safe analog to
-   fd_funk_rec_clone. This function will try to atomically query and
-   copy the given funk record from the youngest ancestor transaction
-   of the given txn and copy it into a new record of the same key into
-   the current funk txn.
+/* fd_funk_rec_insert_para does thread-safe insertion of a funk record.
 
    Detailed Behavior:
 
@@ -345,22 +338,14 @@ fd_funk_rec_clone( fd_funk_t *               funk,
    that we were attempting to acquire the lock. If a keypair is found,
    we will free the lock and exit the function.
 
-   Otherwise, we will allocate the account and copy over the data from
-   the ancestor record. Now we will add this into the record map. At
-   this point, we can now free the lock on the hash chain.
-
-   The caller can specify the alignment and min_sz they would like for
-   the value of the record. If the caller wishes to use default
-   alignment, they can pass 0UL (see fd_funk_val_truncate() for more
-   details). */
+   Otherwise, we will allocate a new account record. Now we will add
+   this into the record map. At this point, we can now free the lock on
+   the hash chain. */
 
 void
-fd_funk_rec_try_clone_safe( fd_funk_t *               funk,
-                            fd_funk_txn_t *           txn,
-                            fd_funk_rec_key_t const * key,
-                            ulong                     align,
-                            ulong                     min_sz );
-
+fd_funk_rec_insert_para( fd_funk_t *               funk,
+                         fd_funk_txn_t *           txn,
+                         fd_funk_rec_key_t const * key );
 
 /* fd_funk_rec_remove removes the live record with the
    given (xid,key) from funk. Returns FD_FUNK_SUCCESS (0) on

src/funk/fd_funk_val.h

@@ -41,15 +41,15 @@ fd_funk_val_max( fd_funk_rec_t const * rec ) { /* Assumes pointer in caller's ad
 
 FD_FN_PURE static inline void *             /* Lifetime is the lesser of rec or the value size is modified */
 fd_funk_val( fd_funk_rec_t const * rec,     /* Assumes pointer in caller's address space to a live funk record */
-                fd_wksp_t const *        wksp ) { /* ==fd_funk_wksp( funk ) where funk is a current local join */
+             fd_wksp_t const *     wksp ) { /* ==fd_funk_wksp( funk ) where funk is a current local join */
   ulong val_gaddr = rec->val_gaddr;
   if( !val_gaddr ) return NULL; /* Covers the marked ERASE case too */ /* TODO: consider branchless */
   return fd_wksp_laddr_fast( wksp, val_gaddr );
 }
 
 FD_FN_PURE static inline void const *             /* Lifetime is the lesser of rec or the value size is modified */
 fd_funk_val_const( fd_funk_rec_t const * rec,     /* Assumes pointer in caller's address space to a live funk record */
-                      fd_wksp_t const *        wksp ) { /* ==fd_funk_wksp( funk ) where funk is a current local join */
+                   fd_wksp_t const *     wksp ) { /* ==fd_funk_wksp( funk ) where funk is a current local join */
   ulong val_gaddr = rec->val_gaddr;
   if( !val_gaddr ) return NULL; /* Covers the marked ERASE case too */ /* TODO: consider branchless */
   return fd_wksp_laddr_fast( wksp, val_gaddr );

src/funk/test_funk_concur2.cxx

@@ -28,19 +28,18 @@ static void * work_thread( void * arg ) {
     key.ul[0] = key_idx;
 
     /* First try to clone the record from the ancestor. */
-    fd_funk_rec_try_clone_safe( funk, txn, &key, alignof(ulong), sizeof(ulong) );
+    fd_funk_rec_insert_para( funk, txn, &key );
 
     /* Ensure that the record exists for the current txn. */
-
     fd_funk_rec_query_t query_check[1];
     fd_funk_rec_t const * rec_check = fd_funk_rec_query_try( funk, txn, &key, query_check );
     FD_TEST( rec_check );
 
     /* Now modify the record. */
-
     fd_funk_rec_query_t query_modify[1];
     fd_funk_rec_t * rec = fd_funk_rec_modify( funk, txn, &key, query_modify );
     FD_TEST( rec );
+    FD_TEST( fd_funk_val_truncate( rec, fd_funk_alloc( funk ), fd_funk_wksp( funk ), alignof(ulong), sizeof(ulong), NULL ) );
     void * val = fd_funk_val( rec, fd_funk_wksp(funk) );
     ulong * val_ul = (ulong *)val;
     *val_ul += 1UL;