Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

MetricController.track & Event.wrapper may use non-trivial power: called frequently #21292

Closed
mcomella opened this issue Sep 14, 2021 · 5 comments
Assignees
Labels
Feature:Telemetry meta perf:P3 nice to have/meet (tracking only) performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Sep 14, 2021

MetricController.track calls the Event.wrapper method, which is a long chain of comparisons, twice to submit telemetry. To test the impact of these methods, I did a simple workflow:

  • click wiki homescreen
  • click url bar, nav amazon.com
  • click tabs tray, new tab
  • search "foobar" and click default search result

I added custom markers and took profiles:

In these profiles of this simple workflow, we see:

  • MetricController.track is called 19 times for a total duration of 57.3ms
  • Event.wrapper is called 36 times for a total of 24.3ms (I believe this is a subset of MetricController)
  • (caveat: these duration may be inflated because these calls are fast and clock granularity might not be good enough)
  • (note: I used window.filteredMarkers.map((m) => m.end - m.start).reduce((acc, i) => acc + i) to measure this)

These durations will be unnoticeable to the user, however, they will use battery. wrt Event.wrapper, this doesn't need to be a long chain of comparisons – perhaps by calling the Glean API directly and removing MetricController – so we can stop the users' device from doing work it doesn't need to. We should investigate MetricController for similar wins.

wrt prioritization, there may be larger gains we can do to improve battery life (e.g. #15921) but those are likely harder to implement so it could be worth addressing this issue.

Note: #19967 may also address this issue.

@mcomella mcomella added the performance Possible performance wins label Sep 14, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Sep 14, 2021
@mcomella mcomella changed the title GleanMetricsService.track & Event.wrapper may use non-trivial power: called frequently MetricController.track & Event.wrapper may use non-trivial power: called frequently Sep 14, 2021
@mcomella mcomella added the perf:P3 nice to have/meet (tracking only) label Sep 22, 2021
@mcarare mcarare self-assigned this Mar 2, 2022
@mcarare mcarare added Feature:Telemetry meta and removed needs:triage Issue needs triage labels Mar 2, 2022
@mcarare
Copy link
Contributor

mcarare commented Mar 3, 2022

This issue will serve as a meta. I will link here the issues handling the removal of the wrapper in separate components. This will make QA work easier and ensure we do not have regressions by tackling the whole work in smaller pieces.

mcarare added a commit to mcarare/fenix that referenced this issue May 2, 2022
mcarare added a commit to mcarare/fenix that referenced this issue May 3, 2022
@mcarare
Copy link
Contributor

mcarare commented May 3, 2022

This can be closed as all MetricController.track and Event.wrapper usages have been removed from the app.

@mcarare mcarare closed this as completed May 3, 2022
mcarare added a commit to mcarare/fenix that referenced this issue May 4, 2022
mcarare added a commit to mcarare/fenix that referenced this issue May 5, 2022
mcarare added a commit to mcarare/fenix that referenced this issue May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature:Telemetry meta perf:P3 nice to have/meet (tracking only) performance Possible performance wins
Projects
None yet
Development

No branches or pull requests

3 participants