Bug 200576 – Remove GetbyIdStatus::m_variants inline capacity

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Caio Lima]

This inline capacity increases the sizeof(GetbyIdStatus) by the sizeof(GetByIdVariant). This class is heavily allocated into applications like Gmail, Facebook and Youtube and its size can be significant into memory usage of those applications.

[Caio Lima]

Created attachment 375927 [details]
Patch

[Caio Lima]

Ping review

[Mark Lam]

Comment on attachment 375927 [details]
Patch

Question: in the original, when the inline capacity was allocated, how often did the GetByIdStatus end up having 0 variants, 1 variant, and more than 1 variant?  The reason I ask is because just because we don't allocate the inline storage doesn't mean we won't just end up allocating it out of line, which might use more memory on net.  The answer to the above will give us a more accurate view of what is actually happening in terms of memory usage.  Better yet, can you actually measure peak memory usage for Youtube, Facebook, and Gmail with and without your change?

[Caio Lima]

(In reply to Mark Lam from comment #3)
> Comment on attachment 375927 [details]
> Patch
> 
> Question: in the original, when the inline capacity was allocated, how often
> did the GetByIdStatus end up having 0 variants, 1 variant, and more than 1
> variant?  The reason I ask is because just because we don't allocate the
> inline storage doesn't mean we won't just end up allocating it out of line,
> which might use more memory on net.  The answer to the above will give us a
> more accurate view of what is actually happening in terms of memory usage. 
> Better yet, can you actually measure peak memory usage for Youtube,
> Facebook, and Gmail with and without your change?

You have a point. I also double checked the numbers reported into the ChangeLog and it is considering allocations made only when we have more than 1 variant. In this case we need to also check peak memory for Gmail, youtube, Facebook and Twitter. Those numbers are reported bellow.

Facebook
Baseline: [548217137, 595023583, 719466876, 695031066]
Changes: [755340490, 744783097, 685176355, 647125982]

Mean Baseline: 639434665.5 (+-129235980.45566362)
Mean Changes: 708106481.0 (+-81244046.41008914)
--------------------------------------------------------
Gmail
Baseline: [800496414, 687438356, 691310661, 723753186]
Changes: [725568435, 686771646, 707648703, 708064106]

Mean Baseline: 725749654.25 (+-83418467.10122812)
Mean Changes: 707013222.5 (+-25251956.446118236)
--------------------------------------------------------
Youtube
Baseline: [514033245, 489025502, 457474593, 474551046]
Changes: [488632405, 490662379, 485534583, 479888722]

Mean Baseline: 483771096.5 (+-38099986.08839291)
Mean Changes: 486179522.25 (+-7469305.708872259)
--------------------------------------------------------
Twitter

Baseline: [322805946, 437475822, 365729371, 347809756]
Changes: [457314727, 451219264, 442859219, 477609382]

Mean Baseline: 368455223.75 (+-78392924.68422335)
Mean Changes: 457250648.0 (+-23565382.365001798)
--------------------------------------------------------

I collected those numbers using Safari's memory sampler, navigating each site for 2 minutes. It also reported the Fast Malloc Zone Bytes size.

Facebook
Baseline: [450289664, 477196288, 566833152, 566972416]
Changes: [604372992, 629719040, 560513024, 520310784]

Mean Baseline: 515322880.0 (+-96370596.4398483)
Mean Changes: 578728960.0 (+-76874954.55518961)
--------------------------------------------------------
Gmail
Baseline: [612704256, 557539328, 546308096, 568705024]
Changes: [569339904, 546295808, 559218688, 537972736]

Mean Baseline: 571314176.0 (+-46255052.229637325)
Mean Changes: 553206784.0 (+-22053579.052745104)
--------------------------------------------------------
Youtube
Baseline: [436858880, 414253056, 385675264, 399450112]
Changes: [406315008, 411574272, 410955776, 401158144]

Mean Baseline: 409059328.0 (+-34849135.84152007)
Mean Changes: 407500800.0 (+-7695414.722506702)
--------------------------------------------------------
Twitter

Baseline: [256970752, 364830720, 318169088, 288817152]
Changes: [368472064, 384253952, 375996416, 404676608]

Mean Baseline: 307196928.0 (+-72933629.42939076)
Mean Changes: 383349760.0 (+-24839897.48556769)
--------------------------------------------------------

From the results above, this change seems to be neutral to worse in memory usage in almost all cases. Gmail is the only application where the memory usage seems to be better if we remove inline storage. Given that, I think we shouldn't apply this Patch, since there is no net win, and likely to have a net loss.