1568514 - Add telemetry to report memory usage for auto-hyphenation

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement, P3)

Description

[Jonathan Kew [:jfkthame]]

Bug 1567437 added about:memory reporting for runtime memory usage by hyphenation resources. This reveals the memory impact that a content process suffers when a site using hyphens:auto for any given language.

However, while this lets us see the memory footprint of loading any given language's resource, it doesn't tell us how often these resources are actually being used in the wild. The vast majority of content doesn't currently use hyphens:auto, so work to reduce the memory footprint will be relevant only to the subset of content that is using the feature.

It would be useful to have telemetry for per-content-process hyphenation memory usage, or something like that.

Given that reporting the memory used by hyphenation resources could reveal (or at least give a clue to) the language(s) of the content that a user is viewing, we'll need to be aware of the privacy implications here.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Data collection review request

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1568514 - Add telemetry histograms to report hyphenation memory footprint and resource load times.

Comment 3

[Jonathan Kew [:jfkthame]]

Here's a proposed patch that would add a histogram for hyphenation data memory. Waiting on response to the data-review request before seeking review here.

Comment 4

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Comment on attachment 9080579 [details]
Data collection review request

Taking this review since chutten will be out for the next several days. I see that you have a concern that this probe could leak to telemetry whether a user visited a page with content in a particular human language during a browser session; I assert that is not sensitive enough to warrant additional review as category 3 data. Looking at Histograms.json, I see that we collect content language explicitly in a couple of other feature contexts.

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, Histograms.json.

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

Yes, the Firefox telemetry opt-out.

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

n/a, temporary collection.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** 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.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

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

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

Yes; jfkthame is responsible for making a decision about whether to renew the collection.

9) Does the data collection use a third-party collection tool?

No.

Comment 5

[Jonathan Kew [:jfkthame]]

Thanks for the quick data-review!

:heycam, I'm in two minds how best to report the memory usage here: should we record one histogram entry for each resource that's loaded, or record the total for all resources that the process has loaded to date? If we do that, we'll in effect be double-counting the earlier resources when a new one is loaded, as the total will be cumulative but we'll have already recorded an entry for the earlier amount. But perhaps that doesn't much matter. Any preference?

(Initially I was intending to simply report the total at process shutdown time, but not every process -- especially on Android! -- gets to shut down cleanly, so it's not clear to me how easily we can do that. Recording the memory each time a resource is loaded seemed likely to be more reliable.)

Comment 6

[Cameron McCormack (:heycam)]

If we want to know how much memory we would be saving, then I think we'd want to record how much memory each content process ends up using for hyphenation data before it's shut down. Reading through https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/main-ping.html I'm not certain that reporting a single value for a content process is possible, since the session gets reset at least every 24 h.

Reporting a new histogram value each time a new dictionary gets loaded seems like it would skew the results. Using scalars lets us overwrite a previous value, as we load each dictionary, but still gets reset each 24 h I think.

Do we expect the values reported for mobile users to be significantly different? If not, then it may be fine to rely only on Desktop reported values, if that lets us report a single value per content process.

Might be worth getting some feedback from someone on the Telemetry team on the best way to get the data we need?

Comment 7

[Jonathan Kew [:jfkthame]]

I looked into using a scalar here, updating it each time a new resource is loaded, but AFAICS that doesn't work well here because we really want a separate value for each content process, but the telemetry ping just contains a single value for "content process". So it'll report the most recently-set value, but discards any value(s) that may have been set by other content processes. A histogram works better here as it enables each content process to contribute its own value.

Chris, am I overlooking something here? Do you have any suggestions for the best way to report the usage of each content process, given that (AFAIK) we can't readily rely on reporting at content-process shutdown? On android, in particular, I believe the process is liable to be unceremoniously killed without getting any opportunity to report its data.

Comment 8

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

I see you've mentioned Android, so just a note that telemetry is not collected from release-channel Fennec users, and aiui Fennec releases are following the ESR branch. (I'm not sure what the Fenix story is, but maybe you are!)

Comment 9

[Jonathan Kew [:jfkthame]]

OK, that's interesting (I wasn't aware that we don't collect Fennec release-channel data). So in practice I don't think we're going to get any meaningful amount of Android data for now, probably not until Fenix ships.

Given that, maybe collecting data at content-process shutdown time would be reasonable, at least for now. If we call Telemetry::Accumulate or something like that from within LayoutStatics::Shutdown, for example, can you confirm whether that'll get reported successfully?

Comment 10

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

I'm not sure! That sounds like a great question for a client telemetry engineer; I'll let chutten answer once he's back from holiday.

Comment 11

[Jonathan Kew [:jfkthame]]

In addition to reporting the amount of memory used, I'd like to report the time taken to load the hyphenation resource. On my local machine (reasonably fast MacBook Pro), this varies from a couple of milliseconds for some of the really small, simple resources to 120-140ms or so for the largest resources. This is a one-time cost per hyphenated language for any given process -- once the resource is loaded, we hold onto it -- but it does block the initial reflow of the first page that uses a given resource and therefore needs to load it.

It would be nice to know how frequently users hit significant delays here, as a re-architected hyphenation library where the resources are "precompiled" to a runtime-friendly format and mapped directly from disk has the potential to largely eliminate this cost.

I don't think adding telemetry for this (suggested histogram: HYPHENATION_LOAD_TIME, reporting the elapsed time in milliseconds taken by hnj_hyphen_load) makes any material difference to the data-review issues here. It'll tend to be somewhat correlated with the memory footprint anyway (larger resources take longer to load), scaled according to CPU and file-reading speed on the user's system. Tim, do you have any concerns with this or need more details?

Comment 12

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

No concerns, but the collection will need data-review.

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: Data collection review request (updated)

This is an updated version of the original data-review form, covering both proposed pieces of data (memory and time).

Comment 14

[Chris H-C :chutten]

(In reply to Jonathan Kew (:jfkthame) from comment #9)

OK, that's interesting (I wasn't aware that we don't collect Fennec release-channel data). So in practice I don't think we're going to get any meaningful amount of Android data for now, probably not until Fenix ships.

Given that, maybe collecting data at content-process shutdown time would be reasonable, at least for now. If we call Telemetry::Accumulate or something like that from within LayoutStatics::Shutdown, for example, can you confirm whether that'll get reported successfully?

If we're undergoing an orderly xpcom shutdown (ie, not crashing (Desktop only)) Telemetry recorded late in shutdown (before or synchronously within profile-before-change) should be reported just fine.

Comment 15

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Comment on attachment 9083375 [details]
Data collection review request (updated)

Thanks for your patience and for the thoughtful answers!

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, Histograms.json.

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

Yes, the Firefox telemetry opt-out.

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

n/a, temporary collection.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** 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.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

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

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

Yes; jfkthame is responsible for making a decision about whether to renew the collection.

9) Does the data collection use a third-party collection tool?

No.

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to Chris H-C :chutten from comment #14)

If we're undergoing an orderly xpcom shutdown (ie, not crashing (Desktop only)) Telemetry recorded late in shutdown (before or synchronously within profile-before-change) should be reported just fine.

Hmm, it looks like profile-before-change is only sent in the parent process, I want each content process to record its own histogram value before it goes away (as well as the parent, in the probably-rare case that it uses hyphenation).

Apparently we have a content-child-shutdown notification that looks like it should work for this.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47f90b1619ae
Add telemetry histograms to report hyphenation memory footprint and resource load times. r=heycam

Comment 18

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/47f90b1619ae