diff --git a/core/src/cost_model.rs b/core/src/cost_model.rs index 0f32a437ad..18e9799707 100644 --- a/core/src/cost_model.rs +++ b/core/src/cost_model.rs @@ -92,7 +92,7 @@ impl CostModel { pub fn initialize_cost_table(&mut self, cost_table: &[(Pubkey, u64)]) { for (program_id, cost) in cost_table { - match self.upsert_instruction_cost(program_id, cost) { + match self.upsert_instruction_cost(program_id, *cost) { Ok(c) => { debug!( "initiating cost table, instruction {:?} has cost {}", @@ -147,7 +147,7 @@ impl CostModel { pub fn upsert_instruction_cost( &mut self, program_key: &Pubkey, - cost: &u64, + cost: u64, ) -> Result { self.instruction_execution_cost_table .upsert(program_key, cost); @@ -232,12 +232,12 @@ mod tests { let mut testee = CostModel::default(); let known_key = Pubkey::from_str("known11111111111111111111111111111111111111").unwrap(); - testee.upsert_instruction_cost(&known_key, &100).unwrap(); + testee.upsert_instruction_cost(&known_key, 100).unwrap(); // find cost for known programs assert_eq!(100, testee.find_instruction_cost(&known_key)); testee - .upsert_instruction_cost(&bpf_loader::id(), &1999) + .upsert_instruction_cost(&bpf_loader::id(), 1999) .unwrap(); assert_eq!(1999, testee.find_instruction_cost(&bpf_loader::id())); @@ -267,7 +267,7 @@ mod tests { let mut testee = CostModel::default(); testee - .upsert_instruction_cost(&system_program::id(), &expected_cost) + .upsert_instruction_cost(&system_program::id(), expected_cost) .unwrap(); assert_eq!( expected_cost, @@ -293,7 +293,7 @@ mod tests { let mut testee = CostModel::default(); testee - .upsert_instruction_cost(&system_program::id(), &program_cost) + .upsert_instruction_cost(&system_program::id(), program_cost) .unwrap(); assert_eq!(expected_cost, testee.find_transaction_cost(&tx)); } @@ -371,7 +371,7 @@ mod tests { ); // insert instruction cost to table - assert!(cost_model.upsert_instruction_cost(&key1, &cost1).is_ok()); + assert!(cost_model.upsert_instruction_cost(&key1, cost1).is_ok()); // now it is known insturction with known cost assert_eq!(cost1, cost_model.find_instruction_cost(&key1)); @@ -390,7 +390,7 @@ mod tests { let mut cost_model = CostModel::default(); cost_model - .upsert_instruction_cost(&system_program::id(), &expected_execution_cost) + .upsert_instruction_cost(&system_program::id(), expected_execution_cost) .unwrap(); let tx_cost = cost_model.calculate_cost(&tx); assert_eq!(expected_account_cost, tx_cost.account_access_cost); @@ -408,11 +408,11 @@ mod tests { let mut cost_model = CostModel::default(); // insert instruction cost to table - assert!(cost_model.upsert_instruction_cost(&key1, &cost1).is_ok()); + assert!(cost_model.upsert_instruction_cost(&key1, cost1).is_ok()); assert_eq!(cost1, cost_model.find_instruction_cost(&key1)); // update instruction cost - assert!(cost_model.upsert_instruction_cost(&key1, &cost2).is_ok()); + assert!(cost_model.upsert_instruction_cost(&key1, cost2).is_ok()); assert_eq!(updated_cost, cost_model.find_instruction_cost(&key1)); } @@ -454,8 +454,8 @@ mod tests { if i == 5 { thread::spawn(move || { let mut cost_model = cost_model.write().unwrap(); - assert!(cost_model.upsert_instruction_cost(&prog1, &cost1).is_ok()); - assert!(cost_model.upsert_instruction_cost(&prog2, &cost2).is_ok()); + assert!(cost_model.upsert_instruction_cost(&prog1, cost1).is_ok()); + assert!(cost_model.upsert_instruction_cost(&prog2, cost2).is_ok()); }) } else { thread::spawn(move || { diff --git a/core/src/cost_tracker.rs b/core/src/cost_tracker.rs index 782c920465..217c19c2a2 100644 --- a/core/src/cost_tracker.rs +++ b/core/src/cost_tracker.rs @@ -1,4 +1,4 @@ -//! `cost_tracker` keeps tracking tranasction cost per chained accounts as well as for entire block +//! `cost_tracker` keeps tracking transaction cost per chained accounts as well as for entire block //! It aggregates `cost_model`, which provides service of calculating transaction cost. //! The main functions are: //! - would_transaction_fit(&tx), immutable function to test if `tx` would fit into current block diff --git a/core/src/cost_update_service.rs b/core/src/cost_update_service.rs index f3e4253540..084a3e2e1a 100644 --- a/core/src/cost_update_service.rs +++ b/core/src/cost_update_service.rs @@ -135,26 +135,27 @@ impl CostUpdateService { fn update_cost_model(cost_model: &RwLock, execute_timings: &ExecuteTimings) -> bool { let mut dirty = false; - let mut cost_model_mutable = cost_model.write().unwrap(); - for (program_id, stats) in &execute_timings.details.per_program_timings { - let cost = stats.0 / stats.1 as u64; - match cost_model_mutable.upsert_instruction_cost(program_id, &cost) { - Ok(c) => { - debug!( - "after replayed into bank, instruction {:?} has averaged cost {}", - program_id, c - ); - dirty = true; - } - Err(err) => { - debug!( + { + let mut cost_model_mutable = cost_model.write().unwrap(); + for (program_id, tining) in &execute_timings.details.per_program_timings { + let cost = tining.accumulated_us / tining.count as u64; + match cost_model_mutable.upsert_instruction_cost(program_id, cost) { + Ok(c) => { + debug!( + "after replayed into bank, instruction {:?} has averaged cost {}", + program_id, c + ); + dirty = true; + } + Err(err) => { + debug!( "after replayed into bank, instruction {:?} failed to update cost, err: {}", program_id, err ); + } } } } - drop(cost_model_mutable); debug!( "after replayed into bank, updated cost model instruction cost table, current values: {:?}", cost_model.read().unwrap().get_instruction_cost_table() @@ -187,6 +188,7 @@ impl CostUpdateService { #[cfg(test)] mod tests { use super::*; + use solana_runtime::message_processor::ProgramTiming; use solana_sdk::pubkey::Pubkey; #[test] @@ -219,10 +221,13 @@ mod tests { let count: u32 = 10; expected_cost = accumulated_us / count as u64; - execute_timings - .details - .per_program_timings - .insert(program_key_1, (accumulated_us, count)); + execute_timings.details.per_program_timings.insert( + program_key_1, + ProgramTiming { + accumulated_us, + count, + }, + ); CostUpdateService::update_cost_model(&cost_model, &execute_timings); assert_eq!( 1, @@ -249,10 +254,13 @@ mod tests { // to expect new cost is Average(new_value, existing_value) expected_cost = ((accumulated_us / count as u64) + expected_cost) / 2; - execute_timings - .details - .per_program_timings - .insert(program_key_1, (accumulated_us, count)); + execute_timings.details.per_program_timings.insert( + program_key_1, + ProgramTiming { + accumulated_us, + count, + }, + ); CostUpdateService::update_cost_model(&cost_model, &execute_timings); assert_eq!( 1, diff --git a/core/src/execute_cost_table.rs b/core/src/execute_cost_table.rs index 1a3ee443a9..fd0c67a9a7 100644 --- a/core/src/execute_cost_table.rs +++ b/core/src/execute_cost_table.rs @@ -78,15 +78,15 @@ impl ExecuteCostTable { self.table.get(key) } - pub fn upsert(&mut self, key: &Pubkey, value: &u64) { + pub fn upsert(&mut self, key: &Pubkey, value: u64) { let need_to_add = self.table.get(key).is_none(); let current_size = self.get_count(); if current_size == self.capacity && need_to_add { self.prune_to(&((current_size as f64 * PRUNE_RATIO) as usize)); } - let program_cost = self.table.entry(*key).or_insert(*value); - *program_cost = (*program_cost + *value) / 2; + let program_cost = self.table.entry(*key).or_insert(value); + *program_cost = (*program_cost + value) / 2; let (count, timestamp) = self .occurrences @@ -154,9 +154,9 @@ mod tests { let key2 = Pubkey::new_unique(); let key3 = Pubkey::new_unique(); - testee.upsert(&key1, &1); - testee.upsert(&key2, &2); - testee.upsert(&key3, &3); + testee.upsert(&key1, 1); + testee.upsert(&key2, 2); + testee.upsert(&key3, 3); testee.prune_to(&(capacity - 1)); @@ -176,10 +176,10 @@ mod tests { let key2 = Pubkey::new_unique(); let key3 = Pubkey::new_unique(); - testee.upsert(&key1, &1); - testee.upsert(&key1, &1); - testee.upsert(&key2, &2); - testee.upsert(&key3, &3); + testee.upsert(&key1, 1); + testee.upsert(&key1, 1); + testee.upsert(&key2, 2); + testee.upsert(&key3, 3); testee.prune_to(&(capacity - 1)); @@ -204,14 +204,14 @@ mod tests { assert!(testee.get_cost(&key1).is_none()); // insert one record - testee.upsert(&key1, &cost1); + testee.upsert(&key1, cost1); assert_eq!(1, testee.get_count()); assert_eq!(cost1, testee.get_average()); assert_eq!(cost1, testee.get_mode()); assert_eq!(&cost1, testee.get_cost(&key1).unwrap()); // insert 2nd record - testee.upsert(&key2, &cost2); + testee.upsert(&key2, cost2); assert_eq!(2, testee.get_count()); assert_eq!((cost1 + cost2) / 2_u64, testee.get_average()); assert_eq!(cost2, testee.get_mode()); @@ -219,7 +219,7 @@ mod tests { assert_eq!(&cost2, testee.get_cost(&key2).unwrap()); // update 1st record - testee.upsert(&key1, &cost2); + testee.upsert(&key1, cost2); assert_eq!(2, testee.get_count()); assert_eq!(((cost1 + cost2) / 2 + cost2) / 2, testee.get_average()); assert_eq!((cost1 + cost2) / 2, testee.get_mode()); @@ -243,18 +243,18 @@ mod tests { let cost4: u64 = 130; // insert one record - testee.upsert(&key1, &cost1); + testee.upsert(&key1, cost1); assert_eq!(1, testee.get_count()); assert_eq!(&cost1, testee.get_cost(&key1).unwrap()); // insert 2nd record - testee.upsert(&key2, &cost2); + testee.upsert(&key2, cost2); assert_eq!(2, testee.get_count()); assert_eq!(&cost1, testee.get_cost(&key1).unwrap()); assert_eq!(&cost2, testee.get_cost(&key2).unwrap()); // insert 3rd record, pushes out the oldest (eg 1st) record - testee.upsert(&key3, &cost3); + testee.upsert(&key3, cost3); assert_eq!(2, testee.get_count()); assert_eq!((cost2 + cost3) / 2_u64, testee.get_average()); assert_eq!(cost3, testee.get_mode()); @@ -264,8 +264,8 @@ mod tests { // update 2nd record, so the 3rd becomes the oldest // add 4th record, pushes out 3rd key - testee.upsert(&key2, &cost1); - testee.upsert(&key4, &cost4); + testee.upsert(&key2, cost1); + testee.upsert(&key4, cost4); assert_eq!(((cost1 + cost2) / 2 + cost4) / 2_u64, testee.get_average()); assert_eq!((cost1 + cost2) / 2, testee.get_mode()); assert_eq!(2, testee.get_count()); diff --git a/core/src/progress_map.rs b/core/src/progress_map.rs index 6fe2b68b01..c20e129bd4 100644 --- a/core/src/progress_map.rs +++ b/core/src/progress_map.rs @@ -121,13 +121,13 @@ impl ReplaySlotStats { .per_program_timings .iter() .collect(); - per_pubkey_timings.sort_by(|a, b| b.1 .0.cmp(&a.1 .0)); - let total: u64 = per_pubkey_timings.iter().map(|a| a.1 .0).sum(); + per_pubkey_timings.sort_by(|a, b| b.1.accumulated_us.cmp(&a.1.accumulated_us)); + let total: u64 = per_pubkey_timings.iter().map(|a| a.1.accumulated_us).sum(); for (pubkey, time) in per_pubkey_timings.iter().take(5) { datapoint_info!( "per_program_timings", ("pubkey", pubkey.to_string(), String), - ("execute_us", time.0, i64) + ("execute_us", time.accumulated_us, i64) ); } datapoint_info!( diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index ce01e8ff13..258f1aaf11 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -56,6 +56,12 @@ impl Executors { } } +#[derive(Default, Debug)] +pub struct ProgramTiming { + pub accumulated_us: u64, + pub count: u32, +} + #[derive(Default, Debug)] pub struct ExecuteDetailsTimings { pub serialize_us: u64, @@ -66,7 +72,7 @@ pub struct ExecuteDetailsTimings { pub total_account_count: u64, pub total_data_size: usize, pub data_size_changed: usize, - pub per_program_timings: HashMap, + pub per_program_timings: HashMap, } impl ExecuteDetailsTimings { @@ -81,8 +87,8 @@ impl ExecuteDetailsTimings { self.data_size_changed += other.data_size_changed; for (id, other) in &other.per_program_timings { let time_count = self.per_program_timings.entry(*id).or_default(); - time_count.0 += other.0; - time_count.1 += other.1; + time_count.accumulated_us += other.accumulated_us; + time_count.count += other.count; } } } @@ -1244,8 +1250,8 @@ impl MessageProcessor { let program_id = instruction.program_id(&message.account_keys); let time_count = timings.per_program_timings.entry(*program_id).or_default(); - time_count.0 += time.as_us(); - time_count.1 += 1; + time_count.accumulated_us += time.as_us(); + time_count.count += 1; err?; }