1442441 - MEMORY_DISTRIBUTION_AMONG_CONTENT should only be recorded in the parent process

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P4)

Description

[Anthony Miyaguchi [:amiyaguchi]]

Currently MEMORY_DISTRIBUTION_AMONG_CONTENT is recorded in the parent and content process. However, this value is only reported at the parent process.

Comment 1

[Chris H-C :chutten]

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.

Comment 2

[satya.ko]

Hey Chris,
Can you assign this bug to me?

Best
Satya

Comment 3

[Chris H-C :chutten]

Certainly. Please let me know if you have any questions.

Comment 4

[satya.ko]

cool. I will let you know if have any queries.

Comment 5

[Chris H-C :chutten]

How are things coming along? Anything I can help with?

Comment 6

[satya.ko]

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.

Comment 7

[Chris H-C :chutten]

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 :)

Comment 8

[satya.ko]

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?

Comment 9

[satya.ko]

(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.

Comment 10

[Chris H-C :chutten]

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.

Comment 11

[Chris H-C :chutten]

Did you manage to resolve your hg issue? How are things going?

Comment 12

[Chris H-C :chutten]

Unassigning from lack of activity.

Comment 13

[jason]

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

[Chris H-C :chutten]

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?

Comment 15

[jason]

Attachment: Bug 1442441 - Removed the content notation from the MEMORY_DISTRIBUTION_AMONG_CONTENT prop's record_in_processes field;

Review commit: https://reviewboard.mozilla.org/r/237088/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237088/

Comment 16

[jason]

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

Comment 17

[Chris H-C :chutten]

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.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c973c46110b2

Comment 20

[jason]

(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

[Chris H-C :chutten]

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

Comment 22

[jason]

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.