Closed Bug 1380770 Opened 2 years ago Closed 2 years ago
Add minor GC reason to Minor
And nursery size, and promotion rate.
I haven't actually tried this out yet, but it seems worth getting feedback on. I appended '-kb' to specify units, which doesn't really match anything else and is somewhat arbitrary. I also am reporting the parts used to calculate the promotion rate, not just the rate itself, because the additional info seems potentially useful -- but normally the rate is the most interesting thing. I could add it in too, but it seems redundant. Maybe the rate plus one of the others, since you can infer the third but at least you'd have the rate? Ugh.
Attachment #8886472 - Flags: review?(jcoppeard)
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Comment on attachment 8886472 [details] [diff] [review] Add more info to GCMinor marker JSON Review of attachment 8886472 [details] [diff] [review]: ----------------------------------------------------------------- The additional info seems useful, but I'd say report the promotion rate too. It's clearer and that's more important than a little redundancy in my view. ::: js/src/gc/Nursery.cpp @@ +481,5 @@ > + > + json.property("reason", JS::gcreason::ExplainReason(minorGCTriggerReason_)); > + json.property("nursery-used-kb", nurseryUsedBytes_ / 1024); > + json.property("tenured-kb", tenuredBytes_ / 1024); > + json.property("nursery-kb", numChunks() * (ChunkSize >> 10)); I'd just use bytes and let the whoever gets this data format it how they like. ::: js/src/gc/Nursery.h @@ +352,5 @@ > ProfileDurations profileDurations_; > ProfileDurations totalDurations_; > uint64_t minorGcCount_; > + int nurseryUsedBytes_; > + int tenuredBytes_; These should be size_t. If they relate to the last collection they should be named to indicate that - maybe lastUsedByes or something.
Attachment #8886472 - Flags: review?(jcoppeard) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/6b05837ec165 Add more info to GCMinor marker JSON, r=jonco
You need to log in before you can comment on or make changes to this bug.