Remove content process in memory_total histogram's "record_in_processes"

RESOLVED FIXED in Firefox 61

Status

()

enhancement
P4
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sunahsuh, Assigned: kylemsguy, Mentored)

Tracking

(Blocks 1 bug)

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment)

Seems like another likely candidate for a histogram that's only recorded in the parent process:
https://mzl.la/2HPnS4t
Indeed it is, good catch. Let's set this up as a mentored bug.

To help Mozilla out with this bug, here's the steps:

0) Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
1) Download and build the Firefox source code: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
- If you have any problems, please ask on IRC (https://wiki.mozilla.org/Irc) in the #introduction channel. They're there to help you get started.
- You can also read the Developer Guide, which has answers to most development questions: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
2) Start working on this bug. You'll be editing toolkit/components/telemetry/Histograms.json to remove the "content" notation from the MEMORY_TOTAL probe's record_in_processes field. 
- If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the #telemetry channel on IRC (https://wiki.mozilla.org/Irc) most hours of most days.
3) Build your change with `mach build` and test your change with `mach test toolkit/components/telemetry/tests/`. You should also run your newly-built Firefox with `mach run` and check the page `about:telemetry` (type it into your address bar) to make sure MEMORY_TOTAL is still recorded.
4) Submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code.
- Here's the guide: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
5) After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!
6) ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.
Mentor: chutten
Whiteboard: [good first bug][lang=js]
Priority: -- → P4

Comment 2

a year ago
I can take this bug too if possible
(In reply to Videet Singhai from comment #2)
> I can take this bug too if possible

We may have a "good second bug" for you after bug 1442402 instead of having you work on a second "good first bug." So let's take it one bug at a time :)
Assignee

Comment 4

a year ago
Hello, I'd like to take this bug.
Okay, I have assigned this bug to you. Please let me know if you have any questions.
Assignee: nobody → kylemsguy
Status: NEW → ASSIGNED
See Also: → 1441716
Assignee

Comment 6

a year ago
Hello, I've made the change, run the appropriate tests, and submitted the commit: https://reviewboard-hg.mozilla.org/gecko/rev/eecbf329b278
I am not familiar with reviewboard-hg.mozilla.org. Could you resubmit this either as an attached patch (Splinter review) or using MozReview? The "How to Submit a Patch" guide I linked to in the instructions recommends mozreview: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
:keylemsguy, are you still working on this? Do you need any help?
Flags: needinfo?(kylemsguy)
Assignee

Comment 9

a year ago
Hey sorry, I haven't had the chance to work on this, but I'll give it another shot right now.

When I submitted the previous commit, I used `hg push review`, as was suggested in the MozReview documentation. I'm not sure why it's not appearing in MozReview.

As for submitting a patch on Bugzilla, I've only just found the link to attach a patch. I'll resubmit my patch via that method.
Flags: needinfo?(kylemsguy)
Did it have a properly-formatted commit message? Maybe it was missing a bug number and reviewer notation so it wouldn't be able to link it back here or email me...

The top line of a commit message should have the number, a short summary, and a r?. Something like:

bug 1442061 - Remove "content" from "record_in_processes" of MEMORY_TOTAL r?chutten
Assignee

Comment 12

a year ago
This was the commit message from my previous attempt: 

Bug 1442061 - Remove content process in memory_total histogram's "record_in_processes" r?chutten
Comment on attachment 8962501 [details] [diff] [review]
0001-Bug-1442061-Remove-content-process-in-memory_total-h.patch

Review of attachment 8962501 [details] [diff] [review]:
-----------------------------------------------------------------

All else fails, splinter review.

Thank you for your contribution!
Attachment #8962501 - Flags: review?(chutten) → review+
I'll mark this for checkin.

Do you need some help finding something to work on next? We have a list of mentored Telemetry bugs over here, if you'd like to take a look: https://georgf.github.io/bugzhub/#mentored
Keywords: checkin-needed

Comment 15

a year ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38646fa333aa
Remove content process in memory_total histogram's "record_in_processes" r=chutten
Keywords: checkin-needed

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/38646fa333aa
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.