Bug 227957 – [BMalloc] Lazily allocate physical pages

• Status: RESOLVED
• Resolution: FIXED
• Component: bmalloc

[Michael Saboff]

Currently we call madvise(p, vmSize, MADV_FREE_REUSE) when we want to commit physical pages to a range of reserved memory.  On Darwin systems, the kernel will do this for us as we touch the pages.  Therefore we can eliminate this call, reducing our memory footprint when pages within a range are not used.

[Radar WebKit Bug Importer]

<rdar://problem/80583372>

[Michael Saboff]

Created attachment 433509 [details]
Patch

This patch provides a 3.5% (AS) to 4.6% (x86) reduction in memory in RAMification in my tests.

When testing JetStream2 and Speedometer2, it didn't regress on AS and is a 1.2% progression on both benchmarks for x86.

[Mark Lam]

Comment on attachment 433509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433509&action=review

r=me

> Source/bmalloc/bmalloc/VMAllocate.h:-218
> -    SYSCALL(madvise(p, vmSize, MADV_FREE_REUSE));

nit: Can you put the comment blurb from your ChangeLog in here about DARWIN not needing this madvise?  Otherwise, I can see someone not-in-the-know naively re-adding this back in the future with no visible immediate fallout to provide feedback that it should not be done.

[Michael Saboff]

(In reply to Mark Lam from comment #3)
> Comment on attachment 433509 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433509&action=review
> 
> r=me
> 
> > Source/bmalloc/bmalloc/VMAllocate.h:-218
> > -    SYSCALL(madvise(p, vmSize, MADV_FREE_REUSE));
> 
> nit: Can you put the comment blurb from your ChangeLog in here about DARWIN
> not needing this madvise?  Otherwise, I can see someone not-in-the-know
> naively re-adding this back in the future with no visible immediate fallout
> to provide feedback that it should not be done.

Sure.

[Geoffrey Garen]

Comment on attachment 433509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433509&action=review

> Source/bmalloc/bmalloc/VMAllocate.h:220
> +    vmValidatePhysical(p, vmSize);

Seems unnecessary to lose this assertion on Darwin.

[Michael Saboff]

Comment on attachment 433509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433509&action=review

>> Source/bmalloc/bmalloc/VMAllocate.h:220
>> +    vmValidatePhysical(p, vmSize);
> 
> Seems unnecessary to lose this assertion on Darwin.

I'll add it back.

[Michael Saboff]

Committed r279922 (239669@main): <https://commits.webkit.org/239669@main>