[webkit-reviews] review denied: [Bug 192389] bmalloc uses more memory on iOS compared to macOS due to physical page size differences : [Attachment 357062] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 192389: bmalloc uses more memory on iOS compared to macOS due to physical
page size differences
https://bugs.webkit.org/show_bug.cgi?id=192389

Attachment 357062: Patch

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




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

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

> Source/bmalloc/bmalloc/Heap.cpp:129
> -	   for (size_t pageSize = m_vmPageSizePhysical;
> +	   for (size_t pageSize = smallPageSize;
>	       pageSize < pageSizeMax;
> -	       pageSize += m_vmPageSizePhysical) {
> +	       pageSize += smallPageSize) {

I think you need some code here to ensure that you don't use 12kB pages on 16kB
page systems. On 16kB page systems, we want pages sizes of [ 4kB, 8kB, 16kB ...
]

Also worth testing whether including 2kB in the mix reduces memory use without
causing a throughput regression.

I think you also need some code to ensure that you don't use the 20kB page
size, etc. on 16kB systems. Those are really awkward values that will produce a
lot of physical page size waste by defeating the scavenger.

The simplest solution is just to use [ 4kB, 8kB, 16kB, 32kB ... ]. But it might
also be useful to include 2kB.