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

feat: stabilize lower_regular_op_cost feature #4979

Merged

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Oct 11, 2021

Feature to stabilize

This PR stabilizes new, lower regular_op_cost. It is lowered from 3_856_371 to 2_207_874. regular_op_cost is roughly the cost of primitive wasm operation, and is a major cost factor for computation-intensive contracts. The underlying change which allowed us to lower the cost is the switch from wasmer0 to wasmer2 implementation, which improved real wasmer perfomance across the board

Testing and QA

Cost measurement was reproduced by several people using both old and new estimator. The cost measurement itself is rather simple (probably the simplest of all our costs), so the relative risk here is low. Additionally, the reduction "makes sense" in terms of various ad-hoc comparisons of wall-clock times I did when comparing wasm runtimes.

Note that we are stabilizing this feature on a very short notice. While this in general is risky, I think it's ok for this particular case, as it is just cost reduction, and there's strong evidence that new wasmer is indeed faster. In a sense, most risk here is actually carried by #4934 , but that was on nightly for a significantly longed time.

Checklist

  • No nightly test regression
  • Update CHANGELOG.md to include this protocol feature in the Unreleased section.

@matklad
Copy link
Contributor Author

matklad commented Oct 11, 2021

@bowenwang1996 bad news: I didn't see PR template suggested anywhere, and I had to copy-paste it manually. Good news -- the idea of the template works, going to fill the changelog now as I've forgot to:)

@bowenwang1996
Copy link
Collaborator

@bowenwang1996 bad news: I didn't see PR template suggested anywhere, and I had to copy-paste it manually. Good news -- the idea of the template works, going to fill the changelog now as I've forgot to:)

Sad :( Also the checkboxes didn't really work :(

@bowenwang1996
Copy link
Collaborator

Looks like I forgot one space...

@near-bulldozer near-bulldozer bot merged commit fad8b3d into near:master Oct 12, 2021
@matklad matklad deleted the m/stablize-lowered-regular-op-cost branch October 12, 2021 11:04
matklad added a commit to matklad/nearcore that referenced this pull request Oct 26, 2021
forgot to do this in near#4979

This doesn't affect any logic anywhere, just makes it easier to figure
out what our measured costs are.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants