Closed Bug 916316 Opened 11 years ago Closed 11 years ago

Speed up FHR date formatting

Categories

(Firefox Health Report Graveyard :: Client: Android, defect)

All
Android
defect
Not set
normal

Tracking

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

RESOLVED FIXED
Firefox 27
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

(Whiteboard: [qa+])

Attachments

(1 file)

Profiling suggests that this is 55% of document generation.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
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!
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+]
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
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in fx-team][qa+] → [qa+]
Target Milestone: --- → Firefox 27
tracking-fennec: ? → ---
[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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f963196e0062
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.
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-
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: