diff --git a/core/src/cost_update_service.rs b/core/src/cost_update_service.rs index 41740962db..ab2e3b46fb 100644 --- a/core/src/cost_update_service.rs +++ b/core/src/cost_update_service.rs @@ -128,25 +128,11 @@ impl CostUpdateService { } let units = program_timings.accumulated_units / program_timings.count as u64; - match cost_model + cost_model .write() .unwrap() - .upsert_instruction_cost(program_id, units) - { - Ok(c) => { - debug!( - "after replayed into bank, instruction {:?} has averaged cost {}", - program_id, c - ); - update_count += 1; - } - Err(err) => { - debug!( - "after replayed into bank, instruction {:?} failed to update cost, err: {}", - program_id, err - ); - } - } + .upsert_instruction_cost(program_id, units); + update_count += 1; } debug!( "after replayed into bank, updated cost model instruction cost table, current values: {:?}", diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index cd04a22732..8934e7b7f2 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -79,23 +79,7 @@ impl CostModel { .map(|(key, cost)| (key, cost)) .chain(BUILT_IN_INSTRUCTION_COSTS.iter()) .for_each(|(program_id, cost)| { - match self - .instruction_execution_cost_table - .upsert(program_id, *cost) - { - Some(c) => { - debug!( - "initiating cost table, instruction {:?} has cost {}", - program_id, c - ); - } - None => { - debug!( - "initiating cost table, failed for instruction {:?}", - program_id - ); - } - } + self.upsert_instruction_cost(program_id, *cost); }); debug!( "restored cost model instruction cost table from blockstore, current values: {:?}", @@ -116,17 +100,9 @@ impl CostModel { tx_cost } - pub fn upsert_instruction_cost( - &mut self, - program_key: &Pubkey, - cost: u64, - ) -> Result { + pub fn upsert_instruction_cost(&mut self, program_key: &Pubkey, cost: u64) { self.instruction_execution_cost_table .upsert(program_key, cost); - match self.instruction_execution_cost_table.get_cost(program_key) { - Some(cost) => Ok(*cost), - None => Err("failed to upsert to ExecuteCostTable"), - } } pub fn get_instruction_cost_table(&self) -> &HashMap { @@ -293,13 +269,11 @@ 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); // find cost for known programs assert_eq!(100, testee.find_instruction_cost(&known_key)); - testee - .upsert_instruction_cost(&bpf_loader::id(), 1999) - .unwrap(); + testee.upsert_instruction_cost(&bpf_loader::id(), 1999); assert_eq!(1999, testee.find_instruction_cost(&bpf_loader::id())); // unknown program is assigned with default cost @@ -375,9 +349,7 @@ mod tests { let expected_cost = 8; let mut testee = CostModel::default(); - testee - .upsert_instruction_cost(&system_program::id(), expected_cost) - .unwrap(); + testee.upsert_instruction_cost(&system_program::id(), expected_cost); assert_eq!( expected_cost, testee.get_transaction_cost(&simple_transaction) @@ -405,9 +377,7 @@ mod tests { let expected_cost = program_cost * 2; let mut testee = CostModel::default(); - testee - .upsert_instruction_cost(&system_program::id(), program_cost) - .unwrap(); + testee.upsert_instruction_cost(&system_program::id(), program_cost); assert_eq!(expected_cost, testee.get_transaction_cost(&tx)); } @@ -488,9 +458,9 @@ mod tests { ); // insert instruction cost to table - assert!(cost_model.upsert_instruction_cost(&key1, cost1).is_ok()); + cost_model.upsert_instruction_cost(&key1, cost1); - // now it is known insturction with known cost + // now it is known instruction with known cost assert_eq!(cost1, cost_model.find_instruction_cost(&key1)); } @@ -508,9 +478,7 @@ mod tests { let expected_execution_cost = 8; let mut cost_model = CostModel::default(); - cost_model - .upsert_instruction_cost(&system_program::id(), expected_execution_cost) - .unwrap(); + cost_model.upsert_instruction_cost(&system_program::id(), expected_execution_cost); let tx_cost = cost_model.calculate_cost(&tx); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); assert_eq!(expected_execution_cost, tx_cost.execution_cost); @@ -527,11 +495,11 @@ mod tests { let mut cost_model = CostModel::default(); // insert instruction cost to table - assert!(cost_model.upsert_instruction_cost(&key1, cost1).is_ok()); + cost_model.upsert_instruction_cost(&key1, cost1); assert_eq!(cost1, cost_model.find_instruction_cost(&key1)); // update instruction cost - assert!(cost_model.upsert_instruction_cost(&key1, cost2).is_ok()); + cost_model.upsert_instruction_cost(&key1, cost2); assert_eq!(updated_cost, cost_model.find_instruction_cost(&key1)); } @@ -573,8 +541,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()); + cost_model.upsert_instruction_cost(&prog1, cost1); + cost_model.upsert_instruction_cost(&prog2, cost2); }) } else { thread::spawn(move || { diff --git a/runtime/src/execute_cost_table.rs b/runtime/src/execute_cost_table.rs index c1ef3449cb..8c85315234 100644 --- a/runtime/src/execute_cost_table.rs +++ b/runtime/src/execute_cost_table.rs @@ -77,8 +77,10 @@ impl ExecuteCostTable { self.table.get(key) } - pub fn upsert(&mut self, key: &Pubkey, value: u64) -> Option { - let need_to_add = self.table.get(key).is_none(); + // update-or-insert should be infallible. Query the result of upsert, + // often requires additional calculation, should be lazy. + pub fn upsert(&mut self, key: &Pubkey, value: u64) { + let need_to_add = !self.table.contains_key(key); 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)); @@ -93,8 +95,6 @@ impl ExecuteCostTable { .or_insert((0, Self::micros_since_epoch())); *count += 1; *timestamp = Self::micros_since_epoch(); - - Some(*program_cost) } // prune the old programs so the table contains `new_size` of records, @@ -184,9 +184,9 @@ mod tests { let key2 = Pubkey::new_unique(); let key3 = Pubkey::new_unique(); - // simulate a lot of occurences to key1, so even there're longer than + // simulate a lot of occurrences to key1, so even there're longer than // usual delay between upsert(key1..) and upsert(key2, ..), test - // would still satisfy as key1 has enough occurences to compensate + // would still satisfy as key1 has enough occurrences to compensate // its age. for i in 0..1000 { testee.upsert(&key1, i);