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

Simplify the implementation of sbrk(). #20793

Merged
merged 3 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 28 additions & 44 deletions system/lib/libc/sbrk.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#ifdef __EMSCRIPTEN_TRACING__
void emscripten_memprof_sbrk_grow(intptr_t old, intptr_t new);
#else
#define emscripten_memprof_sbrk_grow(...) ((void)0)
#endif

#include <emscripten/heap.h>
Expand Down Expand Up @@ -56,65 +58,47 @@ uintptr_t* emscripten_get_sbrk_ptr() {
// Enforce preserving a minimal alignof(maxalign_t) alignment for sbrk.
#define SBRK_ALIGNMENT (__alignof__(max_align_t))

#ifdef __EMSCRIPTEN_SHARED_MEMORY__
#define READ_SBRK_PTR(sbrk_ptr) (__c11_atomic_load((_Atomic(uintptr_t)*)(sbrk_ptr), __ATOMIC_SEQ_CST))
#else
#define READ_SBRK_PTR(sbrk_ptr) (*(sbrk_ptr))
juj marked this conversation as resolved.
Show resolved Hide resolved
#endif

void *sbrk(intptr_t increment_) {
uintptr_t old_size;
uintptr_t increment = (uintptr_t)increment_;
increment = (increment + (SBRK_ALIGNMENT-1)) & ~(SBRK_ALIGNMENT-1);
#ifdef __EMSCRIPTEN_SHARED_MEMORY__
// Our default dlmalloc uses locks around each malloc/free, so no additional
// work is necessary to keep things threadsafe, but we also make sure sbrk
// itself is threadsafe so alternative allocators work. We do that by looping
// and retrying if we hit interference with another thread.
uintptr_t expected;
uintptr_t *sbrk_ptr = (uintptr_t*)emscripten_get_sbrk_ptr();

// To make sbrk thread-safe, implement a CAS loop to update the
// value of sbrk_ptr.
while (1) {
#endif // __EMSCRIPTEN_SHARED_MEMORY__
uintptr_t* sbrk_ptr = emscripten_get_sbrk_ptr();
#ifdef __EMSCRIPTEN_SHARED_MEMORY__
uintptr_t old_brk = __c11_atomic_load((_Atomic(uintptr_t)*)sbrk_ptr, __ATOMIC_SEQ_CST);
#else
uintptr_t old_brk = *sbrk_ptr;
#endif
uintptr_t old_brk = READ_SBRK_PTR(sbrk_ptr);
uintptr_t new_brk = old_brk + increment;
// Check for an overflow, which would indicate that we are trying to
// allocate over maximum addressable memory.
if (increment > 0 && new_brk <= old_brk) {
goto Error;
}
old_size = emscripten_get_heap_size();
if (new_brk > old_size) {
// Try to grow memory.
if (!emscripten_resize_heap(new_brk)) {
goto Error;
}
// Check for a) an overflow, which would indicate that we are trying to
// allocate over maximum addressable memory. and b) if necessary,
// increase the WebAssembly Memory size, and abort if that fails.
if ((increment > 0 && new_brk <= old_brk)
|| (new_brk > emscripten_get_heap_size() && !emscripten_resize_heap(new_brk))) {
SET_ERRNO();
return (void*)-1;
}
#ifdef __EMSCRIPTEN_SHARED_MEMORY__
// Attempt to update the dynamic top to new value. Another thread may have
// beat this one to the update, in which case we will need to start over
// by iterating the loop body again.
expected = old_brk;
__c11_atomic_compare_exchange_strong(
(_Atomic(uintptr_t)*)sbrk_ptr,
&expected, new_brk,
__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
if (expected != old_brk) {
continue;
}
#else // __EMSCRIPTEN_SHARED_MEMORY__
uintptr_t expected = old_brk;

__c11_atomic_compare_exchange_strong((_Atomic(uintptr_t)*)sbrk_ptr,
&expected, new_brk, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);

if (expected != old_brk) continue; // CAS failed, another thread raced in between.
#else
*sbrk_ptr = new_brk;
#endif // __EMSCRIPTEN_SHARED_MEMORY__
#endif

#ifdef __EMSCRIPTEN_TRACING__
emscripten_memprof_sbrk_grow(old_brk, new_brk);
#endif
return (void*)old_brk;

#ifdef __EMSCRIPTEN_SHARED_MEMORY__
}
#endif // __EMSCRIPTEN_SHARED_MEMORY__

Error:
SET_ERRNO();
return (void*)-1;
}

int brk(void* ptr) {
Expand Down
4 changes: 2 additions & 2 deletions test/code_size/hello_webgl2_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"a.js": 4699,
"a.js.gz": 2419,
"a.wasm": 10482,
"a.wasm.gz": 6726,
"a.wasm.gz": 6728,
"total": 15750,
"total_gz": 9524
"total_gz": 9526
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl2_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 567,
"a.html.gz": 379,
"a.js": 18004,
"a.js.gz": 8135,
"a.js": 18025,
"a.js.gz": 8138,
"a.mem": 3123,
"a.mem.gz": 2693,
"total": 21694,
"total_gz": 11207
"total": 21715,
"total_gz": 11210
}
4 changes: 2 additions & 2 deletions test/code_size/hello_webgl_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"a.js": 4185,
"a.js.gz": 2243,
"a.wasm": 10482,
"a.wasm.gz": 6726,
"a.wasm.gz": 6728,
"total": 15236,
"total_gz": 9348
"total_gz": 9350
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 567,
"a.html.gz": 379,
"a.js": 17481,
"a.js.gz": 7960,
"a.js": 17502,
"a.js.gz": 7961,
"a.mem": 3123,
"a.mem.gz": 2693,
"total": 21171,
"total_gz": 11032
"total": 21192,
"total_gz": 11033
}