[webkit-reviews] review granted: [Bug 193523] Gigacages should start allocations from a slide : [Attachment 359430] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 193523: Gigacages should start allocations from a slide
https://bugs.webkit.org/show_bug.cgi?id=193523

Attachment 359430: Patch

https://bugs.webkit.org/attachment.cgi?id=359430&action=review




--- Comment #18 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 359430
  --> https://bugs.webkit.org/attachment.cgi?id=359430
Patch

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

r=me with suggestions and fixes.

> Source/bmalloc/ChangeLog:8
> +	   This patch makes is so that Gigacage Heaps slide the start of the

typo: /makes is so/makes it so/.

> Source/bmalloc/bmalloc/Gigacage.cpp:47
> +constexpr uint64_t gigacageRunway = 32llu * 1024 * 1024 * 1024;

Why not just use a size_t gigacageRunway to begin with?

> Source/bmalloc/bmalloc/Gigacage.cpp:112
> +	   return static_cast<size_t>(gigacageRunway);

You can remove this cast since we can define gigacageRunway as a size_t to
begin with.

> Source/bmalloc/bmalloc/Heap.cpp:67
> +	       RELEASE_BASSERT(gigacageSize() >
Gigacage::minimumCageSizeAfterSlide);

Can you just make this a static_assert in Source/bmalloc/bmalloc/Gigacage.h
instead?

    static_assert(primitiveGigacageSize > minimumCageSizeAfterSlide);
    static_assert(jsValueGigacageSize > minimumCageSizeAfterSlide);

That way, we can catch this configuration error at compile time instead.