Closed Bug 1480001 Opened Last year Closed Last year

Collect allocation counts only when profiling is enabled.

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
thunderbird_esr52 --- unaffected
thunderbird_esr60 --- unaffected
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

Details

Attachments

(1 file, 1 obsolete file)

After Bug 1473213 lands wait a week and then make that code run only when the gecko profiler is active.
And also no-longer collect these counts only on Nightly.
Assignee: nobody → pbone
Status: NEW → ASSIGNED
Attachment #8999913 - Flags: review?(jcoppeard)
Comment on attachment 8999913 [details] [diff] [review]
Bug 1480001 - Enable allocation counts only when the profiler is active

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

::: js/src/gc/Nursery.cpp
@@ +392,5 @@
>  
>      void* thing = (void*)position();
>      position_ = position() + size;
> +    // We count this regardless of the profiler's state, assuming that it costs just as much to
> +    // count it, as to check the profiler's state and decide not to count it.

It would be good if we didn't have to do this, but it probably doesn't make any difference.

@@ +619,5 @@
> +    // These counters only contain consistent data if the profiler is enabled,
> +    // and then there's no guarentee.  But if this code is executing, we know
> +    // the profiler is enabled.  Therefore these counters are consistent.
> +    // Assert it just in case.  Unlike the tenured heap,
> +    // Nursery::renderProfileJSON() is not used by telemetry.

This is only true because of the way CycleCollectedJSRuntime::GCNurseryCollectionCallback works.  This is outside our component so I think it's best to check here.

nit: single space between sentences in comments.
Attachment #8999913 - Flags: review?(jcoppeard) → review+
I want this to land on 63 since it's the 2nd part of Bug 1473213, but it's already soft-freeze time, I'm going to attempt to land it anyway since it's small.
r+ carried forward.
Attachment #8999913 - Attachment is obsolete: true
Attachment #9004777 - Flags: review+
I can't build inbound locally right now.  But this patch works on central.
Keywords: checkin-needed
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/940f9b38540e
Enable allocation counts only when the profiler is active. r=jonco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/940f9b38540e
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1494278
You need to log in before you can comment on or make changes to this bug.