Skip to content

Commit b432152

Browse files
authored
Update comments in emmalloc.c regarding root regions and sbrk(). (#20784)
1 parent ab63f7a commit b432152

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

system/lib/emmalloc.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
* - sbrk() is used to claim new memory (sbrk handles geometric/linear
1313
* - overallocation growth)
14-
* - sbrk() can be used by other code outside emmalloc.
14+
* - sbrk() can also be called by other code, not reserved to emmalloc only.
1515
* - sbrk() is very fast in most cases (internal wasm call).
1616
* - sbrk() returns pointers with an alignment of alignof(max_align_t)
1717
*
@@ -27,6 +27,8 @@
2727
* merged.
2828
* - Memory allocation takes constant time, unless the alloc needs to sbrk()
2929
* or memory is very close to being exhausted.
30+
* - Free and used regions are managed inside "root regions", which are slabs
31+
* of memory acquired via calls to sbrk().
3032
*
3133
* Debugging:
3234
*
@@ -486,10 +488,11 @@ static bool claim_more_memory(size_t numBytes)
486488
validate_memory_regions();
487489
#endif
488490

489-
// Make sure we send sbrk requests of aligned sizes. If we do not do that then
490-
// it will not return contiguous regions, which leads to use creating more
491-
// root regions below, inefficiently. (Note that we assume our alignment is
492-
// identical to sbrk's, see the assumptions at the start of this file.)
491+
// Make sure we always send sbrk requests with the same alignment that sbrk()
492+
// allocates memory at. Otherwise we will not properly interpret returned memory
493+
// to form a seamlessly contiguous region with earlier root regions, which would
494+
// lead to inefficiently treating the sbrk()ed region to be a new disjoint root
495+
// region.
493496
numBytes = (size_t)ALIGN_UP(numBytes, MALLOC_ALIGNMENT);
494497

495498
// Claim memory via sbrk
@@ -517,7 +520,7 @@ static bool claim_more_memory(size_t numBytes)
517520
if (startPtr == previousSbrkEndAddress)
518521
{
519522
#ifdef EMMALLOC_VERBOSE
520-
MAIN_THREAD_ASYNC_EM_ASM(err('claim_more_memory: expanding previous'));
523+
MAIN_THREAD_ASYNC_EM_ASM(err('claim_more_memory: sbrk() returned a region contiguous to last root region, expanding the existing root region'));
521524
#endif
522525
Region *prevEndSentinel = prev_region((Region*)startPtr);
523526
assert(debug_region_is_consistent(prevEndSentinel));
@@ -544,9 +547,11 @@ static bool claim_more_memory(size_t numBytes)
544547
}
545548
else
546549
{
547-
// Create a root region at the start of the heap block
550+
// Unfortunately some other user has sbrk()ed to acquire a slab of memory for themselves, and now the sbrk()ed
551+
// memory we got is not contiguous with our previous managed root regions.
552+
// So create a new root region at the start of the sbrk()ed heap block.
548553
#ifdef EMMALLOC_VERBOSE
549-
MAIN_THREAD_ASYNC_EM_ASM(err('claim_more_memory: creating new root region'));
554+
MAIN_THREAD_ASYNC_EM_ASM(err('claim_more_memory: sbrk() returned a disjoint region to last root region, some external code must have sbrk()ed outside emmalloc(). Creating a new root region'));
550555
#endif
551556
create_used_region(startPtr, sizeof(Region));
552557

0 commit comments

Comments
 (0)