Use a vector for the statistics phase stack

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript: GC
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
As pointed out in but 1361458.  This currently uses an Array but it would be much simpler if it used a Vector.
(Assignee)

Comment 1

5 months ago
Created attachment 8870482 [details] [diff] [review]
bug1365539-phase-stack

Use a vector rather than array and separate length.

I had to fiddle with the the setGCCallback testing function - hopefully I understood this correctly.
Attachment #8870482 - Flags: review?(sphink)
Comment on attachment 8870482 [details] [diff] [review]
bug1365539-phase-stack

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

::: js/src/gc/GenerateStatsPhases.py
@@ +232,5 @@
>              phase.name = "%s_%d" % (phaseKind.name, index + 1)
>  
> +# Find the maximum phase nesting.
> +
> +MaxPhaseNesting = max([phase.depth for phase in AllPhases]) + 1

You don't need to materialize the intermediate array. This can be

MaxPhaseNesting = max(phase.depth for phase in AllPhases) + 1

::: js/src/gc/Statistics.h
@@ +205,5 @@
>      TimeDuration getMaxGCPauseSinceClear();
>  
>      PhaseKind currentPhaseKind() const;
>  
> +    static const size_t MAX_SUSPENED_PHASES = MAX_PHASE_NESTING * 3;

*SUSPENDED

The suspension mechanism is barely used now. It's still needed for one thing, though, so it can't simply be removed. It feels like a wart of excess complexity, though.

::: js/src/tests/shell/gcstats.js
@@ +30,5 @@
>  garbage();
>  
>  setGCCallback({
>    action: "majorGC",
> +  depth: 8,

Uh, yeah, whatever.
Attachment #8870482 - Flags: review?(sphink) → review+

Comment 3

5 months ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64fedb38fb15
Refactor GC statistics phase stack to use a vector r=sfink

Comment 4

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/64fedb38fb15
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.