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

Feat: added wasm optimization #835

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from
Draft

Feat: added wasm optimization #835

wants to merge 11 commits into from

Conversation

mrLSD
Copy link
Member

@mrLSD mrLSD commented Sep 7, 2023 •
edited
Loading

Description

  • Added wasm optimization with wasm-opt.
  • Refactored test gas cost.

Gas cost

Gas costs decreased by 3-5%.

Binary size

Binary size decreased to 9%.

Tests

Refactored test gas cost.

@mrLSD mrLSD changed the title Feat> wasm-opt and refactored gas cost tests Feat: added wasm optimization Sep 7, 2023
@mrLSD mrLSD self-assigned this Sep 7, 2023
@mrLSD mrLSD added C-enhancement Category: New feature or request A-ci Area: Continuous Integration (CI) A-binary-size Area: Binary size improvements, or inflation. labels Sep 7, 2023
@mrLSD
Copy link
Member Author

mrLSD commented Sep 7, 2023

Related to #818

@mrLSD mrLSD marked this pull request as draft September 7, 2023 14:21
@mrLSD mrLSD added the A-testing Area: If something has added tests, or changed them. label Sep 7, 2023
@mrLSD mrLSD marked this pull request as ready for review September 7, 2023 14:51
@mrLSD mrLSD requested a review from aleksuss September 7, 2023 14:51
@mrLSD mrLSD requested review from birchmd and removed request for birchmd September 7, 2023 14:53
aleksuss
aleksuss requested changes Sep 7, 2023
View reviewed changes
Makefile.toml Outdated Show resolved Hide resolved
@aleksuss
Copy link
Member

aleksuss commented Sep 7, 2023

Also, we need to modify building artifacts script.

birchmd
birchmd reviewed Sep 7, 2023
View reviewed changes
Copy link
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

Cool! Nice to see low-hanging fruit like this giving a performance improvement.

We will probably also need to update the README or something to say that wasm-opt is now a required tool for building the project.

@aleksuss
Copy link
Member

Looks good, but I would postpone merging it right now.

@aleksuss
Copy link
Member

@mrLSD, why don't we use wasm-opt for xcc-router smart contract?

@mrLSD
Copy link
Member Author

mrLSD commented Sep 14, 2023

@mrLSD, why don't we use wasm-opt for xcc-router smart contract?

Because it's not part of the test-flow:

[tasks.test-flow]
category = "Test"
dependencies = [
    "build-test",
    "test-contracts",
    "test-workspace",
    "bench-modexp",
]

@aleksuss aleksuss removed the request for review from joshuajbouw October 11, 2023 21:32
@aleksuss
Merge branch 'develop' into feat/mrLSD/wasm-opt
48c6330 
# Conflicts:
#	.github/workflows/tests.yml
#	engine-tests/src/tests/uniswap.rs
@aleksuss aleksuss added the S-do-not-merge Status: Do not merge label Oct 13, 2023
@aleksuss aleksuss marked this pull request as draft October 13, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@birchmd birchmd birchmd approved these changes

@aleksuss aleksuss aleksuss approved these changes

Assignees

@mrLSD mrLSD

Labels
A-binary-size Area: Binary size improvements, or inflation. A-ci Area: Continuous Integration (CI) A-testing Area: If something has added tests, or changed them. C-enhancement Category: New feature or request S-do-not-merge Status: Do not merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mrLSD @aleksuss @birchmd