Bug 201631 – [JSC] Manually adding 256 size class to make the current JSC allocation behavior stable

• Component: JavaScriptCore
• Status: NEW

[Yusuke Suzuki]

We have old and hacky code like this.

add(sizeof(UnlinkedFunctionCodeBlock));

This manually adds sizeof(UnlinkedFunctionCodeBlock) to size class sequence.
This is really fragile since size-class sequence depends on UnlinkedFunctionCodeBlock size.
The size-class sequence is very fundamental thing for JSC's allocation pattern. And changing this completely changes how JSC allocates.

bug 201613 changed sizeof(UnlinkedFunctionCodeBlock) and dramatically changes the allocation behavior of JSC.
We should make the previous allocation pattern stable.

Instead of adding `add(sizeof(UnlinkedFunctionCodeBlock));`, adding `add(256)` directly to make the previous behavior as baseline.

[Yusuke Suzuki]

Created attachment 378443 [details]
Patch

[Yusuke Suzuki]

The previous size-class sequence
JSC Heap MarkedSpace size class dump: 16, 32, 48, 64, 80, 112, 160, 224, 256, 320, 432, 608, 880, 1232, 1776, 2672, 4016, 5360, 8032

The current size-class sequence
JSC Heap MarkedSpace size class dump: 16, 32, 48, 64, 80, 112, 160, 224, 240, 320, 432, 608, 880, 1232, 1776, 2672, 4016, 5360, 8032

This patch will explicitly select the previous sequence. 224 <-> 240 is only 16, one cell size. We know 256 works well so we will pick 256 here.
256's characteristics is nice. 256*2 will becomes 608 size class, and 608*2 will becomes 1232. We know the previous size-class sequence tends to work well with the code like,

var array = [];
while (...)
    array.push(value);

[Mark Lam]

Comment on attachment 378443 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:38
> +        This patch explicitly adds 256 into the size-class to make the size-class sequence the thing we know it works well.

I suggest /know it works well/know works well/.

> Source/JavaScriptCore/heap/MarkedSpace.cpp:139
> +            // Manually adding 256 case, empiricailly know this produces good sequence of size classes.

I suggest rephrasing slightly: "Manually adding 256 case. We know empirically that this produces efficient memory usage of size classes."

[Mark Lam]

Comment on attachment 378443 [details]
Patch

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

>> Source/JavaScriptCore/heap/MarkedSpace.cpp:139
>> +            // Manually adding 256 case, empiricailly know this produces good sequence of size classes.
> 
> I suggest rephrasing slightly: "Manually adding 256 case. We know empirically that this produces efficient memory usage of size classes."

Better yet: "Manually adding 256 case. We know empirically that having this size class results in more efficient memory usage." or something like that.

[Yusuke Suzuki]

Hmm, after checking the regression, it seems that this does not affect much. I'll look into further.
macOS regression seems due to scavenger's change.

[Yusuke Suzuki]

Comment on attachment 378443 [details]
Patch

I'll look into the problem further. After I'm confident this is better, I'll upload the revised version of this.