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: remove deprecated wee_alloc #1151

Closed
wants to merge 1 commit into from

Conversation

njelich
Copy link

@njelich njelich commented Feb 25, 2024

The wee_alloc library has been unmaintained for 3 years and has serious issues (e.g. a memory leak affecting some WASM compiles)

rustwasm/wee_alloc#106

@njelich
Copy link
Author

njelich commented Feb 26, 2024

Rust uses https://github.com/alexcrichton/dlmalloc-rs as a default allocator for WASM.

@frol
Copy link
Collaborator

frol commented Feb 26, 2024

@njelich Are there any expected side-effects of this change?

@njelich
Copy link
Author

njelich commented Feb 28, 2024

I ran the full test suite, and other than previously failing tests as mentioned in the other PR, everything passed. The build size for the cargo-template project increases from 133KB to 139KB, but that is a fixed increase of 5-6KB on any project.

@njelich
Copy link
Author

njelich commented Mar 7, 2024

Based on discussion in the NEAR Tools Community channel, this change will not be implemented right now. For future maintainers, here are the benchmarks of different allocator options that were discussed:

Results of testing different allocators on cargo-near-new-project-template, with regards to final WASM compile size:

wee_alloc (current) - 132.57KB
dlmalloc - 138.73KB
talc (unknown arena size) - 136.28KB
talc (known arena size) - 134.72KB
rlsf - 132.70KB
lol_alloc (single threaded) - 131.60KB
lol_alloc (generic) - 131.74KB
custom_bump_alloc - 130.97KB

@njelich njelich closed this Mar 7, 2024
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.

None yet

2 participants