[Parallel Executor] Rolling Commit

[danielxiangzl]

This PR implements the methodology called rolling commit to track the prefix of committed transaction in the parallel execution accurately, without introducing much overhead compared to the current lazy commit approach which can only commit all the transactions together.

Here are some p2p benchmark numbers compared to the previous commit method (lazy commit of the whole block) on my MBP with 10 cores/threads. The time unit is milliseconds. Note that the numbers here can be noisy and better experiments can be done if needed. Two versions of rolling commit protect txn status with Mutex and RwLock, respectively.

(# account, # txns)	(100, 1k)	(100, 10k)	(100, 50k)	(1k, 1k)	(1k, 10k)	(1k, 50k)
Lazy cmt	57	301	1318	52	257	1145
Rolling cmt w. Mutex	60	327	1480	55	299	1284
Rolling cmt w. RwLock	52	284	1271	50	249	1098
PR #5948	60	335	1525	56	314	1412

UPDATE
p2p benchmark numbers compared to the previous commit method (lazy commit of the whole block) on AWS ubuntu c5a.16xlarge instance. The time unit is milliseconds.

With 8 threads

(# account, # txns)	(100, 1k)	(100, 10k)	(1k, 1k)	(1k, 10k)
Lazy cmt	81	562	76	496
Rolling cmt	82	558	75	494

With 16 threads

(# account, # txns)	(100, 1k)	(100, 10k)	(1k, 1k)	(1k, 10k)
Lazy cmt	64	375	57	291
Rolling cmt	64	372	57	289

Broader Context
Rolling commit is needed for per-block limit & aggregator. We have a dedicated-thread solution implemented in this PR #5948 (which has additional validation overhead), but this is an attempt to remove the trade-offs and get the rolling commit almost for free. The idea is to track the waves of validations, and know when the last one happens.

[gelash]

Awesome!

Before a full review, a few points we may want to consider before landing:

• whether to yield (std::hint) and when
• can committing thread when there are no tasks starve other threads out of locks for non negligible time (fairness)
• try_lock (but likely why not)
• can required_wave be simplified or removed (I think not)

Otherwise, as noted, let's try to add a few extra unit / proptests that stress the committing logic

[gelash]

Let's make sure try_commit does not starve other threads.
We can use parking lot rwlock for execution and validation statuses - please adjust the uses based on whether we only require a read, or whether we also update the respective statuses.
Another way to use it is to grab read lock (upgradable) first, and then upgrade it if writing becomes necessary, I suggested this flow for the try_commit execution status, as an example, not sure if it's useful elsewhere.

We can then also revert changes to mutex (no need to wrap try_lock, we won't be using it on aptos infallable).

[gelash]

Let's make sure try_commit does not starve other threads. We can use parking lot rwlock for execution and validation statuses - please adjust the uses based on whether we only require a read, or whether we also update the respective statuses. Another way to use it is to grab read lock (upgradable) first, and then upgrade it if writing becomes necessary, I suggested this flow for the try_commit execution status, as an example, not sure if it's useful elsewhere.

We can then also revert changes to mutex (no need to wrap try_lock, we won't be using it on aptos infallable).

and let's regenerate those numbers make sure we don't lose performance somehow (we shouldn't)

[zekun000]

beautiful

[zekun000]

should we change this to u32? it seems cleaner for the validator index use

[danielxiangzl]

Since TxnIndex is also used in many other places other than validation_idx, I will keep the current one.

[gelash]

I am merging the definitions in my diff (MVHashMap redefined and a TODO anyway), and I can try to redefine it there.

[sasha8]

Nit: similar to Zekun's comment, .0 and .1 are not very readable.

[danielxiangzl]

Added comments.

[github-actions]

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 15c7a147ac567f39763553b12e9fdeee7e43eabf

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 15c7a147ac567f39763553b12e9fdeee7e43eabf (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 8060 TPS, 4738 ms latency, 6900 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 15c7a147ac567f39763553b12e9fdeee7e43eabf
compatibility::simple-validator-upgrade::single-validator-upgrade : 4604 TPS, 8958 ms latency, 11800 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 15c7a147ac567f39763553b12e9fdeee7e43eabf
compatibility::simple-validator-upgrade::half-validator-upgrade : 4638 TPS, 8773 ms latency, 11500 ms p99 latency,no expired txns
4. upgrading second batch to new version: 15c7a147ac567f39763553b12e9fdeee7e43eabf
compatibility::simple-validator-upgrade::rest-validator-upgrade : 7069 TPS, 5342 ms latency, 9200 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 15c7a147ac567f39763553b12e9fdeee7e43eabf passed
Test Ok

• Grafana dashboard
• Humio Logs
• Pyroscope Profiling
• Test runner output
• Test run is land-blocking

[github-actions]

✅ Forge suite land_blocking success on 15c7a147ac567f39763553b12e9fdeee7e43eabf

performance benchmark with full nodes : 6260 TPS, 6315 ms latency, 11100 ms p99 latency,(!) expired 600 out of 2673880 txns
Test Ok

• Grafana dashboard
• Humio Logs
• Pyroscope Profiling
• Test runner output
• Test run is land-blocking