TransactionCost - remove allocation

[apfitzge]

Problem

• TransactionCost holds an allocation to track writable accounts. This is generally not needed since we can always figure out which accounts the transaction writes since the transaction stays in scope longer than TransactionCost
• Doing the allocation and free is also bad for performance

Summary of Changes

• Remove writable_accounts from TransactionCost

Fixes #

[apfitzge]

not ideal place for this function to live. Will, hopefully, put this somewhere shared after this is resolved: #1918 (comment)

While this iteration is not as fast as simply iterating over a Vec; I do not think it will be so much slower that we reduce overall performance.

[apfitzge]

cost_tracker benches:

master:
test bench_cost_tracker_contentious_transaction     ... bench:   4,341,232 ns/iter (+/- 96,382)
test bench_cost_tracker_non_contentious_transaction ... bench:   6,784,462 ns/iter (+/- 142,771)

PR:
test bench_cost_tracker_contentious_transaction     ... bench:   3,542,738 ns/iter (+/- 52,597)
test bench_cost_tracker_non_contentious_transaction ... bench:   5,616,565 ns/iter (+/- 99,960)

[apfitzge]

This was incorrect. Lead to test_cost_tracker_ok_add_two_same_accounts failing when we used proper writable accounts count.

[apfitzge]

cost model benches (from #2168):

master:

test bench_cost_model                       ... bench:     988,185 ns/iter (+/- 33,633)
test bench_cost_model_requested_write_locks ... bench:   1,077,616 ns/iter (+/- 12,925)

PR:

test bench_cost_model                       ... bench:     818,381 ns/iter (+/- 20,589)
test bench_cost_model_requested_write_locks ... bench:     814,116 ns/iter (+/- 48,559)

More room for improvement here, this is slower than I'd like. But this PR is a step in the right direction.

[tao-stones]

overall the changes look great, removing the vec from TransactionCost is a plus. Going to look more closely; after first glance, feel having to passing writable_accounts: impl Iterator<Item = &'a Pubkey> to every cost-model functions is a bit of repetitive. Possible to to make it a member of TransactionCost? If not, maybe cost-model functions take f(&sanitized_transaction, &tx_cost) make more sense than f(write_lock_account_iter, &tx_cost). Just a quick thought tho.

[apfitzge]

Yeah I like that more than the iterator interface

[apfitzge]

Closing this PR for now. Other work going on in area, want to avoid causing rebase issues for this relatively minor improvement.