Speedometer triggers minorGC very often

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: smaug, Assigned: jonco)

Tracking

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(2 attachments)

This is the log of the minor GCs. The histogram of reasons is:

      1 DOM_WORKER
      1 LOAD_END
      2 API
      3 EAGER_ALLOC_TRIGGER
      6 INTER_SLICE_GC
     11 ALLOC_TRIGGER
     16 OUT_OF_NURSERY
     20 FULL_STORE_BUFFER

Total time (in microseconds) grouped by reason:

362 DOM_WORKER
3047 API
3657 LOAD_END
6655 EAGER_ALLOC_TRIGGER
30285 ALLOC_TRIGGER
33590 INTER_SLICE_GC
68648 OUT_OF_NURSERY
81462 FULL_STORE_BUFFER
(Assignee)

Updated

2 years ago
Whiteboard: [qf]
(In reply to Steve Fink [:sfink] [:s:] from comment #1)
>      20 FULL_STORE_BUFFER

It would be interesting to know which store buffer this is, and why we end up with a lot of entries in it..
(Assignee)

Comment 3

2 years ago
I made a patch to add separate reasons for each store buffer and got the following results:

 359 FULL_GENERIC_BUFFER
 101 FULL_CELL_PTR_BUFFER
  69 FULL_WHOLE_CELL_BUFFER
  52 FULL_SLOT_BUFFER

So it wasn't the cell pointer buffer as I had guessed but the generic buffer that's filling up.
(Assignee)

Comment 4

2 years ago
We use the generic buffer for updating shape accessor getter/setter objects, Map/Set objects containing nursery pointers and ConstraintTypeSets.  Looking at counts for generic buffer entries I get:

69553 GenericRef ShapeGetterSetterRef
 2871 GenericRef TypeSetRef
  542 GenericRef OrderedHashTableRef

In this case, it looks like accessor shapes are the culprit.
(Assignee)

Updated

2 years ago
Depends on: 1381058
(Assignee)

Comment 5

2 years ago
I experimented with changing shape hashing to use unique IDs and then using the whole cell store buffer to update a shape's nursery pointers.  This hit problems with sweep order: the unique IDs table is swept before we sweep arenas containing shapes, so it's not possible for a shape's parent to look up a dead child to remove it if we do this.
(Assignee)

Comment 6

2 years ago
Instead, here's a patch to keep a vector of accessor shapes containing nursery pointers on the zone and trace them in one go on minor GC.  This avoids adding a generic ref for each shape as we did previously.

Minor GC counts running speedometer with this patch:

  85 FULL_CELL_PTR_BUFFER
  79 FULL_SLOT_BUFFER
  51 FULL_WHOLE_CELL_BUFFER
  17 FULL_GENERIC_BUFFER

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6bf5093ad73fc9665ea17b1bec0ec5c2a11ce0b
Assignee: nobody → jcoppeard
Attachment #8886656 - Flags: review?(sphink)
Comment on attachment 8886656 [details] [diff] [review]
bug1380778-accessor-shape-vector

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

Wow. That seems to have totally nailed it.

::: js/src/vm/Shape-inl.h
@@ +157,5 @@
> +
> +    auto& storeBuffer = shape->runtimeFromActiveCooperatingThread()->gc.storeBuffer();
> +    if (nurseryShapes.length() == 1) {
> +        storeBuffer.putGeneric(NurseryShapesRef(shape->zone()));
> +    } if (nurseryShapes.length() == MaxShapeVectorLength) {

missing 'else'
Attachment #8886656 - Flags: review?(sphink) → review+

Comment 8

2 years ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0c73b22611c
Fixup shape pointers into the nursery using a dedicated list r=sfink

Updated

2 years ago
Whiteboard: [qf] → [qf:p1]

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d0c73b22611c
https://hg.mozilla.org/mozilla-central/rev/fbe95017106a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.