Aggregate cost_model into cost_tracker

[tao-stones]

Problem

Previously banking_stage has both cost_model and cost_tracker, there are times banking_stage needs to acquire lock on both objects in order to calculated transaction cost from cost_model and update cost_tracker. This could potentially lead to deadlock if future code is not careful on the order of lock acquiring.

Furthermore, banking_stage does not have to access cost_model directly, it only needs to access cost_tracker which in turn can aggregate cost_model. Making this change also paves the way to move cost tracking to its own service module, therefore offload it from banking main thread to its own thread.

Summary of Changes

• aggregate cost_model into cost_tracker, decouple it form banking_stage
• removed un-used functions
• CostModel::calculate_cost(&tx) returns &TransactionCost to avoid unnecessary heap allocation.

Fixes #

[codecov]

Codecov Report

Merging #18374 (a804280) into master (89a3e4f) will decrease coverage by 0.0%.
The diff coverage is 95.9%.

@@            Coverage Diff            @@
##           master   #18374     +/-   ##
=========================================
- Coverage    82.3%    82.3%   -0.1%     
=========================================
  Files         435      435             
  Lines      121424   121356     -68     
=========================================
- Hits       100020    99927     -93     
- Misses      21404    21429     +25     

[t-nelson]

r+. Just a lock resolution question then this is ready to go by my eye. I like the green/red ratio 😉

[t-nelson]

Seems like we don't need to hold the lock for the duration of this method?

let tx_cost = self
    .cost_model
    .write()
    .unwrap()
    .calculate_cost(transaction);

add_transaction_cost() below looks like it can take a similar treatment

[tao-stones]

Right, this is a bit tricky because cost_model has this signature for calculate_cost:

    pub fn calculate_cost(&mut self, transaction: &Transaction) -> &TransactionCost {

It reuses a preallocated TransactionCost, and returns calculation result as a ref to it. The motivation is calculate_cost is called for every transaction, therefore it is in hot path. It saves noticeable overhead by avoiding allocating TransactionCost at heap for each call. But this requires the lock to live as long as tx_cost. Otherwise Rust complains:

error[E0716]: temporary value dropped while borrowed
  --> core/src/cost_tracker.rs:47:23
   |
47 |         let tx_cost = self.cost_model.write().unwrap().calculate_cost(transaction);
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                            - temporary value is freed at the end of this statement
   |                       |
   |                       creates a temporary which is freed while still in use
48 |         self.would_fit(
49 |             &tx_cost.writable_accounts,
   |             -------------------------- borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

I compared the execution timing between two approaches, reuse heap object and holding lock a bit longer had much better performance.
I am glad you pointed it out, maybe I over complicated it, and there is a better way to avoid many head alloc.

Pull request has been modified.