[aptos-framework] Fix needless mutable references and borrows #14656

vineethk · 2024-09-17T01:36:09Z

Description

In this PR, we fix move lint violations in the aptos framework for the lint rule introduced here: #14651

In essence, any &mut and borrow_global_mut that do not need to be mutable, and can be replaced by & and borrow_global respectively, are found and suggested by the new lint checker. This PR is a result of applying those suggestions.

Type of Change

Refactoring (fix lint violations)

Which Components or Systems Does This Change Impact?

Aptos Framework

How Has This Been Tested?

Existing aptos framework tests were run using both compiler v1 and v2.

Key Areas to Review

Any change that does not preserve the intended semantics of the original code.

trunk-io · 2024-09-17T01:36:12Z

⏱️ 6h 37m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	50m	🟩 🟩
forge-compat-test / forge	39m	🟩 🟩
forge-framework-upgrade-test / forge	35m	🟩 🟩
forge-e2e-test / forge	33m	🟥 🟩
rust-move-tests	29m	🟥 🟥 🟥
rust-move-unit-coverage	21m	🟩
rust-move-unit-coverage	20m	🟩
rust-move-unit-coverage	15m	🟩
rust-move-unit-coverage	14m	🟩
rust-move-unit-coverage	14m	🟩
general-lints	13m	🟩 🟩 🟩 🟩 🟩 (+2 more)
rust-cargo-deny	12m	🟩 🟩 🟩 🟩 🟩 (+2 more)
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
test-target-determinator	9m	🟩 🟩
execution-performance / test-target-determinator	9m	🟩 🟩
rust-move-tests	9m	🟩
check-dynamic-deps	8m	🟩 🟩 🟩 🟩 🟩 (+2 more)
check	7m	🟩 🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
rust-move-tests	3m	⬜
rust-move-unit-coverage	3m	⬜
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	27s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	22s	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	19s	🟩 🟩
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	6s	🟩 🟩
Backport PR	5s	🟥
determine-docker-build-metadata	5s	🟩 🟩
permission-check	2s	🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
execution-performance / single-node-performance	24m	18m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

vineethk · 2024-09-17T01:36:23Z

[aptos-framework] Fix needless mutable references and borrows #14656 👈
[compiler-v2][lint] Needless mutable reference checker #14651
main

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

Join @vineethk and the rest of your teammates on Graphite

codecov · 2024-09-17T01:49:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.8%. Comparing base (f5134a5) to head (e28a93f).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14656   +/-   ##
=======================================
  Coverage    59.8%    59.8%           
=======================================
  Files         851      851           
  Lines      207415   207415           
=======================================
  Hits       124184   124184           
  Misses      83231    83231

Flag	Coverage Δ
	`59.8% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-09-20T14:39:05Z

✅ Forge suite `realistic_env_max_load` success on `e28a93ff814925b8f9546a4b678c1b30c609501c`

two traffics test: inner traffic : committed: 14248.71 txn/s, latency: 2786.06 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3300 ms), latency samples: 5417680
two traffics test : committed: 99.99 txn/s, latency: 1655.41 ms, (p50: 1500 ms, p70: 1600, p90: 1700 ms, p99: 7000 ms), latency samples: 1720
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.266, avg: 0.235", "QsPosToProposal: max: 1.102, avg: 1.073", "ConsensusProposalToOrdered: max: 0.328, avg: 0.297", "ConsensusOrderedToCommit: max: 0.429, avg: 0.408", "ConsensusProposalToCommit: max: 0.723, avg: 0.705"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.82s no progress at version 2275481 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.61s no progress at version 2275479 (avg 8.61s) [limit 15].
Test Ok

github-actions · 2024-09-20T14:40:32Z

✅ Forge suite `framework_upgrade` success on `25a081116546670e62ca927ba90478de78557056` ==> `e28a93ff814925b8f9546a4b678c1b30c609501c`

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c (PR)
Upgrade the nodes to version: e28a93ff814925b8f9546a4b678c1b30c609501c
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1072.04 txn/s, submitted: 1076.51 txn/s, failed submission: 4.46 txn/s, expired: 4.46 txn/s, latency: 3033.59 ms, (p50: 2400 ms, p70: 3300, p90: 5600 ms, p99: 6900 ms), latency samples: 96100
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1036.22 txn/s, submitted: 1038.66 txn/s, failed submission: 2.44 txn/s, expired: 2.44 txn/s, latency: 2869.55 ms, (p50: 2400 ms, p70: 3000, p90: 5100 ms, p99: 6700 ms), latency samples: 93400
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c passed
Upgrade the remaining nodes to version: e28a93ff814925b8f9546a4b678c1b30c609501c
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1094.65 txn/s, submitted: 1097.93 txn/s, failed submission: 3.28 txn/s, expired: 3.28 txn/s, latency: 2632.61 ms, (p50: 2400 ms, p70: 2700, p90: 4500 ms, p99: 6300 ms), latency samples: 100160
Test Ok

github-actions · 2024-09-20T14:44:51Z

✅ Forge suite `compat` success on `25a081116546670e62ca927ba90478de78557056` ==> `e28a93ff814925b8f9546a4b678c1b30c609501c`

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c (PR)
1. Check liveness of validators at old version: 25a081116546670e62ca927ba90478de78557056
compatibility::simple-validator-upgrade::liveness-check : committed: 14603.00 txn/s, latency: 2217.71 ms, (p50: 1800 ms, p70: 1900, p90: 3500 ms, p99: 14800 ms), latency samples: 502000
2. Upgrading first Validator to new version: e28a93ff814925b8f9546a4b678c1b30c609501c
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7923.49 txn/s, latency: 3534.84 ms, (p50: 3700 ms, p70: 4100, p90: 4200 ms, p99: 4300 ms), latency samples: 145020
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7848.70 txn/s, latency: 4068.72 ms, (p50: 4200 ms, p70: 4300, p90: 6000 ms, p99: 6200 ms), latency samples: 258880
3. Upgrading rest of first batch to new version: e28a93ff814925b8f9546a4b678c1b30c609501c
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7878.75 txn/s, latency: 3643.24 ms, (p50: 4100 ms, p70: 4200, p90: 4300 ms, p99: 4500 ms), latency samples: 145720
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7898.36 txn/s, latency: 4048.73 ms, (p50: 4400 ms, p70: 4400, p90: 4900 ms, p99: 5300 ms), latency samples: 262660
4. upgrading second batch to new version: e28a93ff814925b8f9546a4b678c1b30c609501c
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 9366.47 txn/s, latency: 2858.03 ms, (p50: 2600 ms, p70: 3400, p90: 4200 ms, p99: 5400 ms), latency samples: 187360
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 9461.90 txn/s, latency: 3208.77 ms, (p50: 2600 ms, p70: 3400, p90: 6100 ms, p99: 8300 ms), latency samples: 313980
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c passed
Test Ok

vineethk mentioned this pull request Sep 17, 2024

[compiler-v2][lint] Needless mutable reference checker #14651

Merged

2 tasks

vineethk force-pushed the vk/aptos-framework/needless-mut-refs branch from 11b5882 to 4be5108 Compare September 17, 2024 01:39

vineethk marked this pull request as ready for review September 17, 2024 01:45

vineethk requested review from zjma, alinush, davidiw, movekevin and wrwg as code owners September 17, 2024 01:45

vineethk requested review from rahxephon89, brmataptos, lightmark and rtso September 17, 2024 01:45

rahxephon89 approved these changes Sep 17, 2024

View reviewed changes

brmataptos approved these changes Sep 17, 2024

View reviewed changes

vineethk force-pushed the vk/needless-mut-ref branch from 981068f to f778db8 Compare September 18, 2024 19:22

vineethk force-pushed the vk/aptos-framework/needless-mut-refs branch from 4be5108 to f2301f9 Compare September 18, 2024 19:23

vineethk force-pushed the vk/needless-mut-ref branch 2 times, most recently from 9ed494c to e39b231 Compare September 19, 2024 14:02

Base automatically changed from vk/needless-mut-ref to main September 19, 2024 14:40

[aptos-framework] Fix needless mutable references and borrows

f645ed6

vineethk force-pushed the vk/aptos-framework/needless-mut-refs branch from f2301f9 to f645ed6 Compare September 19, 2024 19:20

Merge branch 'main' into vk/aptos-framework/needless-mut-refs

da35ba3

vineethk enabled auto-merge (squash) September 20, 2024 00:59

This comment has been minimized.

Sign in to view

Merge branch 'main' into vk/aptos-framework/needless-mut-refs

e28a93f

This comment has been minimized.

Sign in to view

vineethk merged commit ce6158a into main Sep 20, 2024
51 checks passed

vineethk deleted the vk/aptos-framework/needless-mut-refs branch September 20, 2024 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[aptos-framework] Fix needless mutable references and borrows #14656

[aptos-framework] Fix needless mutable references and borrows #14656

vineethk commented Sep 17, 2024 •

edited

Loading

trunk-io bot commented Sep 17, 2024 •

edited

Loading

vineethk commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 20, 2024

github-actions bot commented Sep 20, 2024

github-actions bot commented Sep 20, 2024

[aptos-framework] Fix needless mutable references and borrows #14656

[aptos-framework] Fix needless mutable references and borrows #14656

Conversation

vineethk commented Sep 17, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

trunk-io bot commented Sep 17, 2024 • edited Loading

vineethk commented Sep 17, 2024 • edited Loading

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 20, 2024

✅ Forge suite realistic_env_max_load success on e28a93ff814925b8f9546a4b678c1b30c609501c

github-actions bot commented Sep 20, 2024

✅ Forge suite framework_upgrade success on 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c

github-actions bot commented Sep 20, 2024

✅ Forge suite compat success on 25a081116546670e62ca927ba90478de78557056 ==> e28a93ff814925b8f9546a4b678c1b30c609501c

vineethk commented Sep 17, 2024 •

edited

Loading

trunk-io bot commented Sep 17, 2024 •

edited

Loading

vineethk commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `e28a93ff814925b8f9546a4b678c1b30c609501c`

✅ Forge suite `framework_upgrade` success on `25a081116546670e62ca927ba90478de78557056` ==> `e28a93ff814925b8f9546a4b678c1b30c609501c`

✅ Forge suite `compat` success on `25a081116546670e62ca927ba90478de78557056` ==> `e28a93ff814925b8f9546a4b678c1b30c609501c`