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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

Comment 2

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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 3

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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!

Comment 4

[David Flanagan [:djf]]

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...

Comment 5

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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!

Comment 6

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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 7

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

Comment on attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master

Setting review flag on updated patch. Thanks!

Comment 8

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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 9

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

Profiler results on BuildId 20151106 

Without patch:
https://people.mozilla.org/~bgirard/cleopatra/#report=da39f8b8c2238f269634375427eb4db5cc1b082e&javascriptOnly=true&selection=0,744

With patch:
https://people.mozilla.org/~bgirard/cleopatra/#report=2052cd2e6239248dfcf977d71dee579f65aa32af&javascriptOnly=true&selection=0,753

Comment 10

[David Flanagan [:djf]]

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!

Comment 11

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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.

Comment 12

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/7eb0ea62cdc713eb3109d4701ff959f1bba1ef92

Comment 13

[Punam Dahiya [:pdahiya] (OOO until 3/31)]

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!!