-
Notifications
You must be signed in to change notification settings - Fork 152
Open
Labels
Description
Describe the feature
Problem
In [build_subblock] we snapshot candidate transactions with transactions.lock().clone().
tempo/crates/commonware-node/src/subblocks.rs
Line 698 in 7f34c56
| let txs = transactions.lock().clone(); |
IndexMap, including its internal buckets/indices, even though the build step only needs to iterate the current entries. With large candidate sets (up to MAX_SUBBLOCK_TXS), this creates avoidable allocations and memcpy pressure on a hot path.
Why it matters
- Cloning
IndexMapcopies bucket/index structures in addition to keys/values, which is more expensive than a flat snapshot of the data we actually need. - This path is executed whenever we (re)build a subblock; extra allocations grow with candidate size.
- We can keep the same lock granularity (short lock for snapshot, no lock during EVM execution) while reducing memory work.
Where
crates/commonware-node/src/subblocks.rs
build_subblock around line ~698:
let txs = transactions.lock().clone();(snapshot)for (tx_hash, tx) in txs { ... }
Suggested direction
- Replace the
IndexMap::clone()snapshot with a lightweightVecsnapshot:Vec<(TxHash, Arc<Recovered<TempoTxEnvelope>>)>viaiter()+Arc::clone()- Keep the lock only for snapshot creation, as today.
- Optional: collect invalid
TxHashvalues during iteration and remove them in one locked pass to cut repeated lock/unlock churn. - Keep current semantics: no lock held during
evm.transact_commit, still allow removal of invalid transactions by hash.
Additional context
No response