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

RESOLVED FIXED

Status

Firefox OS
Gaia
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pdahiya, Assigned: pdahiya)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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
(Assignee)

Updated

3 years ago
Blocks: 1196586
(Assignee)

Updated

3 years ago
Assignee: nobody → pdahiya
Created attachment 8683214 [details] [review]
[gaia] punamdahiya:Bug1221650 > mozilla-b2g:master
(Assignee)

Comment 2

3 years ago
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    |
(Assignee)

Comment 3

3 years ago
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+
(Assignee)

Comment 5

3 years ago
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!
(Assignee)

Comment 6

3 years ago
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    |
(Assignee)

Comment 7

3 years ago
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)
(Assignee)

Comment 8

3 years ago
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+
(Assignee)

Comment 11

3 years ago
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.
(Assignee)

Comment 12

3 years ago
Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/7eb0ea62cdc713eb3109d4701ff959f1bba1ef92
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

3 years ago
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.