Closed
Bug 1480001
Opened 6 years ago
Closed 6 years ago
Collect allocation counts only when profiling is enabled.
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
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)
(deleted),
patch
|
pbone
:
review+
|
Details | Diff | Splinter Review |
After Bug 1473213 lands wait a week and then make that code run only when the gecko profiler is active.
Assignee | ||
Updated•6 years ago
|
status-firefox61:
--- → unaffected
status-firefox62:
--- → unaffected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Depends on: 1473213
Assignee | ||
Comment 1•6 years ago
|
||
And also no-longer collect these counts only on Nightly.
Comment 2•6 years ago
|
||
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+
Assignee | ||
Comment 3•6 years ago
|
||
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.
status-firefox63:
--- → affected
Assignee | ||
Comment 4•6 years ago
|
||
r+ carried forward.
Attachment #8999913 -
Attachment is obsolete: true
Attachment #9004777 -
Flags: review+
Assignee | ||
Comment 5•6 years ago
|
||
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
Comment 7•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•