make upsert infallible op

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)
+                .upsert_instruction_cost(program_id, units);
-            {
+            update_count += 1;
-                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
-                    );
-                }
-            }
         }
         debug!(
            "after replayed into bank, updated cost model instruction cost table, current values: {:?}",

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
+                self.upsert_instruction_cost(program_id, *cost);
-                    .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
-                        );
-                    }
-                }
             });
         debug!(
             "restored cost model instruction cost table from blockstore, current values: {:?}",
@@ -116,17 +100,9 @@ impl CostModel {
         tx_cost
     }
 
-    pub fn upsert_instruction_cost(
+    pub fn upsert_instruction_cost(&mut self, program_key: &Pubkey, cost: u64) {
-        &mut self,
-        program_key: &Pubkey,
-        cost: u64,
-    ) -> Result<u64, &'static str> {
         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<Pubkey, u64> {
@@ -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
+        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()));
 
         // unknown program is assigned with default cost
@@ -375,9 +349,7 @@ mod tests {
         let expected_cost = 8;
 
         let mut testee = CostModel::default();
-        testee
+        testee.upsert_instruction_cost(&system_program::id(), expected_cost);
-            .upsert_instruction_cost(&system_program::id(), expected_cost)
-            .unwrap();
         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
+        testee.upsert_instruction_cost(&system_program::id(), program_cost);
-            .upsert_instruction_cost(&system_program::id(), program_cost)
-            .unwrap();
         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
+        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.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());
+                        cost_model.upsert_instruction_cost(&prog1, cost1);
-                        assert!(cost_model.upsert_instruction_cost(&prog2, cost2).is_ok());
+                        cost_model.upsert_instruction_cost(&prog2, cost2);
                     })
                 } else {
                     thread::spawn(move || {

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<u64> {
+    // update-or-insert should be infallible. Query the result of upsert,
-        let need_to_add = self.table.get(key).is_none();
+    // 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);