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.

Sign up for GitHub

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

Jump to bottom

enable write io gas for transaction and events #13008

Merged
merged 1 commit into from Apr 24, 2024
Merged

enable write io gas for transaction and events #13008

merged 1 commit into from Apr 24, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Apr 24, 2024 •
edited

Description

Enabling io gas for transaction itself and events, implemented here #12770

the impact on commonly used txns are minimal:

Transfer 7 7 0.0%
2ndTransfer 7 7 0.0%
CreateAccount 993 993 0.0%
CreateTransfer 995 995 0.0%
MintTokenV2 633 633 0.0%
MutateTokenV2 8 8 0.0%
MintLargeTokenV2 1452 1452 0.0%
MutateLargeTokenV2 11 12 $${ \color{red} 9.1 \% }$$
CreateStakePool 2350 2350 0.0%
RotateConsensusKey 66 66 0.0%
JoinValidator100 53 53 0.0%
AddStake 15 15 0.0%
UnlockStake 40 40 0.0%
WithdrawStake 10 10 0.0%
LeaveValidatorSet100 12 12 0.0%
CreateCollection 962 962 0.0%
CreateTokenFirstTime 1489 1489 0.0%
MintTokenV1 8 8 0.0%
MutateTokenV1 483 483 0.0%
MutateTokenV12ndTime 8 8 0.0%
MutateTokenAdd10NewProperties 65 65 0.0%
MutateTokenMutate10ExistingProperties 14 14 0.0%
PublishSmall 1127 1127 0.0%
UpgradeSmall 75 75 0.0%
PublishLarge 11061 11063 0.0%
UpgradeLarge 1451 1453 $${ \color{green} 0.1 \% }$$
PublishDependencyChain-1 533 533 0.0%
PublishDependencyChain-2 558 558 0.0%
PublishDependencyChain-3 573 573 0.0%
UseDependencyChain-1 3 3 0.0%
UseDependencyChain-2 3 3 0.0%
UseDependencyChain-3 4 4 0.0%

Type of Change

  • New feature

Which Components or Systems Does This Change Impact?

  • Aptos Framework

How Has This Been Tested?

test_gas

Key Areas to Review

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

@trunk-io Trunk.io
Copy link

trunk-io bot commented Apr 24, 2024 •
edited

⏱️ 4h total CI duration on this PR
Job Cumulative Duration Recent Runs
execution-performance / single-node-performance 47m 🟥 🟩 
rust-targeted-unit-tests 39m 🟩 🟩 
rust-smoke-tests 39m 🟩 
rust-move-tests 22m 🟩 🟩 
cli-e2e-tests / run-cli-tests 16m 🟥 🟩 
forge-e2e-test / forge 14m 🟩 
forge-compat-test / forge 13m 🟩 
rust-images / rust-all 12m 🟩 
rust-lints 11m 🟩 🟩 
run-tests-main-branch 8m 🟩 🟩 
check 4m 🟩 
general-lints 4m 🟩 🟩 
rust-build-cached-packages 4m 🟩 
check-dynamic-deps 3m 🟩 🟩 
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 2m 🟩 
node-api-compatibility-tests / node-api-compatibility-tests 51s 🟩 
semgrep/ci 25s 🟩 
file_change_determinator 10s 🟩 

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

Job Duration vs 7d avg Delta
rust-lints 5m 7m -23%

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@msmouse msmouse requested review from vgao1996, igor-aptos and a team April 24, 2024 17:18
@msmouse msmouse marked this pull request as ready for review April 24, 2024 17:18
@msmouse msmouse added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Apr 24, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.10.1 ==> 093ca67e0dc3419d90dfbbbb6461962a1f29a298

Compatibility test results for aptos-node-v1.10.1 ==> 093ca67e0dc3419d90dfbbbb6461962a1f29a298 (PR)
1. Check liveness of validators at old version: aptos-node-v1.10.1
compatibility::simple-validator-upgrade::liveness-check : committed: 5165 txn/s, latency: 4848 ms, (p50: 5000 ms, p90: 5900 ms, p99: 11100 ms), latency samples: 242760
2. Upgrading first Validator to new version: 093ca67e0dc3419d90dfbbbb6461962a1f29a298
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1718 txn/s, latency: 16750 ms, (p50: 19600 ms, p90: 22600 ms, p99: 22900 ms), latency samples: 89380
3. Upgrading rest of first batch to new version: 093ca67e0dc3419d90dfbbbb6461962a1f29a298
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1833 txn/s, latency: 15706 ms, (p50: 18400 ms, p90: 22300 ms, p99: 22600 ms), latency samples: 91660
4. upgrading second batch to new version: 093ca67e0dc3419d90dfbbbb6461962a1f29a298
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3310 txn/s, latency: 9235 ms, (p50: 9800 ms, p90: 12600 ms, p99: 13200 ms), latency samples: 142340
5. check swarm health
Compatibility test for aptos-node-v1.10.1 ==> 093ca67e0dc3419d90dfbbbb6461962a1f29a298 passed
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 093ca67e0dc3419d90dfbbbb6461962a1f29a298

two traffics test: inner traffic : committed: 8047 txn/s, latency: 4876 ms, (p50: 4800 ms, p90: 5700 ms, p99: 10200 ms), latency samples: 3476360
two traffics test : committed: 100 txn/s, latency: 1991 ms, (p50: 1900 ms, p90: 2100 ms, p99: 5600 ms), latency samples: 1740
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.205, avg: 0.202", "QsPosToProposal: max: 0.245, avg: 0.219", "ConsensusProposalToOrdered: max: 0.450, avg: 0.395", "ConsensusOrderedToCommit: max: 0.390, avg: 0.373", "ConsensusProposalToCommit: max: 0.783, avg: 0.768"]
Max round gap was 1 [limit 4] at version 1734299. Max no progress secs was 4.906836 [limit 15] at version 1734299.
Test Ok

@msmouse msmouse requested a review from zekun000 April 24, 2024 22:58
@msmouse msmouse merged commit dd40342 into main Apr 24, 2024
99 checks passed
@msmouse msmouse deleted the 0424-enable-write-gas-for-events branch April 24, 2024 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@areshand areshand areshand approved these changes

@grao1991 grao1991 grao1991 approved these changes

@vgao1996 vgao1996 Awaiting requested review from vgao1996

@igor-aptos igor-aptos Awaiting requested review from igor-aptos

@zekun000 zekun000 Awaiting requested review from zekun000

Assignees
No one assigned
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@msmouse @areshand @grao1991