search

near / near-sdk-rs

Rust library for writing NEAR smart contracts
https://near-sdk.io
Apache License 2.0
462 stars 249 forks source link

Don't use wee_alloc! #910

Closed MaxGraey closed 2 years ago

MaxGraey commented 2 years ago

There are many reasons not to do it:

Just use default dlmalloc

See more details: https://github.com/rustwasm/wee_alloc/issues/107

austinabell commented 2 years ago

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 commented 2 years ago

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 https://github.com/near/near-sdk-rs/blob/fa293b406d7f4a4f66bf5f3b09219fbce731fdd7/near-sdk/Cargo.toml#L30).