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

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied 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 359341: Patch

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




--- Comment #3 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 359341
  --> https://bugs.webkit.org/attachment.cgi?id=359341
Patch

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

r- because red bots, also a suggested improvement

> Source/bmalloc/bmalloc/Heap.cpp:68
> +	       ptrdiff_t offset = ((random * vmPageSize()) % (gigacageSize() -
Gigacage::minimumCageSizeAfterSlide));

This seems wrong. You're losing low bits of entropy by multiplying.

The best way to do this is probably to compute a random offset, allowing for
minimumCageSizeAfterSlide, and then round down to page alignment at the very
end. That way, you don't need an ASSERT about page alignment either. You can
trade an assert for code that enforces the condition you want.