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

add limit on crypto_algebra element memory usage #8236

Merged
merged 6 commits into from
May 17, 2023
Merged

add limit on crypto_algebra element memory usage #8236

merged 6 commits into from
May 17, 2023

Conversation

zjma
Copy link
Contributor

@zjma zjma commented May 16, 2023 •
edited by alinush
Loading

Introducing a per-transaction limit on the memory used by crypto_algebra.move elements.

@zjma zjma marked this pull request as ready for review May 16, 2023 22:10
@zjma zjma requested review from saharct and alinush as code owners May 16, 2023 22:10
@sherry-x sherry-x requested review from runtian-zhou and vgao1996 May 16, 2023 22:22
alinush
alinush approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@alinush alinush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@alinush alinush changed the title add limit on algebra element memory usage add limit on crypto_algebra element memory usage May 17, 2023
@alinush alinush requested a review from mstraka100 May 17, 2023 16:04
@zjma zjma enabled auto-merge (squash) May 17, 2023 18:43
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
mstraka100
mstraka100 approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@mstraka100 mstraka100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 6aea7e1aad143ce37b44c5af3d3de85f59f22dda

performance benchmark : 6070 TPS, 6519 ms latency, 26200 ms p99 latency,(!) expired 40 out of 2592080 txns
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 10340 TPS, 3624 ms latency, 5700 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 6aea7e1aad143ce37b44c5af3d3de85f59f22dda
compatibility::simple-validator-upgrade::single-validator-upgrade : 5594 TPS, 7219 ms latency, 9900 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 6aea7e1aad143ce37b44c5af3d3de85f59f22dda
compatibility::simple-validator-upgrade::half-validator-upgrade : 5865 TPS, 6569 ms latency, 9000 ms p99 latency,no expired txns
4. upgrading second batch to new version: 6aea7e1aad143ce37b44c5af3d3de85f59f22dda
compatibility::simple-validator-upgrade::rest-validator-upgrade : 8094 TPS, 4630 ms latency, 7600 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda passed
Test Ok

@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda

Compatibility test results for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda (PR)
Upgrade the nodes to version: 6aea7e1aad143ce37b44c5af3d3de85f59f22dda
framework_upgrade::framework-upgrade::full-framework-upgrade : 6199 TPS, 6395 ms latency, 18400 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 6aea7e1aad143ce37b44c5af3d3de85f59f22dda passed
Test Ok

@zjma zjma merged commit 8371518 into main May 17, 2023
@zjma zjma deleted the zjma/algebra-element-memory-limit branch May 17, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mstraka100 mstraka100 mstraka100 approved these changes

@vgao1996 vgao1996 vgao1996 approved these changes

@alinush alinush alinush approved these changes

@saharct saharct Awaiting requested review from saharct

@runtian-zhou runtian-zhou Awaiting requested review from runtian-zhou

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zjma @alinush @vgao1996 @mstraka100