1529265 - ZoneGCStats::collectedZoneCount seems to be buggy/confusing

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jan de Mooij [:jandem]]

This field:

  /* Number of zones collected in this GC. */
  int collectedZoneCount = 0;

Is used here:

  bool isFullCollection() const {
    return collectedZoneCount == collectableZoneCount;
  }

Both of these fields are computed in GCRuntime::scanZonesBeforeGC:

    if (zone->canCollect()) {
      zoneStats.collectableZoneCount++;
    }
    if (zone->isGCScheduled()) {
      zoneStats.collectedZoneCount++;
      ...

It's possible however for a zone to have isGCScheduled() with canCollect() == false (via JS::PrepareForFullGC for example).

This can result in repeatedly triggering full GCs in Gecko:

https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/dom/base/nsJSEnvironment.cpp#2214

Comment 1

[Jan de Mooij [:jandem]]

I think we shouldn't increment collectedZoneCount when zone->canCollect() is false?

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1529265 - Don't include scheduled zones we can't collect in zoneStats.collectedZoneCount. r?sfink!

Comment 3

[Paul Bone [:pbone]]

Makes sense to me.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e59c9bfdc96
Don't include scheduled zones we can't collect in zoneStats.collectedZoneCount. r=sfink

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0e59c9bfdc96