Closed Bug 1221650 Opened 6 years ago Closed 6 years ago

[Gallery] - Lazyload gaia-header.js to improve app startup time

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pdahiya, Assigned: pdahiya)

References

Details

Attachments

(1 file)

On running profiler on gallery app, gaia-header.js takes a substantial time at app startup. As gallery app, first screen doesn't display gaia-header, its safe to lazy load gaia-header.js.

https://people.mozilla.org/~bgirard/cleopatra/#report=62db6b7cd65b1b106d41bd76f93fcd3fa31dfd6d&javascriptOnly=true&selection=0,735
Blocks: 1196586
Assignee: nobody → pdahiya
Raptor results on latest master flame-kk 512, gallery app 30 runs

without patch
| Metric                | Mean     | Median  | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | ------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 775.233  | 783     | 637    | 822    | 33.611  | 787.261   |
| navigationInteractive | 776.833  | 784.500 | 638    | 823    | 33.566  | 788.845   |
| visuallyLoaded        | 1166.200 | 1171    | 1035   | 1227   | 32.791  | 1177.934  |
| contentInteractive    | 1166.633 | 1171    | 1036   | 1228   | 32.657  | 1178.320  |
| mediaEnumerated       | 1508.867 | 1523    | 1339   | 1574   | 57.651  | 1529.497  |
| fullyLoaded           | 9136.867 | 9104    | 8559   | 9562   | 216.011 | 9214.165  |
| uss                   | 15.235   | 15.232  | 15.102 | 15.422 | 0.053   | 15.254    |
| pss                   | 19.297   | 19.294  | 19.170 | 19.497 | 0.053   | 19.317    |
| rss                   | 35.429   | 35.428  | 35.305 | 35.633 | 0.053   | 35.448    |


with patch

| Metric                | Mean     | Median   | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | -------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 700.467  | 699      | 607    | 753    | 30.347  | 711.326   |
| navigationInteractive | 702.067  | 700      | 608    | 755    | 30.292  | 712.906   |
| visuallyLoaded        | 1140.800 | 1137.500 | 1093   | 1224   | 29.938  | 1151.513  |
| contentInteractive    | 1141.133 | 1138     | 1093   | 1224   | 29.920  | 1151.840  |
| mediaEnumerated       | 1581.067 | 1582     | 1512   | 1670   | 35.395  | 1593.733  |
| fullyLoaded           | 9114.567 | 9097.500 | 8871   | 9479   | 146.978 | 9167.162  |
| pss                   | 19.264   | 19.263   | 19.124 | 19.499 | 0.056   | 19.284    |
| uss                   | 15.195   | 15.195   | 15.059 | 15.410 | 0.053   | 15.214    |
| rss                   | 35.398   | 35.402   | 35.254 | 35.645 | 0.058   | 35.419    |
Comment on attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

Hi David

Attaching patch that lazyload gaia-header.js and shows performance gain of ~26ms. Please review.

Thanks!
Attachment #8683214 - Flags: review?(dflanagan)
Comment on attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

r+, but please address the comments on github.

I wonder why the speedup is larger at the navigationLoaded mark than at the contentLoaded mark. I suppose this might mean that we're bumping up against hard limits on how fast IndexedDB can start up...
Attachment #8683214 - Flags: review?(dflanagan) → review+
Thanks David for review, I have addressed comments in github. I will update patch after moving 'contentInteractive' performance mark and update bug with numbers from raptor. Thanks!
Updated patch by moving performance.mark('contentInteractive') after the select and thumbnail event handler are registered. Here are the numbers on today's build flame-kk

without patch

| Metric                | Mean     | Median   | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | -------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 752      | 754.500  | 668    | 807    | 36.905  | 774.874   |
| navigationInteractive | 753.500  | 755.500  | 669    | 808    | 37.092  | 776.490   |
| visuallyLoaded        | 1232.700 | 1228.500 | 1163   | 1279   | 32.885  | 1253.082  |
| contentInteractive    | 1233     | 1229.500 | 1163   | 1279   | 32.784  | 1253.320  |
| mediaEnumerated       | 1573.900 | 1575.500 | 1500   | 1621   | 35.422  | 1595.855  |
| fullyLoaded           | 9075.700 | 9068.500 | 8911   | 9312   | 139.516 | 9162.173  |
| pss                   | 19.315   | 19.316   | 19.260 | 19.430 | 0.048   | 19.345    |
| rss                   | 35.483   | 35.488   | 35.430 | 35.598 | 0.047   | 35.512    |
| uss                   | 15.221   | 15.227   | 15.164 | 15.328 | 0.045   | 15.249    |

With patch:

| Metric                | Mean     | Median   | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | -------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 691.800  | 698      | 575    | 750    | 36.465  | 707.781   |
| navigationInteractive | 693.200  | 699.500  | 576    | 751    | 36.482  | 709.189   |
| visuallyLoaded        | 1217.200 | 1217     | 1148   | 1287   | 34.458  | 1232.302  |
| contentInteractive    | 1217.900 | 1218     | 1149   | 1288   | 34.544  | 1233.040  |
| mediaEnumerated       | 1655.200 | 1650.500 | 1581   | 1743   | 38.906  | 1672.251  |
| fullyLoaded           | 9306.400 | 9297.500 | 8972   | 10155  | 271.205 | 9425.261  |
| uss                   | 15.117   | 15.145   | 14.914 | 15.379 | 0.112   | 15.166    |
| pss                   | 19.210   | 19.238   | 19.005 | 19.483 | 0.114   | 19.260    |
| rss                   | 35.379   | 35.408   | 35.172 | 35.656 | 0.114   | 35.429    |
Comment on attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

Setting review flag on updated patch. Thanks!
Attachment #8683214 - Flags: review+ → review?(dflanagan)
Updated numbers from today's build BuilId 20150006030214 flame-kk 30 runs. With patch we are seeing consistent perf numbers improved by  26 - 36 ms.

with patch

| Metric                | Mean     | Median   | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | -------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 641.067  | 644.500  | 528    | 683    | 29.061  | 651.466   |
| navigationInteractive | 642.467  | 646      | 530    | 684    | 28.962  | 652.830   |
| visuallyLoaded        | 1064.100 | 1060.500 | 922    | 1181   | 47.159  | 1080.976  |
| contentInteractive    | 1065.400 | 1061     | 926    | 1181   | 46.407  | 1082.006  |
| mediaEnumerated       | 1522.933 | 1526.500 | 1415   | 1614   | 46.621  | 1539.616  |
| fullyLoaded           | 9079.467 | 9065.500 | 8850   | 9365   | 115.114 | 9120.660  |
| uss                   | 15.216   | 15.213   | 15.129 | 15.543 | 0.068   | 15.241    |
| rss                   | 35.483   | 35.478   | 35.395 | 35.828 | 0.071   | 35.508    |
| pss                   | 19.301   | 19.297   | 19.212 | 19.641 | 0.070   | 19.327    |

Without patch:
| Metric                | Mean     | Median   | Min    | Max    | StdDev  | 95% Bound |
| --------------------- | -------- | -------- | ------ | ------ | ------- | --------- |
| navigationLoaded      | 723.233  | 723.500  | 572    | 799    | 36.004  | 736.117   |
| navigationInteractive | 724.500  | 724.500  | 573    | 800    | 36.038  | 737.396   |
| visuallyLoaded        | 1107.767 | 1106     | 1000   | 1179   | 31.493  | 1119.036  |
| contentInteractive    | 1108.167 | 1106.500 | 1001   | 1180   | 31.403  | 1119.404  |
| mediaEnumerated       | 1457.100 | 1454.500 | 1317   | 1577   | 58.601  | 1478.070  |
| fullyLoaded           | 9022.467 | 9001     | 8552   | 9412   | 153.025 | 9077.226  |
| uss                   | 15.266   | 15.262   | 15.152 | 15.551 | 0.073   | 15.292    |
| rss                   | 35.486   | 35.482   | 35.379 | 35.785 | 0.074   | 35.513    |
| pss                   | 19.338   | 19.337   | 19.224 | 19.635 | 0.075   | 19.365    |
Comment on attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

Looks good, but again see github. If you're going to move the event handler registrations and contentInteractive mark, I'd actually move them into the lazy loader callback so they don't happen until the headers are actually loaded. Or don't move them at all.

Thanks for figuring out that open.html was making the header load!
Attachment #8683214 - Flags: review?(dflanagan) → review+
Thanks David for review. I am keeping scope of this patch to lazy-load gaia-header.js and keeping event handler registrations and contentInteractive mark code as it was. I will land the patch once tests are done running on treeherder.
Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/7eb0ea62cdc713eb3109d4701ff959f1bba1ef92
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Results on raptor.mozilla.org on build after landing the patch (2015-11-10 10:55) shows visually loaded time come down from 1102 ms to 1000ms. Yay!!
You need to log in before you can comment on or make changes to this bug.