MEMORY_DISTRIBUTION_AMONG_CONTENT should only be recorded in the parent process

RESOLVED FIXED in Firefox 61

Status

()

P4
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: amiyaguchi, Assigned: joberts.ff, Mentored)

Tracking

unspecified
mozilla61
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

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

Attachments

(1 attachment)

Currently MEMORY_DISTRIBUTION_AMONG_CONTENT is recorded in the parent and content process. However, this value is only reported at the parent process.
Let's set this up as a good first bug, like bug 1442061

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_DISTRIBUTION_AMONG_CONTENT 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_DISTRIBUTION_AMONG_CONTENT 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
Hey Chris,
Can you assign this bug to me?

Best
Satya
Certainly. Please let me know if you have any questions.
Assignee: nobody → satyakonidala.us
Status: NEW → ASSIGNED
(Reporter)

Updated

a year ago
See Also: → bug 1441716

Comment 4

a year ago
cool. I will let you know if have any queries.
How are things coming along? Anything I can help with?
Flags: needinfo?(satyakonidala.us)

Comment 6

a year ago
The build took an awful lot of time :D . Will be sending the patch shortly. Just reading up a bit on mercurial on how to make one.
Flags: needinfo?(satyakonidala.us)
Be sure to reach out if you're stuck for more than 15min without making progress. #introduction and #developers on IRC are excellent resources for on-the-spot troubleshooting, and I'm available here if you don't mind waiting for answers :)
Flags: needinfo?(satyakonidala.us)

Comment 8

a year ago
Hi, 
When I try to issue the command "hg bzexport", I get an error saying "abort: Local changes found; refresh first!". Can you help me with this?
Flags: needinfo?(satyakonidala.us)

Comment 9

a year ago
(In reply to satya.ko from comment #8)
> Hi, 
> When I try to issue the command "hg bzexport", I get an error saying "abort:
> Local changes found; refresh first!". Can you help me with this?

I tried looking up on internet, but couldn't get any useful info on what to do.
I'm afraid that's something I haven't seen before. Are you able to ask on #introduction? It sounds as though you may need to update your local changes before you try exporting the revision to this bug.
Did you manage to resolve your hg issue? How are things going?
Flags: needinfo?(satyakonidala.us)

Comment 12

11 months ago
Unassigning from lack of activity.
Assignee: satyakonidala.us → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(satyakonidala.us)
(Assignee)

Comment 13

11 months ago
I would like to give this bug a try.  I've already built firefox, and found the histograms.json file that needs to be modified.  Before changing the source, I went to the about:telemetry to see if I could find the issue, however, the MEMORY_DISTRIBUTION_AMONG_CONTENT is not listed on the parent.  Am I missing something?

Comment 14

11 months ago
Thanks for volunteering! I have assigned the bug to you.

MEMORY_DISTRIBUTION_AMONG_CONTENT is a keyed histogram (which are kept separate from non-keyed histograms for historical reasons) so are you perhaps looking in the "Histograms" section instead?

Also, we only accumulate to it when there are content processes about. Try loading mozilla.org and firefox.com in a couple tabs and then check back?
Assignee: nobody → joberts.ff
Status: NEW → ASSIGNED
(Assignee)

Comment 16

11 months ago
Yes I was looking the the "Histograms" section.  I pushed the patch, and it's ready for review.

Comment 17

11 months ago
mozreview-review
Comment on attachment 8968389 [details]
Bug 1442441 - Removed the content notation from the MEMORY_DISTRIBUTION_AMONG_CONTENT prop's record_in_processes field;

https://reviewboard.mozilla.org/r/237088/#review243018

Thank you for your contribution. Excellent commit message.

I'll go ahead and push this to autoland which should mean it will land in Firefox Nightly 61 in the next day or so!

Do you have an idea for what you'll want your next contribution to be? If not, I might be able to help you find one if you let me know what sort of thing you'd like to do.
Attachment #8968389 - Flags: review?(chutten) → review+

Comment 18

11 months ago
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c973c46110b2
Removed the content notation from the MEMORY_DISTRIBUTION_AMONG_CONTENT prop's record_in_processes field; r=chutten

Comment 19

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c973c46110b2
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(Assignee)

Comment 20

11 months ago
(In reply to Chris H-C :chutten from comment #17)
> Comment on attachment 8968389 [details]
> Bug 1442441 - Removed the content notation from the
> MEMORY_DISTRIBUTION_AMONG_CONTENT prop's record_in_processes field;
> 
> https://reviewboard.mozilla.org/r/237088/#review243018
> 
> Thank you for your contribution. Excellent commit message.
> 
> I'll go ahead and push this to autoland which should mean it will land in
> Firefox Nightly 61 in the next day or so!
> 
> Do you have an idea for what you'll want your next contribution to be? If
> not, I might be able to help you find one if you let me know what sort of
> thing you'd like to do.

That's great I'm glad I could help out.

I would like to write some code in my next contribution. In my day job I write .Net Windows Form applications in C#.  So, If you please point me in a coding direction.

But before I move on to my next bug. What do i need to do on my working directory?  I would have to review the Mercurial information again, but I think I would run "hg up central" to get the latest... right?

Comment 21

11 months ago
Er, sorry. I'm not the right person to ask for mercurial help as I use git to work on mozilla-central[1]. However, I'm sure there's someone on IRC channel #introduction who will be able to help you out.

As for coding, let's peruse the list of mentored bugs we have kicking around in our component[2].

Nothing immediately jumps out at me. Maybe bug 1362953 if you're interested in refactoring our C++ core's thread safety to be more declarative by adding lock references? Actually, that may require the last few steps on bug 1430531 being completed first so the JS API is running under the protection of locks. On the JS side I just found bug 1369049 and made it mentored if you'd like some light work to get you started. It'll need a test.

What do you think?

[1]: https://github.com/glandium/git-cinnabar/wiki/Mozilla:-A-git-workflow-for-Gecko-development
[2]: https://georgf.github.io/bugzhub/#mentored
(Assignee)

Comment 22

11 months ago
Thanks, I'll take a look at those links. Git might work better for me.

I went ahead a requested bug 1369049 be assigned to me.
You need to log in before you can comment on or make changes to this bug.