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

[POC]: fix(rust): allocation not working #78

Closed

Conversation

FaberVitale
Copy link
Contributor

@FaberVitale FaberVitale commented Sep 11, 2021

A poc for a fix to the allocation errors in rust.

Description

The main issue regarding allocation is tied to the fact that carts
can have at most a single memory page while heap allocation is implemented by requesting memory.grow and its memory is placed after the initial one.

This is all unfortunately working as intended, however, due to the way that the current wasm target works. The heap is fundamentally allocated via memory.grow instructions and can't be preallocated with the wasm32-unknown-unknown target's default allocator. Put another way, the default allocator provided by wasm32-unknown-unknown will unconditionally consider the current memory at startup as part of the wasm's own image (rodata, bss, static data, etc), so the only way it can get memory for it to manage is to call memory.grow.

source: rustwasm/wasm-bindgen#1389 (comment)

wee_alloc

Current fix uses the default allocator but switching to wee_alloc without increasing the max page size would not fix the issue:

Reading through the code it appears that what wee_alloc does is make the assumption that what ever memory we start with is off-limits completely, and instead will use the grow_memory instruction to create the first piece of memory needed for the heap. That effectively means that the index/address of the start of the heap is highest index of what ever the initial size is, plus one.

source: rustwasm/wee_alloc#61

I've tried to use wee_alloc with and without no_std before writing this PR; it always throws oom errors.


Regarding the POC

These changes allowed me to create new rust project and use and mutate Hashmap, Vec and String instances inside update.
I've not tested these changes in other languages nor I've not tried to use wee_alloc as global allocator.
Moreover I currently have no idea regarding the optimal maximum page size.

Why are you updating this.data & this.framebuffer?

  //  runtimes/web/src/runtime.js 
   this.data = new DataView(this.memory.buffer);
   this.framebuffer.updateMemory(this.memory.buffer);

This change is necessary to avoid 2 exceptions that are thrown by

  • <dataview-instance>.setUint<size>()
  • <framebuffer>.clear()

Issues

fixes #37 #36 #30

related issues #39 #42

References

@netlify
Copy link

netlify bot commented Sep 11, 2021

✔️ Deploy Preview for wasm4 ready!

🔨 Explore the source changes: 3fc080c

🔍 Inspect the deploy log: https://app.netlify.com/sites/wasm4/deploys/613cf674a0b2a70008990311

😎 Browse the preview: https://deploy-preview-78--wasm4.netlify.app

@aduros
Copy link
Owner

aduros commented Sep 11, 2021

I'm not too crazy about growing memory to 128 KB.

Do you think we can patch wee_alloc to use __heap_baseas discussed in rustwasm/wee_alloc#88?

@eXponenta
Copy link
Contributor

eXponenta commented Sep 11, 2021

Runtime is designed with limited RAM size, allow growing or increase a page count will be problem.

Problem if rust is problem of rust, you should use allocation free methods, that not required a page recreation or MUST be another way to fix allocator without allow growing.

Looks like a wee_alloc cannot be used on those machine :)

Same and AssemblyScript GC, it require more that 1 page memory for normal working a GC, because there are a some strange bugs with it.

This means that for AS right way use a statically allocated memory and use a object pools.

@FaberVitale
Copy link
Contributor Author

Do you think we can patch wee_alloc to use __heap_baseas discussed in rustwasm/wee_alloc#88?

I do not know enough about that section of their codebase:
you should probably open an issue at their repo.

I've tried to use wee_alloc with 'static_array' but it seems that it expects that the array length is at least page size:
it raises oom error immediately.

@aduros
Copy link
Owner

aduros commented Sep 12, 2021

Same and AssemblyScript GC, it require more that 1 page memory for normal working a GC, because there are a some strange bugs with it.

Whoa, really? cc @MaxGraey

@MaxGraey
Copy link
Contributor

MaxGraey commented Sep 12, 2021

We have not tested GC in an environment where the maximum memory cannot grow and limited to a single memory page. So I admit such problems. GC heuristics are now aimed more at performance than memory saving.

@aduros
Copy link
Owner

aduros commented Sep 20, 2021

@FaberVitale I played around with buddy-alloc and it seems to work well:

// These values can be tuned
const FAST_HEAP_SIZE: usize = 4 * 1024; // 4 KB
const HEAP_SIZE: usize = 16 * 1024; // 16 KB
const LEAF_SIZE: usize = 16;

static mut FAST_HEAP: [u8; FAST_HEAP_SIZE] = [0u8; FAST_HEAP_SIZE];
static mut HEAP: [u8; HEAP_SIZE] = [0u8; HEAP_SIZE];

#[global_allocator]
static ALLOC: NonThreadsafeAlloc = unsafe {
    let fast_param = FastAllocParam::new(FAST_HEAP.as_ptr(), FAST_HEAP_SIZE);
    let buddy_param = BuddyAllocParam::new(HEAP.as_ptr(), HEAP_SIZE, LEAF_SIZE);
    NonThreadsafeAlloc::new(fast_param, buddy_param)
};

Ideally this could be changed to use the __heap_base global variable provided by the linker:

extern "C" {
    static __heap_base: usize;
}

But I couldn't get it working due not being able to refer to __heap_base from the ALLOC initializer. Maybe someone who knows Rust can figure it out?

If that's not possible, leaving it up to the user to manually tune the heap sizes is probably fine for now.

@FaberVitale
Copy link
Contributor Author

@aduros Nice.

I'll play a bit with this setup this weekend.

@FaberVitale
Copy link
Contributor Author

@aduros

You can get the heap size using the following expression

extern "C" {
    static __heap_base: usize;
}

#[no_mangle]
pub extern "C" fn get_heap_base() -> *const () {
    unsafe { &__heap_base as *const _ as *const () }
}

Taken from rustwasm/wee_alloc#88 (comment)

get_heap_base() as usize is always equivalent to the value exported as __heap_base.

I'll tinker a bit to see if we can use 2 pointers based on the __heap_base value instead of using those 2 arrays:

when you build in debug mode there's a bit of framebuffer corruption.

FaberVitale added a commit to FaberVitale/wasm4 that referenced this pull request Sep 26, 2021
Description

Adds buddy-alloc as optional alloocator and increases the stack size.

See also:

- aduros#37

- aduros#78

- aduros#36
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.

[Rust] cannot allocate a String in update
4 participants