Speed up FHR date formatting

RESOLVED FIXED in Firefox 26

Status

Firefox Health Report
Client: Android
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

unspecified
Firefox 27
All
Android
Points:
---

Firefox Tracking Flags

(firefox24 wontfix, firefox25 wontfix, firefox26 fixed, firefox27 fixed)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Profiling suggests that this is 55% of document generation.
(Assignee)

Updated

5 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
https://github.com/mozilla-services/android-sync/pull/354

Review, plz, mcomella. Particularly awesome if you'd re-run your document benchmarking…
Flags: needinfo?(michael.l.comella)
r-: comments on Github. Working on those benchmarks. Going to bench with and without the memoization.
With memoization, 1 run (3 for 15000), Galaxy Nexus:

Event count | Time (millis)
    0           27
 5000         3421
10000         5902
15000         8611 (though I got 11025 on the first run and 8675 on the second)

Source is the same as bug 870171 comment 36.
Without memoization, 1 run, Galaxy Nexus:

Event count | Time (millis)
    0           28
 5000         3367
10000         5775
15000         8508

Source is the same as bug 870171 comment 36.

As per my comment in github, it looks like the memoization is unnecessary.
Flags: needinfo?(michael.l.comella)
For the record, these benchmarks should be compared against the old implementation, bug 870171 comment 36.
With DateFormatter, 1 run, Galaxy Nexus:

Event count | Time (millis)
    0           35
 5000         2693
10000         4863
15000         7408

Source is the same as bug 870171 comment 36.

Nice!
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/acf96abc7a89

Tracking 'cos this is a big perf win for FHR, and I'd like to uplift this after it's baked.
tracking-fennec: --- → ?
Flags: needinfo?(rnewman)
Whiteboard: [fixed in fx-team][qa+]
(Assignee)

Comment 8

5 years ago
Bustage fix:

https://hg.mozilla.org/integration/fx-team/rev/ca8a0d2f0f51
Flags: needinfo?(rnewman)
https://hg.mozilla.org/mozilla-central/rev/acf96abc7a89
https://hg.mozilla.org/mozilla-central/rev/ca8a0d2f0f51
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in fx-team][qa+] → [qa+]
Target Milestone: --- → Firefox 27
tracking-fennec: ? → ---
(Assignee)

Comment 10

5 years ago
Created attachment 810888 [details] [diff] [review]
Patch for Aurora. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Original landing of FHR.

User impact if declined: 
  Slower document generation, both in upload and viewing about:healthreport. This is a pretty big perf win.

Testing completed (on m-c, etc.): 
  Baking, Nightly is fine, unit tests.

Risk to taking this patch (and alternatives if risky): 
  Should be slim.

String or IDL/UUID changes made by this patch:
  None.
Attachment #810888 - Flags: review+
Attachment #810888 - Flags: approval-mozilla-aurora?
Attachment #810888 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/f963196e0062
status-firefox24: --- → affected
status-firefox25: --- → affected
status-firefox26: --- → fixed
Summary: Memoize date formatting → Speed up FHR date formatting
Did you intend to take this on beta too? If not, please set status-firefox25 to wontfix.
status-firefox24: affected → wontfix
status-firefox27: --- → fixed
(Assignee)

Comment 13

5 years ago
Comment on attachment 810888 [details] [diff] [review]
Patch for Aurora. v1

Given that we're in early days for beta (right?), and this is a big perf win for a pretty self-contained change, let's shoot for beta, too.
Attachment #810888 - Flags: approval-mozilla-beta?
Comment on attachment 810888 [details] [diff] [review]
Patch for Aurora. v1

Sadly doesn't meet Beta criteria. We sometimes take performance wins, but this doesn't appear to be user perceivable and we'd rather a patch of this size gets sufficient bake time.
Attachment #810888 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
status-firefox25: affected → wontfix
You need to log in before you can comment on or make changes to this bug.