1480001 - Collect allocation counts only when profiling is enabled.

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Paul Bone [:pbone]]

After Bug 1473213 lands wait a week and then make that code run only when the gecko profiler is active.

Comment 1

[Paul Bone [:pbone]]

Attachment: Bug 1480001 - Enable allocation counts only when the profiler is active

And also no-longer collect these counts only on Nightly.

Comment 2

[Jon Coppeard (:jonco)]

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.

Comment 3

[Paul Bone [:pbone]]

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.

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1480001 - Enable allocation counts only when the profiler is active r=jonco

r+ carried forward.

Comment 5

[Paul Bone [:pbone]]

I can't build inbound locally right now.  But this patch works on central.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/940f9b38540e
Enable allocation counts only when the profiler is active. r=jonco

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/940f9b38540e