Closed Bug 1480001 Opened Last year Closed Last year
Collect allocation counts only when profiling is enabled
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.
I can't build inbound locally right now. But this patch works on central.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/940f9b38540e Enable allocation counts only when the profiler is active. r=jonco
You need to log in before you can comment on or make changes to this bug.