Skip to content

Conversation

@s1na
Copy link
Collaborator

@s1na s1na commented May 2, 2019

Closes #46.

So far it's a really dumb allocator. It simply tries to allocate enough wasm pages everytime the alloc method is called (i.e. pages are not filled with subsequent alloc calls). It also doesn't dealloc. I tested it with runevm+hera and it passes some tests.

@s1na
Copy link
Collaborator Author

s1na commented May 14, 2019

Some notes:

  • The allocation algorithm is a rough translation of @poemm's C malloc
  • It's been only implemented for arch wasm32
  • I tried it with a few simple test cases, but not fully sure about correctness

From a rust developer PoV this code is probably an abomination 😄

@poemm
Copy link

poemm commented May 14, 2019

The C version of dumb malloc() is untested for edge cases and assumes things about LLVM compiler output.

The good news: dumb malloc() compiles to wasm in 15 bytes (or 60 bytes if the preprocessor flag GROWABLE_MEMORY is set to true). This mostly solves the problem of memory allocation in Ewasm -- no host system library is needed for malloc() since its size is nearly negligible.

@axic axic force-pushed the dumb_alloc branch 2 times, most recently from 68a91af to f6cdb4e Compare May 28, 2019 14:57
@axic axic changed the title [WIP] Add dumb allocator Introduce qimalloc as an allocator option May 28, 2019
@axic axic requested a review from jakelang May 28, 2019 15:01
@axic
Copy link
Member

axic commented May 28, 2019

Changed this to use the new crate: https://github.com/wasmx/qimalloc

@s1na
Copy link
Collaborator Author

s1na commented May 28, 2019

Looks good to me, can't approve my own PR though :)

#[macro_use]
extern crate cfg_if;

cfg_if! {
Copy link
Member

Choose a reason for hiding this comment

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

I am pretty sure we don't need the cfg_if dependency here. We can do something like #[cfg(feature = "wee_alloc")] static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;

Copy link
Member

Choose a reason for hiding this comment

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

It also has/needs that #[global_allocator] annotation. Will that work?

Copy link
Member

Choose a reason for hiding this comment

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

I think so, attributes apply to the item below. If the item is not compiled in, then the attribute isn't either.

@axic axic force-pushed the dumb_alloc branch 2 times, most recently from d1e4114 to 7557f21 Compare May 28, 2019 19:41
Copy link
Member

@jakelang jakelang left a comment

Choose a reason for hiding this comment

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

This looks good enough. We'll re-address the cfg-if question later.

@axic
Copy link
Member

axic commented May 28, 2019

Released qimalloc so we can refer to a version and not just a random repo in this crate.

@axic axic merged commit 346b01f into master May 28, 2019
@axic axic deleted the dumb_alloc branch May 28, 2019 20:12
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.

Implement "primitive allocator" which never frees

5 participants