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

Fix memory leak and module drop order #690

Merged
merged 8 commits into from
Apr 1, 2022
Merged

Fix memory leak and module drop order #690

merged 8 commits into from
Apr 1, 2022

Conversation

yun-yeo
Copy link
Contributor

@yun-yeo yun-yeo commented Mar 3, 2022

Summary of changes

close #683

This PR is picking custom wasmer and cosmwasm and wasmvm to fix following problems:

Report of required housekeeping

  • Github issue OR spec proposal link
  • Wrote tests
  • Updated API documentation (client/lcd/swagger-ui/swagger.yaml)
  • Added a relevant changelog entry: clog add [section] [stanza] [message]

(FOR ADMIN) Before merging

  • Added appropriate labels to PR
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Confirm added tests are consistent with the intended behavior of changes
  • Ensure all tests pass

@yun-yeo yun-yeo self-assigned this Mar 3, 2022
@yun-yeo yun-yeo added enhancement New feature or request wasm Wasm contract related update labels Mar 3, 2022
@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #690 (8c48fee) into main (a6b93b7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #690   +/-   ##
=======================================
  Coverage   45.46%   45.46%           
=======================================
  Files         121      121           
  Lines        7063     7063           
=======================================
  Hits         3211     3211           
  Misses       3605     3605           
  Partials      247      247           

hanjukim
hanjukim previously approved these changes Mar 5, 2022
@yun-yeo
Copy link
Contributor Author

yun-yeo commented Apr 1, 2022

wasmer finally released 2.2.1 and cosmwasm is now 0.16.6

It contains all the fixes we needed (drop order fix); therefore vanilla cosmwasm 0.16.6 is usable without any modifications.

We believe drop order fix fixed mem leak too.
Our infra nodes are running 0.16.6 already (to be released soon) and is stable without any sign of memleak

tho wasmer 2.2.1 changed its cache module format, and cosmwasm is forced to rebuild all cache upon request (either execute/query).

^ this can eat up resources under heavy concurrency; We wrote a script to rebuild module cache from .wasm blobs to the new format — https://github.com/kjessec/wasm-cache-rebuilder

@yun-yeo yun-yeo requested a review from Vritra4 April 1, 2022 06:07
Vritra4
Vritra4 previously approved these changes Apr 1, 2022
Copy link

@Vritra4 Vritra4 left a comment

Choose a reason for hiding this comment

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

lgtm

@yun-yeo yun-yeo merged commit 92f6603 into main Apr 1, 2022
@yun-yeo yun-yeo deleted the memory-test branch April 1, 2022 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wasm Wasm contract related update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wasmer Issue Tracking
3 participants