aptos-core
aptos-core copied to clipboard
[compiler-v2] Optimization for flushing writes eagerly
Description
This is a PR split off from https://github.com/aptos-labs/aptos-core/pull/14249. As such, you may notice that you are reviewing some of the same code, I apologize for this. I have taken care to address all the reviews left on relevant files in the original PR.
This PR contains two optimizations:
- Computing flushing writes hints: this is a stackless bytecode pass that computes which writes should be flushed right away (see the processor module documentation for more details) by the file format generator, because they known to be flushed later anyway.
- During file format generation, keeping track of which values on the stack were copied from locals, so that they do not have to be saved back to locals.
Type of Change
- [x] Performance improvement
Which Components or Systems Does This Change Impact?
- [x] Move/Aptos Virtual Machine
How Has This Been Tested?
- All existing tests pass, and when updating baselines, code generated is either better or same.
- See newly added tests under
tests/flush-writesfolder. If you see the.on.expvs..off.exp, it shows the difference between flush writes optimization being on vs. off (all cases are better or same).
Key Areas to Review
Correctness of the optimizations.
⏱️ 3h 18m total CI duration on this PR
| Job | Cumulative Duration | Recent Runs |
|---|---|---|
| execution-performance / single-node-performance | 21m | 🟩 |
| forge-compat-test / forge | 15m | 🟩 |
| forge-framework-upgrade-test / forge | 15m | 🟩 |
| rust-move-unit-coverage | 13m | 🟩 |
| forge-e2e-test / forge | 13m | 🟩 |
| rust-move-unit-coverage | 12m | 🟩 |
| rust-move-tests | 9m | 🟩 |
| rust-move-tests | 9m | 🟩 |
| rust-move-unit-coverage | 9m | 🟩 |
| rust-move-tests | 9m | 🟩 |
| rust-move-unit-coverage | 9m | 🟩 |
| general-lints | 9m | 🟩 🟩 🟩 🟩 🟩 |
| rust-move-tests | 9m | 🟩 |
| rust-cargo-deny | 9m | 🟩 🟩 🟩 🟩 🟩 |
| rust-move-tests | 8m | 🟩 |
| check-dynamic-deps | 6m | 🟩 🟩 🟩 🟩 🟩 |
| rust-doc-tests | 5m | 🟩 |
| execution-performance / test-target-determinator | 5m | 🟩 |
| test-target-determinator | 4m | 🟩 |
| check | 4m | 🟩 |
| semgrep/ci | 2m | 🟩 🟩 🟩 🟩 🟩 |
| file_change_determinator | 1m | 🟩 🟩 🟩 🟩 🟩 |
| file_change_determinator | 52s | 🟩 🟩 🟩 🟩 🟩 |
| permission-check | 20s | 🟩 🟩 🟩 🟩 🟩 |
| permission-check | 15s | 🟩 🟩 🟩 🟩 🟩 |
| permission-check | 14s | 🟩 🟩 🟩 🟩 🟩 |
| permission-check | 12s | 🟩 🟩 🟩 🟩 🟩 |
| file_change_determinator | 10s | 🟩 |
| Backport PR | 5s | 🟥 |
| permission-check | 2s | 🟩 |
| determine-docker-build-metadata | 2s | 🟩 |
| permission-check | 2s | 🟩 |
This stack of pull requests is managed by Graphite. Learn more about stacking.
Join @vineethk and the rest of your teammates on
Graphite
Codecov Report
Attention: Patch coverage is 98.15951% with 3 lines in your changes missing coverage. Please review.
Project coverage is 59.6%. Comparing base (
55ff034) to head (da57492). Report is 1 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| ...compiler-v2/src/pipeline/flush_writes_processor.rs | 97.5% | 2 Missing :warning: |
| ...v2/src/file_format_generator/function_generator.rs | 98.1% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #14394 +/- ##
===========================================
- Coverage 71.9% 59.6% -12.4%
===========================================
Files 2370 839 -1531
Lines 475533 205179 -270354
===========================================
- Hits 342111 122293 -219818
+ Misses 133422 82886 -50536
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Forge is running suite framework_upgrade on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f
- Grafana dashboard (auto-refresh)
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking
Forge is running suite realistic_env_max_load on da574928716a836a417639e3c4a9b2b5c08f0e3f
- Grafana dashboard (auto-refresh)
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking
Forge is running suite compat on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f
- Grafana dashboard (auto-refresh)
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite realistic_env_max_load success on da574928716a836a417639e3c4a9b2b5c08f0e3f
two traffics test: inner traffic : committed: 11629.41 txn/s, latency: 3417.06 ms, (p50: 3300 ms, p90: 3900 ms, p99: 5100 ms), latency samples: 4421760
two traffics test : committed: 99.89 txn/s, latency: 2927.68 ms, (p50: 2600 ms, p90: 3700 ms, p99: 11500 ms), latency samples: 1740
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.279, avg: 0.229", "QsPosToProposal: max: 0.678, avg: 0.497", "ConsensusProposalToOrdered: max: 0.350, avg: 0.338", "ConsensusOrderedToCommit: max: 0.829, avg: 0.717", "ConsensusProposalToCommit: max: 1.164, avg: 1.054"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.72s no progress at version 2403286 (avg 0.24s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 7.28s no progress at version 2403284 (avg 7.28s) [limit 15].
Test Ok
- Grafana dashboard
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite framework_upgrade success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f
Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f (PR)
Upgrade the nodes to version: da574928716a836a417639e3c4a9b2b5c08f0e3f
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1218.25 txn/s, submitted: 1223.36 txn/s, failed submission: 5.11 txn/s, expired: 5.11 txn/s, latency: 2555.87 ms, (p50: 2400 ms, p90: 3900 ms, p99: 5600 ms), latency samples: 109580
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1197.43 txn/s, submitted: 1199.75 txn/s, failed submission: 2.32 txn/s, expired: 2.32 txn/s, latency: 2705.44 ms, (p50: 2400 ms, p90: 4500 ms, p99: 5700 ms), latency samples: 103380
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f passed
Upgrade the remaining nodes to version: da574928716a836a417639e3c4a9b2b5c08f0e3f
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1160.53 txn/s, submitted: 1162.40 txn/s, failed submission: 1.88 txn/s, expired: 1.88 txn/s, latency: 2884.67 ms, (p50: 2600 ms, p90: 4900 ms, p99: 6000 ms), latency samples: 98920
Test Ok
- Grafana dashboard
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite compat success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f
Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f (PR)
1. Check liveness of validators at old version: d1bf834728a0cf166d993f4728dfca54f3086fb0
compatibility::simple-validator-upgrade::liveness-check : committed: 9569.57 txn/s, latency: 3245.49 ms, (p50: 2100 ms, p90: 9100 ms, p99: 16700 ms), latency samples: 349820
2. Upgrading first Validator to new version: da574928716a836a417639e3c4a9b2b5c08f0e3f
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7108.55 txn/s, latency: 3833.06 ms, (p50: 3800 ms, p90: 5500 ms, p99: 5800 ms), latency samples: 132460
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7076.21 txn/s, latency: 4462.47 ms, (p50: 4500 ms, p90: 6600 ms, p99: 6700 ms), latency samples: 240140
3. Upgrading rest of first batch to new version: da574928716a836a417639e3c4a9b2b5c08f0e3f
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7517.24 txn/s, latency: 3731.21 ms, (p50: 4200 ms, p90: 4700 ms, p99: 4800 ms), latency samples: 139820
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7372.11 txn/s, latency: 4347.36 ms, (p50: 4600 ms, p90: 5300 ms, p99: 5700 ms), latency samples: 245740
4. upgrading second batch to new version: da574928716a836a417639e3c4a9b2b5c08f0e3f
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 9227.01 txn/s, latency: 2956.78 ms, (p50: 2700 ms, p90: 5700 ms, p99: 6300 ms), latency samples: 167120
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8864.25 txn/s, latency: 3427.51 ms, (p50: 2900 ms, p90: 6700 ms, p99: 9100 ms), latency samples: 306860
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f passed
Test Ok
- Grafana dashboard
- Humio Logs
- Axiom Logs
- Validator CPU Profile
- Fullnode CPU Profile
- Test runner output
- Test run is land-blocking