aptos-core icon indicating copy to clipboard operation
aptos-core copied to clipboard

[compiler-v2] Optimization for flushing writes eagerly

Open vineethk opened this issue 1 year ago • 3 comments

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:

  1. 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.
  2. 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-writes folder. If you see the .on.exp vs. .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.

vineethk avatar Aug 23 '24 02:08 vineethk

⏱️ 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 🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

trunk-io[bot] avatar Aug 23 '24 02:08 trunk-io[bot]

  • #14394 Graphite 👈
  • main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vineethk and the rest of your teammates on Graphite Graphite

vineethk avatar Aug 23 '24 02:08 vineethk

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.

codecov[bot] avatar Aug 23 '24 02:08 codecov[bot]

Forge is running suite framework_upgrade on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f

github-actions[bot] avatar Aug 23 '24 15:08 github-actions[bot]

Forge is running suite realistic_env_max_load on da574928716a836a417639e3c4a9b2b5c08f0e3f

github-actions[bot] avatar Aug 23 '24 15:08 github-actions[bot]

Forge is running suite compat on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> da574928716a836a417639e3c4a9b2b5c08f0e3f

github-actions[bot] avatar Aug 23 '24 15:08 github-actions[bot]

: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

github-actions[bot] avatar Aug 23 '24 16:08 github-actions[bot]

: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

github-actions[bot] avatar Aug 23 '24 16:08 github-actions[bot]

: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

github-actions[bot] avatar Aug 23 '24 16:08 github-actions[bot]