700357 - Do GC_SHRINK when we have lots of decommitable arenas

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Now that we can really take advantage of a GC_SHRINK, we should run it more frequently.  We should keep a heuristic counter of decommitable arenas and run GC_SHRINK when this number gets "big".

Comment 1

[Terrence Cole [:terrence]]

Attachment: v1: gcshrink when we have many decommitable arenas

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 572603 [details] [diff] [review]
v1: gcshrink when we have many decommitable arenas

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

Looks good.

::: js/src/jscntxt.h
@@ +422,5 @@
>      size_t              gcLastBytes;
>      size_t              gcMaxBytes;
>      size_t              gcMaxMallocBytes;
>      uint32              gcEmptyArenaPoolLifespan;
> +    uint32              gcNumFreeArenas;

Please make this volatile and put in a comment that we access it without holding the GC lock, but that it's okay because a race won't affect correctness.

::: js/src/jsgc.cpp
@@ +547,5 @@
>      Chunk *chunk = static_cast<Chunk *>(AllocChunk());
>      if (!chunk)
>          return NULL;
>      chunk->init();
> +    rt->gcNumFreeArenas += ArenasPerChunk;

Could you move this into Chunk::init (right after the loop that initializes each arena)?

Comment 3

[Gregor Wagner [:gwagner]]

Please make sure a GC_SHRINK GC doesn't happen during animations like FOTN or ro.me.
The main reason for the weak trigger behavior were regressions during animations.

Comment 4

[Terrence Cole [:terrence]]

Attachment: v2: With review feedback.

Comment 5

[Terrence Cole [:terrence]]

https://hg.mozilla.org/mozilla-central/rev/410deae0fe13

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Gregor Wagner from comment #3)
> Please make sure a GC_SHRINK GC doesn't happen during animations like FOTN
> or ro.me.
> The main reason for the weak trigger behavior were regressions during
> animations.

FOTN does not do any shrinking GCs with or without the patch.
ro.me does three shrinking GCs without the patch and four with the patch. However, I didn't notice any significant difference in the time taken for shrinking versus non-shrinking GCs. Mostly I think this is because even the normal GCs are so expensive on this benchmark. So I don't think this is really a regression.

Comment 7

[Daniel C]

"There are plans in place to also run these when clicking the Minimize Memory button in about:memory, and when we are under memory pressure"

Which bugs will fix those 2 issues?

Comment 8

[Justin Lebar (not reading bugmail)]

> Which bugs will fix those 2 issues?

Bug 699279, which can be found as one of the "depends on" bugs of bug 670596.