Do GC_SHRINK when we have lots of decommitable arenas

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

Trunk
mozilla10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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".
(Assignee)

Comment 1

6 years ago
Created attachment 572603 [details] [diff] [review]
v1: gcshrink when we have many decommitable arenas
Attachment #572603 - Flags: review?(wmccloskey)
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)?
Attachment #572603 - Flags: review?(wmccloskey) → review+
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.
(Assignee)

Comment 4

6 years ago
Created attachment 572672 [details] [diff] [review]
v2: With review feedback.
Attachment #572603 - Attachment is obsolete: true
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/mozilla-central/rev/410deae0fe13
(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.
Target Milestone: --- → mozilla10
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 7

6 years ago
"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?
> Which bugs will fix those 2 issues?

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

Updated

6 years ago
Depends on: 711623
You need to log in before you can comment on or make changes to this bug.