remove solana-sdk from solana-timings (#3731)

* remove solana-sdk from solana-timings * use std::num::Saturating * update tests after switching to num::Saturating * update timing usage in bank.rs * update timings usage in solana-ledger * update timings usage in core * update timings usage in core tests * update other structs in solana-timings * update more timings usage * update tests * update svm tests * update unified-scheduler-pool testgs
anza-xyz · Dec 6, 2024 · 89872fd · 89872fd
1 parent 8e9e92c
commit 89872fd
Show file tree

Hide file tree

Showing 17 changed files with 170 additions and 214 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs
@@ -44,6 +44,7 @@ use {
     solana_svm_transaction::svm_message::SVMMessage,
     solana_timings::ExecuteTimings,
     std::{
+        num::Saturating,
         sync::{atomic::Ordering, Arc},
         time::Instant,
     },
@@ -792,10 +793,11 @@ impl Consumer {
             (0, 0),
             |(units, times), program_timings| {
                 (
-                    units
-                        .saturating_add(program_timings.accumulated_units)
-                        .saturating_add(program_timings.total_errored_units),
-                    times.saturating_add(program_timings.accumulated_us),
+                    (Saturating(units)
+                        + program_timings.accumulated_units
+                        + program_timings.total_errored_units)
+                        .0,
+                    (Saturating(times) + program_timings.accumulated_us).0,
                 )
             },
         )
@@ -2522,11 +2524,11 @@ mod tests {
             execute_timings.details.per_program_timings.insert(
                 Pubkey::new_unique(),
                 ProgramTiming {
-                    accumulated_us: n * 100,
-                    accumulated_units: n * 1000,
-                    count: n as u32,
+                    accumulated_us: Saturating(n * 100),
+                    accumulated_units: Saturating(n * 1000),
+                    count: Saturating(n as u32),
                     errored_txs_compute_consumed: vec![],
-                    total_errored_units: 0,
+                    total_errored_units: Saturating(0),
                 },
             );
             expected_us += n * 100;

diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs
@@ -1446,10 +1446,10 @@ impl ReplaySlotStats {
                 (0, 0, 0, 0, 0),
                 |(sum_us, sum_units, sum_count, sum_errored_units, sum_errored_count), a| {
                     (
-                        sum_us + a.1.accumulated_us,
-                        sum_units + a.1.accumulated_units,
-                        sum_count + a.1.count,
-                        sum_errored_units + a.1.total_errored_units,
+                        sum_us + a.1.accumulated_us.0,
+                        sum_units + a.1.accumulated_units.0,
+                        sum_count + a.1.count.0,
+                        sum_errored_units + a.1.total_errored_units.0,
                         sum_errored_count + a.1.errored_txs_compute_consumed.len(),
                     )
                 },
@@ -1460,10 +1460,10 @@ impl ReplaySlotStats {
                 "per_program_timings",
                 ("slot", slot as i64, i64),
                 ("pubkey", pubkey.to_string(), String),
-                ("execute_us", time.accumulated_us, i64),
-                ("accumulated_units", time.accumulated_units, i64),
-                ("errored_units", time.total_errored_units, i64),
-                ("count", time.count, i64),
+                ("execute_us", time.accumulated_us.0, i64),
+                ("accumulated_units", time.accumulated_units.0, i64),
+                ("errored_units", time.total_errored_units.0, i64),
+                ("count", time.count.0, i64),
                 (
                     "errored_count",
                     time.errored_txs_compute_consumed.len(),

diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs
@@ -31,7 +31,6 @@ use {
         native_loader,
         precompiles::Precompile,
         pubkey::Pubkey,
-        saturating_add_assign,
         stable_layout::stable_instruction::StableInstruction,
         sysvar,
         transaction_context::{
@@ -601,13 +600,10 @@ impl<'a> InvokeContext<'a> {
             return Err(InstructionError::BuiltinProgramsMustConsumeComputeUnits);
         }
 
-        saturating_add_assign!(
-            timings
-                .execute_accessories
-                .process_instructions
-                .process_executable_chain_us,
-            process_executable_chain_time.end_as_us()
-        );
+        timings
+            .execute_accessories
+            .process_instructions
+            .process_executable_chain_us += process_executable_chain_time.end_as_us();
         result
     }
 

diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs
@@ -289,13 +289,10 @@ pub struct LoadProgramMetrics {
 
 impl LoadProgramMetrics {
     pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) {
-        saturating_add_assign!(
-            timings.create_executor_register_syscalls_us,
-            self.register_syscalls_us
-        );
-        saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us);
-        saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us);
-        saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us);
+        timings.create_executor_register_syscalls_us += self.register_syscalls_us;
+        timings.create_executor_load_elf_us += self.load_elf_us;
+        timings.create_executor_verify_code_us += self.verify_code_us;
+        timings.create_executor_jit_compile_us += self.jit_compile_us;
         datapoint_trace!(
             "create_executor_trace",
             ("program_id", self.program_id, String),

diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs
@@ -43,7 +43,6 @@ use {
         loader_v4, native_loader,
         program_utils::limited_deserialize,
         pubkey::Pubkey,
-        saturating_add_assign,
         system_instruction::{self, MAX_PERMITTED_DATA_LENGTH},
         transaction_context::{IndexOfAccount, InstructionContext, TransactionContext},
     },
@@ -469,10 +468,7 @@ pub fn process_instruction_inner(
         })?;
     drop(program_account);
     get_or_create_executor_time.stop();
-    saturating_add_assign!(
-        invoke_context.timings.get_or_create_executor_us,
-        get_or_create_executor_time.as_us()
-    );
+    invoke_context.timings.get_or_create_executor_us += get_or_create_executor_time.as_us();
 
     executor.ix_usage_counter.fetch_add(1, Ordering::Relaxed);
     match &executor.program {
@@ -1452,7 +1448,7 @@ pub fn execute<'a, 'b: 'a>(
         drop(vm);
         if let Some(execute_time) = invoke_context.execute_time.as_mut() {
             execute_time.stop();
-            saturating_add_assign!(invoke_context.timings.execute_us, execute_time.as_us());
+            invoke_context.timings.execute_us += execute_time.as_us();
         }
 
         ic_logger_msg!(
@@ -1567,12 +1563,9 @@ pub fn execute<'a, 'b: 'a>(
     deserialize_time.stop();
 
     // Update the timings
-    saturating_add_assign!(invoke_context.timings.serialize_us, serialize_time.as_us());
-    saturating_add_assign!(invoke_context.timings.create_vm_us, create_vm_time.as_us());
-    saturating_add_assign!(
-        invoke_context.timings.deserialize_us,
-        deserialize_time.as_us()
-    );
+    invoke_context.timings.serialize_us += serialize_time.as_us();
+    invoke_context.timings.create_vm_us += create_vm_time.as_us();
+    invoke_context.timings.deserialize_us += deserialize_time.as_us();
 
     execute_or_deserialize_result
 }

diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs
@@ -10,7 +10,6 @@ use {
         memory_region::{MemoryRegion, MemoryState},
     },
     solana_sdk::{
-        saturating_add_assign,
         stable_layout::stable_instruction::StableInstruction,
         syscalls::{
             MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN,
@@ -1087,7 +1086,7 @@ fn cpi_common<S: SyscallInvokeSigned>(
     )?;
     if let Some(execute_time) = invoke_context.execute_time.as_mut() {
         execute_time.stop();
-        saturating_add_assign!(invoke_context.timings.execute_us, execute_time.as_us());
+        invoke_context.timings.execute_us += execute_time.as_us();
     }
 
     let instruction = S::translate_instruction(instruction_addr, memory_mapping, invoke_context)?;

diff --git a/programs/loader-v4/src/lib.rs b/programs/loader-v4/src/lib.rs
@@ -13,7 +13,6 @@ use {
         loader_v4_instruction::LoaderV4Instruction,
         program_utils::limited_deserialize,
         pubkey::Pubkey,
-        saturating_add_assign,
         transaction_context::{BorrowedAccount, InstructionContext},
     },
     solana_type_overrides::sync::{atomic::Ordering, Arc},
@@ -456,10 +455,7 @@ pub fn process_instruction_inner(
                 InstructionError::UnsupportedProgramId
             })?;
         get_or_create_executor_time.stop();
-        saturating_add_assign!(
-            invoke_context.timings.get_or_create_executor_us,
-            get_or_create_executor_time.as_us()
-        );
+        invoke_context.timings.get_or_create_executor_us += get_or_create_executor_time.as_us();
         drop(program);
         loaded_program
             .ix_usage_counter

diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -3761,8 +3761,10 @@ impl Bank {
                 .per_program_timings
                 .iter()
                 .fold(0, |acc: u64, (_, program_timing)| {
-                    acc.saturating_add(program_timing.accumulated_units)
-                        .saturating_add(program_timing.total_errored_units)
+                    (std::num::Saturating(acc)
+                        + program_timing.accumulated_units
+                        + program_timing.total_errored_units)
+                        .0
                 });
 
         debug!("simulate_transaction: {:?}", timings);

diff --git a/svm/examples/Cargo.lock b/svm/examples/Cargo.lock
diff --git a/svm/src/message_processor.rs b/svm/src/message_processor.rs
@@ -4,7 +4,6 @@ use {
     solana_sdk::{
         account::WritableAccount,
         precompiles::get_precompile,
-        saturating_add_assign,
         sysvar::instructions,
         transaction::TransactionError,
         transaction_context::{IndexOfAccount, InstructionAccount},
@@ -118,13 +117,10 @@ impl MessageProcessor {
                 execute_timings.details.accumulate(&invoke_context.timings);
                 ExecuteDetailsTimings::default()
             };
-            saturating_add_assign!(
-                execute_timings
-                    .execute_accessories
-                    .process_instructions
-                    .total_us,
-                process_instruction_us
-            );
+            execute_timings
+                .execute_accessories
+                .process_instructions
+                .total_us += process_instruction_us;
 
             result
                 .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;

diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs
@@ -1011,10 +1011,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
 
         drop(invoke_context);
 
-        saturating_add_assign!(
-            execute_timings.execute_accessories.process_message_us,
-            process_message_time.as_us()
-        );
+        execute_timings.execute_accessories.process_message_us += process_message_time.as_us();
 
         let mut status = process_result
             .and_then(|info| {
@@ -1076,14 +1073,8 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         let status = status.map(|_| ());
 
         loaded_transaction.accounts = accounts;
-        saturating_add_assign!(
-            execute_timings.details.total_account_count,
-            loaded_transaction.accounts.len() as u64
-        );
-        saturating_add_assign!(
-            execute_timings.details.changed_account_count,
-            touched_account_count
-        );
+        execute_timings.details.total_account_count += loaded_transaction.accounts.len() as u64;
+        execute_timings.details.changed_account_count += touched_account_count;
 
         let return_data = if config.recording_config.enable_return_data_recording
             && !return_data.data.is_empty()

diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs
@@ -2470,18 +2470,20 @@ fn svm_metrics_accumulation() {
             result
                 .execute_timings
                 .details
-                .create_executor_jit_compile_us,
+                .create_executor_jit_compile_us
+                .0,
             0
         );
         assert_ne!(
-            result.execute_timings.details.create_executor_load_elf_us,
+            result.execute_timings.details.create_executor_load_elf_us.0,
             0
         );
         assert_ne!(
             result
                 .execute_timings
                 .details
-                .create_executor_verify_code_us,
+                .create_executor_verify_code_us
+                .0,
             0
         );
     }

diff --git a/timings/Cargo.toml b/timings/Cargo.toml
@@ -12,7 +12,7 @@ edition = { workspace = true }
 [dependencies]
 eager = { workspace = true }
 enum-iterator = { workspace = true }
-solana-sdk = { workspace = true }
+solana-pubkey = { workspace = true }
 
 [package.metadata.docs.rs]
 targets = ["x86_64-unknown-linux-gnu"]