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…
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.
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: --- → ?
Whiteboard: [fixed in fx-team][qa+]
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in fx-team][qa+] → [qa+]
Target Milestone: --- → Firefox 27
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: approval-mozilla-aurora? → approval-mozilla-aurora+
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
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-
You need to log in before you can comment on or make changes to this bug.