1829127 - Add telemetry needed to evaluate PHC

Status: RESOLVED FIXED

(Core :: Memory Allocator, task)

Description

[Paul Bone [:pbone]]

We should add telemetry to measure PHC's utilisation. I'd like to know:

• How many slots are allocated / freed / never used in each process.
• How much metadata memory PHC requires in each process
• How much fragmentation PHC causes in each process.
• The distributions of process lifetimes.
• Maybe the utilsation of PHC when a process terminates, did we ever use close to all the slots?
• If we ever fail an allocation because all the slots are used?
• How often we "reuse" a freed slot.

Comment 1

[Paul Bone [:pbone]]

(In reply to Paul Bone [:pbone] from comment #0)

We should add telemetry to measure PHC's utilisation. I'd like to know:

• How many slots are allocated / freed / never used in each process.
• How much metadata memory PHC requires in each process
• How much fragmentation PHC causes in each process.
• The distributions of process lifetimes.
• Maybe the utilsation of PHC when a process terminates, did we ever use close to all the slots?
• If we ever fail an allocation because all the slots are used?
• How often we "reuse" a freed slot.

For this bug we want to do only the first three. The others are lower priority and have their own Bug 1849121.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

For this bug we want to do only the first three.

I'm unclear on the value of the third one (fragmentation - as page size is not under your control), so I'd recommend to skip this to get it out of the door faster.

Comment 3

[Paul Bone [:pbone]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #2)

For this bug we want to do only the first three.

I'm unclear on the value of the third one (fragmentation - as page size is not under your control), so I'd recommend to skip this to get it out of the door faster.

it's indirectly under control because it's related to the number of allocated cells. The code in PHC.cpp is already in place to measure it for the memory reporter, it only needs telemetry added.

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1829127 - Add telemetry for PHC slop r=glandium

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1829127 - Add telemetry for PHC utilisation r=glandium

Depends on D192302

Comment 6

[Paul Bone [:pbone]]

Attachment: data-request.txt

Comment 7

[Roger Yang [:royang]]

Comment on attachment 9361156 [details]
data-request.txt

DATA COLLECTION REVIEW RESPONSE:

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, this collection is documented in its definitions files Histograms.json, Scalars.yaml, and/or Events.yaml and in the Probe Dictionary at https://probes.telemetry.mozilla.org.

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's settings.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

N/A, collection to be removed or renewed in 6 months

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical data

5. Is the data collection request for default-on or default-off?

Default on for all channels.

6. Does the instrumentation include the addition of any new identifiers?

No

7. Is the data collection covered by the existing Firefox privacy notice?

Yes

8. Does the data collection use a third-party collection tool?

No

Result: data-review+

Comment 8

[Paul Bone [:pbone]]

Hi Roger, could you do Chris' review on the patches also? I don't seem to be able to select your name in Phabricator.

Thanks.

Comment 9

[Roger Yang [:royang]]

Hi Paul, Sorry, I don't know that part of the code base enough to give you a review.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ecb0eac9c1b
Add telemetry for PHC slop r=glandium
https://hg.mozilla.org/integration/autoland/rev/fc1ab94b7ef4
Add telemetry for PHC utilisation r=glandium

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5ecb0eac9c1b
https://hg.mozilla.org/mozilla-central/rev/fc1ab94b7ef4

Comment 12

[Paul Bone [:pbone]]

Comment on attachment 9361129 [details]
Bug 1829127 - Add telemetry for PHC slop r=glandium

Beta/Release Uplift Approval Request

• User impact if declined: We're planning an experiment for PHC in Firefox 120. This telemetry will help us evaluate what tuning PHC might need. It's not critical for our experiment but it'd be very helpful
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): it's a smallish telemetry patch that has been running well in Nightly.
• String changes made/needed: None
• Is Android affected?: No

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9361129 [details]
Bug 1829127 - Add telemetry for PHC slop r=glandium

Approved for 120.0b9

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/d691d3212284
https://hg.mozilla.org/releases/mozilla-beta/rev/6c2a09cfbe3e