-
Notifications
You must be signed in to change notification settings - Fork 132
Move instruction profiling to a dedicated thread #526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,7 +70,7 @@ use crate::{ | |
| error::{SurfpoolError, SurfpoolResult}, | ||
| helpers::time_travel::calculate_time_travel_clock, | ||
| rpc::utils::{convert_transaction_metadata_from_canonical, verify_pubkey}, | ||
| surfnet::{FINALIZATION_SLOT_THRESHOLD, SLOTS_PER_EPOCH}, | ||
| surfnet::{FINALIZATION_SLOT_THRESHOLD, ProfilingJob, SLOTS_PER_EPOCH}, | ||
| types::{ | ||
| GeyserAccountUpdate, OfflineAccountConfig, RemoteRpcResult, SurfnetTransactionStatus, | ||
| TimeTravelConfig, TokenAccount, TransactionLoadedAddresses, TransactionWithStatusMeta, | ||
|
|
@@ -1096,7 +1096,7 @@ impl SurfnetSvmLocker { | |
| ) -> SurfpoolResult<()> { | ||
| let do_propagate_status_updates = true; | ||
| let signature = transaction.signatures[0]; | ||
| let profile_result = match self | ||
| let (profile_result, ix_profile_rx) = match self | ||
| .fetch_all_tx_accounts_then_process_tx_returning_profile_res( | ||
| remote_ctx, | ||
| transaction, | ||
|
|
@@ -1133,6 +1133,32 @@ impl SurfnetSvmLocker { | |
| self.with_svm_writer(|svm_writer| { | ||
| svm_writer.write_executed_profile_result(signature, profile_result) | ||
| })?; | ||
|
|
||
| // Spawn an async task to receive profiling results and append them to the | ||
| // stored profiles | ||
| if let Some(rx) = ix_profile_rx { | ||
| let locker = self.clone(); | ||
| tokio::spawn(async move { | ||
| let ix_profiles = tokio::task::spawn_blocking(move || rx.recv().ok().flatten()) | ||
| .await | ||
| .ok() | ||
| .flatten(); | ||
| if let Some(profiles) = ix_profiles { | ||
| locker.with_svm_writer(|svm_writer| { | ||
| if let Ok(Some(mut keyed_profile)) = svm_writer | ||
| .executed_transaction_profiles | ||
| .get(&signature.to_string()) | ||
| { | ||
| keyed_profile.instruction_profiles = Some(profiles); | ||
| let _ = svm_writer | ||
| .executed_transaction_profiles | ||
| .store(signature.to_string(), keyed_profile); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
|
|
@@ -1153,7 +1179,7 @@ impl SurfnetSvmLocker { | |
| let skip_preflight = true; // skip preflight checks during transaction profiling | ||
| let sigverify = true; // do verify signatures during transaction profiling | ||
| let do_propagate_status_updates = false; // don't propagate status updates during transaction profiling | ||
| let mut profile_result = svm_locker | ||
| let (mut profile_result, ix_profile_rx) = svm_locker | ||
| .fetch_all_tx_accounts_then_process_tx_returning_profile_res( | ||
| remote_ctx, | ||
| transaction, | ||
|
|
@@ -1171,9 +1197,37 @@ impl SurfnetSvmLocker { | |
| svm_writer.write_simulated_profile_result(uuid, tag, profile_result) | ||
| })?; | ||
|
|
||
| if let Some(rx) = ix_profile_rx { | ||
| let locker = self.clone(); | ||
| tokio::spawn(async move { | ||
| let ix_profiles = tokio::task::spawn_blocking(move || rx.recv().ok().flatten()) | ||
| .await | ||
| .ok() | ||
| .flatten(); | ||
| if let Some(profiles) = ix_profiles { | ||
| locker.with_svm_writer(|svm_writer| { | ||
| if let Ok(Some(mut keyed_profile)) = svm_writer | ||
| .simulated_transaction_profiles | ||
| .get(&uuid.to_string()) | ||
| { | ||
| keyed_profile.instruction_profiles = Some(profiles); | ||
| let _ = svm_writer | ||
| .simulated_transaction_profiles | ||
| .store(uuid.to_string(), keyed_profile); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| Ok(self.with_contextualized_svm_reader(|_| uuid)) | ||
| } | ||
|
|
||
|
|
||
| pub fn profiling_job_tx(&self) -> Option<Sender<ProfilingJob>> { | ||
| self.with_svm_reader(|svm| svm.profiling_job_tx.clone()) | ||
| } | ||
|
|
||
| async fn fetch_all_tx_accounts_then_process_tx_returning_profile_res( | ||
| &self, | ||
| remote_ctx: &Option<(SurfnetRemoteClient, CommitmentConfig)>, | ||
|
|
@@ -1182,7 +1236,7 @@ impl SurfnetSvmLocker { | |
| skip_preflight: bool, | ||
| sigverify: bool, | ||
| do_propagate: bool, | ||
| ) -> SurfpoolResult<KeyedProfileResult> { | ||
| ) -> SurfpoolResult<(KeyedProfileResult, Option<crossbeam_channel::Receiver<Option<Vec<ProfileResult>>>>)> { | ||
| let signature = transaction.signatures[0]; | ||
|
|
||
| // Sigverify the transaction upfront before doing any account fetching or other pre-processing. | ||
|
|
@@ -1338,29 +1392,28 @@ impl SurfnetSvmLocker { | |
|
|
||
| let loaded_addresses = tx_loaded_addresses.as_ref().map(|l| l.loaded_addresses()); | ||
|
|
||
| let ix_profiles = if self.is_instruction_profiling_enabled() { | ||
| match self | ||
| .generate_instruction_profiles( | ||
| &transaction, | ||
| &transaction_accounts, | ||
| &tx_loaded_addresses, | ||
| &accounts_before, | ||
| &token_accounts_before, | ||
| &token_programs, | ||
| pre_execution_capture.clone(), | ||
| &status_tx, | ||
| ) | ||
| .await | ||
| { | ||
| Ok(profiles) => profiles, | ||
| Err(e) => { | ||
| let _ = self.simnet_events_tx().try_send(SimnetEvent::error(format!( | ||
| "Failed to generate instruction profiles: {}", | ||
| e | ||
| ))); | ||
| None | ||
| } | ||
| let ix_profile_rx = if self.is_instruction_profiling_enabled() { | ||
| if let Some(profiling_job_tx) = self.profiling_job_tx() { | ||
| let profiling_svm = self.with_svm_reader(|r| r.clone_for_profiling()); | ||
| let (result_tx, result_rx) = crossbeam_channel::bounded(1); | ||
| let job = ProfilingJob { | ||
| profiling_svm, | ||
| transaction: transaction.clone(), | ||
| transaction_accounts: transaction_accounts.to_vec(), | ||
| loaded_addresses: tx_loaded_addresses.clone(), | ||
| accounts_before: accounts_before.to_vec(), | ||
| token_accounts_before: token_accounts_before.to_vec(), | ||
| token_programs: token_programs.to_vec(), | ||
| pre_execution_capture: pre_execution_capture.clone(), | ||
| result_tx, | ||
| }; | ||
| let _ = profiling_job_tx.send(job); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this Instead let's us 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
}
} |
||
| Some(result_rx) | ||
| } | ||
| else { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If instruction profiling is enabled, and there is no The runloop file does call |
||
| None | ||
| } | ||
|
|
||
| } else { | ||
| None | ||
| }; | ||
|
|
@@ -1381,17 +1434,20 @@ impl SurfnetSvmLocker { | |
| ) | ||
| .await?; | ||
|
|
||
| Ok(KeyedProfileResult::new( | ||
| latest_absolute_slot, | ||
| UuidOrSignature::Signature(signature), | ||
| ix_profiles, | ||
| profile_result, | ||
| readonly_account_states, | ||
| Ok(( | ||
| KeyedProfileResult::new( | ||
| latest_absolute_slot, | ||
| UuidOrSignature::Signature(signature), | ||
| None, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So previously we would immediately return ix profiles, now that obviously isn't possible. But this 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}"
} |
||
| profile_result, | ||
| readonly_account_states, | ||
| ), | ||
| ix_profile_rx, | ||
| )) | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] | ||
| async fn generate_instruction_profiles( | ||
| pub async fn generate_instruction_profiles( | ||
| &self, | ||
| transaction: &VersionedTransaction, | ||
| transaction_accounts: &[Pubkey], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wrap this in an: