Closed
Bug 1409324
Opened 7 years ago
Closed 7 years ago
Update the resize telemetry/profiling for GCMinor
Categories
(Core :: JavaScript: GC, enhancement, P3)
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: pbone, Assigned: pbone)
References
Details
Attachments
(2 files, 1 obsolete file)
1.39 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
2.36 KB,
patch
|
pbone
:
review+
|
Details | Diff | Splinter Review |
Bug 1298018 will mean that the resize phase of a minor GC is going to be exceptionally quick, since it no-longer does any allocation itself and therefore there is no point in measuring it for telemetry or profiling. Instead we may wish to measure any (tiny) pauses as the nursery grows lazilly. Investigate what we currently do for the tenured heap and see if we should replicate that.
Assignee | ||
Updated•7 years ago
|
status-firefox56:
--- → wontfix
status-firefox57:
--- → wontfix
status-firefox-esr52:
--- → wontfix
Depends on: 1298018
Priority: -- → P3
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → pbone
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8928111 -
Flags: review?(sphink)
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8928112 -
Flags: review?(sphink)
Updated•7 years ago
|
Attachment #8928111 -
Flags: review?(sphink) → review+
Comment 3•7 years ago
|
||
Comment on attachment 8928112 [details] [diff] [review] Bug 1409324 (part 2) - Add a new timer for nursery chunk allocation Review of attachment 8928112 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Nursery.cpp @@ +553,5 @@ > json.property("cur_capacity", previousGC.nurseryCapacity); > json.property("new_capacity", spaceToEnd()); > json.property("lazy_capacity", allocatedChunkCount() * ChunkSize); > + if (!timeInChunkAlloc_.IsZero()) > + json.property("time_in_chunk_alloc", timeInChunkAlloc_, json.MICROSECONDS); Personally, I'd call it "chunk_alloc_us", but it's up to you.
Attachment #8928112 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #3) > Comment on attachment 8928112 [details] [diff] [review] > Bug 1409324 (part 2) - Add a new timer for nursery chunk allocation > > Review of attachment 8928112 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/gc/Nursery.cpp > @@ +553,5 @@ > > json.property("cur_capacity", previousGC.nurseryCapacity); > > json.property("new_capacity", spaceToEnd()); > > json.property("lazy_capacity", allocatedChunkCount() * ChunkSize); > > + if (!timeInChunkAlloc_.IsZero()) > > + json.property("time_in_chunk_alloc", timeInChunkAlloc_, json.MICROSECONDS); > > Personally, I'd call it "chunk_alloc_us", but it's up to you. I think I'll rename it. I wanted a name that helped make it clear it wasn't actually part of GC now, since it's now in mutator time (but can still create pauses). Neither name does that well. Thanks.
Assignee | ||
Comment 5•7 years ago
|
||
Updated from review feedback, r+ carried forward.
Attachment #8928112 -
Attachment is obsolete: true
Attachment #8928406 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/05c80a2c4462 (part 1) - Remove old resize profiling from GCMinor. r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/cfa3672ccb30 (part 2) - Add a new timer for nursery chunk allocation r=sfink
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/05c80a2c4462 https://hg.mozilla.org/mozilla-central/rev/cfa3672ccb30
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•