Move instruction profiling to a dedicated thread

[mohitejaikumar]

Implements #320

1. Added start_profiling_runloop that spawns a long-lived "Instruction Profiler" thread via hiro_system_kit::thread_named.
2. ProfilingJob struct - Packages all data needed for profiling (cloned SVM, transaction, accounts, etc.) to send across the thread boundary.
3. Added Option<Sender<ProfilingJob>> to SurfnetSvm.
4. fetch_all_tx_accounts_then_process_tx_returning_profile_res now dispatches the profiling job to the dedicated thread before calling process_transaction_internal.
5. setup_profiling helper - Extracted profiling channel setup into a reusable function called from both start_local_surfnet_runloop and tests.
6. Added Clone to IndexedLoadedAddresses and TransactionLoadedAddresses to enable moving data into the profiling job.
7. Added poll_for_instruction_profiles helper in helpers.rs that polls get_profile_result every 50ms until instruction_profiles is Some or a 10s timeout is reached
8. Updated 5 assertions across 4 tests to use the poll helper:
   test_profile_transaction_multi_instruction_basic, test_profile_transaction_token_transfer, test_profile_transaction_multi_instruction_failure, test_ix_profiling_with_alt_tx, test_instruction_profiling_does_not_mutate_state.

[mohitejaikumar]

@lgalabru I’d really appreciate your review and any feedback you may have.

[MicaiahReid]

This is really awesome, @mohitejaikumar! I still haven't gotten to test yet so I'm not ready to sign-off.

One concern I have - currently this code:

let ix_profiles = match ix_profile_rx {
    Some(rx) => tokio::task::block_in_place(|| rx.recv().ok().flatten()),
    None => None,
};

is happening directly after the transaction is processed for the real result.

So in the original implementation, the an instruction profile for a transaction with 10 instructions executed 10 transactions:

TX 1: (IX 1)
TX 2: (IX 1 + IX 2)
TX 3: (IX 1 + IX 2 + IX 3)
...
TX 10: Final tx with all ixs

Your implementation slightly parallelizes by spawning a thread for 1-9, awaiting tx 10, then awaiting the thread:

TX 1               TX 10
TX 2               (tx 10 complete, awaiting thread completion)
TX 3
...
TX 9

So this only shortens the number of txs the user is waiting on by 1.

---

Can we slightly expand this approach. Rather than waiting on the instruction profiling thread to complete, we return the original result without ix profiles and append them later? Later, if the user fetches the profile result for a signature/uuid, we'll fetch whatever we have, which is likely to include the profile result.

Thoughts @mohitejaikumar, @lgalabru?

[MicaiahReid]

@mohitejaikumar Do you think you'll be able to address my comment?

[mohitejaikumar]

@MicaiahReid yes sir I wanna try it, giving it a try today, was bussy with some work.

[mohitejaikumar]

@MicaiahReid

1. The shared function fetch_all_tx_accounts_then_process_tx_returning_profile_res no longer blocks waiting for profiling results; it returns (KeyedProfileResult, Option<Receiver>) with instruction_profiles: None
2. process_transaction stores the result immediately via write_executed_profile_result, then spawns a tokio::spawn async task that calls spawn_blocking(rx.recv()) and updates executed_transaction_profiles when profiling completes.
3. profile_transaction stores the result immediately via write_simulated_profile_result, then spawns a tokio::spawn async task that updates simulated_transaction_profiles using the UUID

[MicaiahReid]

Awesome work @mohitejaikumar! This is really great. I've left some comments to get this over the finish line!

[MicaiahReid]

Let's wrap this in an:

if simnet_config.instruction_profiling_enabled {
    setup_profiling(&svm_locker);
}

[MicaiahReid]

If instruction profiling is enabled, and there is no profiling_job_tx, I assume this means there was a configuration error (setup_profiling wasn't called). Can we log a warning in this case?

The runloop file does call setup_profiling, so in the default case this won't ever be an issue, but if someone is trying to directly consume surfpool as an SDK and isn't using the runloop functions, it could be a helpful warning.

[MicaiahReid]

So previously we would immediately return ix profiles, now that obviously isn't possible. But this None could maybe be misleading - like there is no ix profile at all, or profiling is disabled. What if the profile results were stored in an enum:

enum InstructionProfiles {
    // ix profiling disabled, serializes to just `null` like the `None` variant of our option would
    Unavailable, 
    // what we would return in this line if profiling is enabled - indicates to the user that results will be available later
    Pending,
    // what is filled in upon completion
    Ready(Vec<ProfileResult>),
    // what is filled in if profiling fails
    Failed(String),
}

We'd then have to have this serialized as such:

{
  "instructionProfiles": null,
  "instructionProfilesStatus": "unavailable"
}

{
  "instructionProfiles": null,
  "instructionProfilesStatus": "pending"
}

{
  "instructionProfiles": [ ... ],
  "instructionProfilesStatus": "ready"
}

{
  "instructionProfiles": null,
  "instructionProfilesStatus": "failed: {error}"
}

[MicaiahReid]

So this .send is blocking. If our rx end of this gets filled up (bounded to 128 messages in the queue), this send will wait for the queue to free up a spot. This means the actual tx processing (which happens below) is being delayed.

Instead let's us try_send and handle errors. Something like:

match profiling_job_tx.try_send(job) {
    Ok(()) => Some(result_rx),
    Err(crossbeam_channel::TrySendError::Full(job)) => {
        // log warning or emit event saying this transaction will not be profiled
        None
    }
    Err(crossbeam_channel::TrySendError::Disconnected(job)) => {
        // worker is gone; log an error and just disable ix profiling so we don't log again and again
        None
    }
}