988369 - Reduce StoreBuffer chunk size on B2G and workers

Status: RESOLVED DUPLICATE of bug 1100652

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

It's currently allocated in 64KiB chunks, which is a bit too big for b2g. Try and make it one page per allocation and start decommitted.

Comment 1

[Terrence Cole [:terrence]]

Attachment: smaller_storebuffer_on_b2g-v0.diff

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8490857 [details] [diff] [review]
smaller_storebuffer_on_b2g-v0.diff

Review of attachment 8490857 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch!

Comment 3

[Terrence Cole [:terrence]]

Thank Andrew for asking about it in bug 1020751!

Comment 4

[Terrence Cole [:terrence]]

https://tbpl.mozilla.org/?tree=Try&rev=6555010758b8

Comment 5

[Terrence Cole [:terrence]]

Seems to cause a fair number of timeouts. Let's try being a bit smarter about how we size things:
https://tbpl.mozilla.org/?tree=Try&rev=89e623b1688e

Comment 6

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: Maybe this isn't the right tracking flag, but this really is needed when using generational GC on B2G.

Comment 7

[Terrence Cole [:terrence]]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> [Tracking Requested - why for this release]: Maybe this isn't the right
> tracking flag, but this really is needed when using generational GC on B2G.

Do we know that that is true though? GGC was memory neutral when it landed on desktop and we've made a ton of fixes since then. It would be nice if we had AWSY coverage on B2G.

Comment 8

[Andrew McCreight [:mccr8]]

(In reply to Terrence Cole [:terrence] from comment #7)
> Do we know that that is true though? GGC was memory neutral when it landed
> on desktop and we've made a ton of fixes since then. It would be nice if we
> had AWSY coverage on B2G.

It was memory neutral on AWSY, but had to be delayed a release because of too many OOM crashes on beta, so I'm not too confident in our ability to quickly detect regressions.  B2G regression detection is unfortunately probably worse than desktop, if anything.

Comment 9

[Benjamin Kerensa [:bkerensa]]

Bhavana, This is B2G so your call.

Comment 10

[Andrew McCreight [:mccr8]]

I haven't noticed any flood of complaints about memory regressions, so maybe this isn't so critical after all.

Comment 11

[Terrence Cole [:terrence]]

Yeah, I expect it's probably a wash with ggc's generally lower max-heap, even on mobile. We do still need to put some more effort into shaving off the store buffer's performance cliffs and I think this bug is going to be part of that solution eventually.

Comment 12

[bhavana bajaj [:bajaj]]

This would not be a blocker for release and given this will probably get into the next fxos release which is 2.2. Tracking it via the B2G flag.

Comment 13

[Terrence Cole [:terrence]]

I think Steve found the problem here: we're currently scaling the thresholds proportional to the block size, so this would have been triggering way too many compactions and minor collections. Steve is currently re-balancing our store buffer tunings for mobile/ffos as part of bug 1078437, so I'll wait until after that is in, if it doesn't fix the size problem outright too.

Comment 14

[Terrence Cole [:terrence]]

We've now totally redesigned the StoreBuffer to vastly reduce the fixed initial outlay. Closing this unless we have specific test-cases that are still terrible.