Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-stdlib] Use vector::move_range inside vector, and evaluate performance / calibrate gas #14862

Open
wants to merge 12 commits into
base: igor/constant_serialized_size
Choose a base branch
from

Conversation

igor-aptos
Copy link
Contributor

@igor-aptos igor-aptos commented Oct 3, 2024

Description

Use vector::move_range inside of vector, to optimize insert, remove, append, trim.
Extend aptos-move/e2e-benchmark/src/main.rs to track gas and gas/s, to allow for quick calibration.
Adding workloads to txn-emitter to be able to use it throughput.

Additionally add a missing replace method, which replaces value at particular index.

Running on extended set of params:
https://gist.github.com/igor-aptos/e8d4e21edcbc75dddcb9382d4e077665

Summary of the performance tests:

  • performance doesn't depend on the size of the values (unless they are primitive), as vector modifies only pointers to them.
  • operation depends very little on how many elements we need to move - moving 1000 elements (i.e. to insert into a vector 1000 elements from the end) is only (!!) 2x slower than moving 1 element, end-to-end.

For gas calibration, on the variety of workloads, current implementation has decent variance. After tuning params to match the averages, variance seems much smaller.

How Has This Been Tested?

Key Areas to Review

Type of Change

  • Performance improvement

Which Components or Systems Does This Change Impact?

  • Aptos Framework

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Copy link

trunk-io bot commented Oct 3, 2024

⏱️ 8h 45m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
execution-performance / single-node-performance 4h 8m 🟥🟥🟥🟥 (+2 more)
execution-performance / test-target-determinator 25m 🟩🟩🟩🟩🟩 (+2 more)
test-target-determinator 23m 🟩🟩🟩🟩🟩 (+2 more)
rust-move-unit-coverage 16m 🟩
rust-move-unit-coverage 16m 🟩
rust-move-unit-coverage 15m 🟩
check-dynamic-deps 15m 🟩🟩🟩🟩🟩 (+4 more)
rust-cargo-deny 14m 🟩🟩🟩🟩🟩 (+3 more)
rust-move-unit-coverage 11m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-tests 10m 🟥
rust-move-tests 10m 🟥
rust-move-tests 10m 🟩
rust-move-unit-coverage 10m 🟩

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

Job Duration vs 7d avg Delta
execution-performance / single-node-performance 36m 16m +121%

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (igor/native_vector_move_range@b36187b). Learn more about missing BASE report.

Additional details and impacted files
@@                       Coverage Diff                        @@
##             igor/native_vector_move_range   #14862   +/-   ##
================================================================
  Coverage                                 ?    60.1%           
================================================================
  Files                                    ?      858           
  Lines                                    ?   211455           
  Branches                                 ?        0           
================================================================
  Hits                                     ?   127237           
  Misses                                   ?    84218           
  Partials                                 ?        0           

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

@igor-aptos igor-aptos force-pushed the igor/native_vector_move_range branch from 75ebf46 to d724063 Compare October 4, 2024 16:59
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 0df908c to 0fab436 Compare October 4, 2024 17:00
@igor-aptos igor-aptos added CICD:run-execution-performance-test Run execution performance test CICD:run-execution-performance-full-test Run execution performance test (full version) labels Oct 4, 2024
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 0fab436 to b1a2e70 Compare October 4, 2024 22:36
@igor-aptos igor-aptos force-pushed the igor/native_vector_move_range branch from d724063 to e4540db Compare October 8, 2024 19:00
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from b1a2e70 to 6b493cd Compare October 8, 2024 19:00
@igor-aptos igor-aptos force-pushed the igor/native_vector_move_range branch from e4540db to ae8e817 Compare October 9, 2024 20:30
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 6b493cd to 6046016 Compare October 9, 2024 20:30
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 6046016 to c5e50b3 Compare October 10, 2024 00:08
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from c5e50b3 to 36f8618 Compare October 10, 2024 18:10
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 36f8618 to 9851f51 Compare October 15, 2024 19:48
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 9851f51 to 3bcedb1 Compare October 16, 2024 21:07
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 3bcedb1 to a5af4dc Compare October 16, 2024 21:17
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from a5af4dc to 9fcf365 Compare October 21, 2024 23:10
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 9fcf365 to 53920f9 Compare November 8, 2024 23:25
@igor-aptos igor-aptos changed the base branch from igor/native_vector_move_range to igor/constant_serialized_size November 8, 2024 23:26
@igor-aptos igor-aptos force-pushed the igor/use_native_vector_move_range branch from 53920f9 to 58c55c0 Compare November 9, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:run-execution-performance-full-test Run execution performance test (full version) CICD:run-execution-performance-test Run execution performance test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant