Closed
Bug 1324051
Opened 8 years ago
Closed 8 years ago
Add telemetry for ActivityStream highlights query time
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect, P1)
Firefox for Android Graveyard
Awesomescreen
Tracking
(firefox53 fixed)
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: ahunt, Assigned: ahunt)
References
Details
(Whiteboard: [MobileAS])
Attachments
(2 files)
This will be useful for:
- Bug 1298786 - adds domain blacklist to highlights
- Bug 1312017 - optimise the highlights query
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ahunt
Iteration: --- → 1.11
Priority: -- → P1
Whiteboard: [MobileAS]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8819354 [details]
Bug 1324051 - Pre: fix constant name typo
https://reviewboard.mozilla.org/r/99176/#review99466
Attachment #8819354 -
Flags: review?(gkruglov) → review+
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8819355 [details]
Bug 1324051 - Add highlights histogram telemetry
https://reviewboard.mozilla.org/r/99178/#review99472
Attachment #8819355 -
Flags: review?(gkruglov) → review+
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3e7a0b5ee00
Pre: fix constant name typo r=Grisha
https://hg.mozilla.org/integration/autoland/rev/f93bcba13541
Add highlights histogram telemetry r=Grisha
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d3e7a0b5ee00
https://hg.mozilla.org/mozilla-central/rev/f93bcba13541
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment 7•8 years ago
|
||
Please note that all new probes require data collection review:
https://wiki.mozilla.org/Firefox/Data_Collection
Flags: needinfo?(ahunt)
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8819355 [details]
Bug 1324051 - Add highlights histogram telemetry
I'd like to request data collection review.
This probe measures the time taken for the Activity Stream highlights query. Activity Stream is still experimental and under active development, we would like to be able to track the performance impact caused by any changes to our highlights query. This query is used every time the new tab page is shown, it's important to minimise performance impacts to make sure Fennec is responsive.
(E.g. we will be making the highlights query more complicated in Bug 1298786, and want to make sure that change doesn't significantly impact performance. We are likely to fiddle with this query a lot more in the future too.)
Flags: needinfo?(ahunt)
Attachment #8819355 -
Flags: feedback?(liuche)
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8819355 [details]
Bug 1324051 - Add highlights histogram telemetry
https://reviewboard.mozilla.org/r/99178/#review100456
::: toolkit/components/telemetry/Histograms.json:6304
(Diff revision 1)
> "alert_emails": ["mobile-frontend@mozilla.com"],
> "bug_numbers": [1293790],
> "cpp_guard": "ANDROID"
> },
> + "FENNEC_ACTIVITY_STREAM_HIGHLIGHTS_LOADER_TIME_MS": {
> + "expires_in_version": "never",
lol I guess that this is already landed, but I would comment that I'd prefer if this was not "never". I'm fine with sticking it at something far in the future like 60, giving a full ride on Nightly to release + 2 cycles, but there should be an expiration.
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #9)
> lol I guess that this is already landed, but I would comment that I'd prefer
> if this was not "never". I'm fine with sticking it at something far in the
> future like 60, giving a full ride on Nightly to release + 2 cycles, but
> there should be an expiration.
I'll think about filing a followup bug to discuss this more formally - and I'm happy to revise this if needed.
Originally I'd just copied the Activity Stream topsites probe, which is also "never". However I think that is actually based on the old Topsites probe (FENNEC_TOPSITES_LOADER_TIME_MS), which also a "never".
I do imagine we'd want to keep these all forever though:
- These queries are run every time the new tab screen is shown (i.e. high user impact) - regressions should therefore be avoided since this is highly visible to pretty much all users, and repeatedly so, during normal use of the app. (In other words: a regression could make the app feel significantly slower to most users.)
- These queries could be impacted by any database changes: even unrelated schema changes could potentially slow down DB queries (at least that's what I've been told regarding changes in desktop's places - schema changes are avoided partly for performance reasons). Changes to the size of the DB could cause slowdowns (e.g. changes regarding how much history is synced, or changes wrt clearing of old history, etc.).
- We seem to make significant changes at least once a year, so we'd have to keep recreating this probe every time we do that.
In other words: it's really important to avoid regressions here, and it's fairly easy to cause regressions in changes that would otherwise seem unrelated - so we probably want to keep monitoring the performance forever.
On the other hand, it's likely the old topsites implementation will be removed at some point in the near future, so being reminded of its expiry would help ensure that it's removed in a timely fashion.
Comment 11•8 years ago
|
||
Hmmm, not entirely sure about whether telemetry is the right place to do perf regression, however, since there doesn't seem to be an easy alternative, I'll just keep this in mind for the future.
Updated•8 years ago
|
Attachment #8819355 -
Flags: feedback?(liuche)
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•