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

Don't use wee_alloc! #910

Closed
MaxGraey opened this issue Aug 31, 2022 · 2 comments
Closed

Don't use wee_alloc! #910

MaxGraey opened this issue Aug 31, 2022 · 2 comments

Comments

@MaxGraey
Copy link

MaxGraey commented Aug 31, 2022

There are many reasons not to do it:

  • Couple of memory leak bugs. One of them
  • It's slow. It takes O(N) time for alloc due to simple linked list under the hood
  • It's unmaintained, mostly 2 yeahs

Just use default dlmalloc

See more details:
rustwasm/wee_alloc#107

@austinabell
Copy link
Contributor

austinabell commented Aug 31, 2022

There are many reasons not to do it:

Memory leaks like this don't matter in our context since the functions are short-lived.

  • It's slow. It takes O(N) time for alloc due to simple linked list under the hood
  • It's unmaintained, mostly 2 yeahs

Agree wee_alloc isn't ideal, but tradeoffs with others. For example, in our context, we care a lot about compiled size, and wee_alloc is quite small. We could even go simpler with something like a bump allocator (since we don't care about de-allocations) or something more involved with a larger code size but more efficient, but it's opinionated.

Just use default dlmalloc

See more details: rustwasm/wee_alloc#107

The dlmalloc default has been too bulky to use by default. Can definitely explore deeper though! In any case, we would not remove wee_alloc likely, but just support other options for allocators and possibly change the default to something, whether it be custom or some lib that's setup for the general case.

Related #457

@austinabell
Copy link
Contributor

Was fine to keep open, but I suppose we can use the other linked issue to track. One thing I will note about this is that wee_alloc can be disabled through the SDK and the dlmalloc default can be used (it's an optional dependency

wee_alloc = { version = "0.4.5", default-features = false, optional = true }
).

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

No branches or pull requests

2 participants