If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

minify CSS source to improve build-in apps Launch latency

RESOLVED WONTFIX

Status

Firefox OS
GonkIntegration
RESOLVED WONTFIX
5 years ago
3 years ago

People

(Reporter: gasolin, Assigned: gasolin)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
According to article 'Best Practices for Speeding Up Your Web Site' from yahoo
http://developer.yahoo.com/performance/rules.html#minify

minify js and css are good idea to improve load times.

Bug 837111 only minify the js source. Since CSS styles are always linked in header section(will block render process before downbloads are finished) and we heavily include lots of css styles in each app, it will be a help for performance.

expect result:

improve the webapp-optimize process with:

Minify all css in header to separate files (Since styles may overlap)

or 

aggregate all css in header to one file

based on further experiments.
(Assignee)

Comment 1

5 years ago
If css minify is in,
'copyBuildingBlock' process should be done in webapp-manifest instead of webapp-zip(which runs after webapp-optimize) to benifit from the css minification.
(Assignee)

Updated

5 years ago
blocking-b2g: --- → koi?
(Assignee)

Comment 2

5 years ago
I took email app for evaluation.

With customized css minify script from clear-css, 3 css files are squished into 1. 
Currently only contain project css files. There are 9 included css in /shared folder not been squished yet.


some test numbers (get with 'Show time to load' option in developer mode):

not optimized email app
923, 1042, 890, 890, 868, 893, 911

JS minify + css minify with linebreak
912, 878, 875, 925, 887

JS minify + css minify without linebreak
896, 884, 889, 901, 866, 855, 894, 938
(Assignee)

Comment 3

5 years ago
email without any optimize

1012 1291 1442 989 1660 973 1556 1009 972 975 avg: 1187

email + css optimize (3 files)

1008 1018 955 1019 994 1052 971 974 949 983 953 avg: 988

It's clear that css minify can help shorten the loadtime.
(Assignee)

Comment 4

5 years ago
https://github.com/mozilla-b2g/gaia/pull/8954

Currently the patch can handle simple css structure (which put into /style folder without subfolder)
I did some ad-hoc testing on the Settings app, just by concatenating (no minification) the styles for the app in one file and the shared styles in another one. This does improve the launch time.
(Assignee)

Comment 6

4 years ago
FYI, 

Based on 'Building faster websites' presentation in Google IO 2013 http://www.igvita.com/slides/2013/fluent-perfcourse.pdf

p76. states 

1. HTML is parsed incrementally
2. Rendering is blocked on CSS

which means

'Get css down to the client as fast as you can' might be a good idea
(Assignee)

Updated

4 years ago
Assignee: nobody → gasolin
(Assignee)

Updated

4 years ago
Summary: minify CSS source → minify CSS source to increase build-in apps performance
(Assignee)

Comment 7

4 years ago
Created attachment 759014 [details]
pull request redirect to github

modified clean.css https://github.com/GoalSmashers/clean-css to aggregate and minify css files in /style folder to style/gaia_build_<html_name>.css

compare to JS aggregated file which located in ./gaia_build_<html_name>.js, 
CSS aggregated styles must put within 'style/gaia_build_<html_name>.css' folder because its relative to image resources.

The patch exclude some apps (ex: communication) that has more complex folder structure.
Attachment #759014 - Flags: review?(timdream)
(Assignee)

Comment 8

4 years ago
It will help improve launch latency of bug 871827 (music), bug 871826 (email), bug 871821 (camera)
Summary: minify CSS source to increase build-in apps performance → minify CSS source to improve build-in apps Launch latency
Comment on attachment 759014 [details]
pull request redirect to github

Sorry for the delay; but I don't think I am capable of reviewing this part of the code.

One thing to note that is the way we adopt this external script; I am not sure what's the best approach, but commenting out dead code and leave a comment is definitely not the way to do it. 

That said, we couldn't take the code as-is for sure because we don't want the build process to be dependent on node.

/me running out of ideas here.
Attachment #759014 - Flags: review?(timdream)
Attachment #759014 - Flags: review?(poirot.alex)
Attachment #759014 - Flags: feedback+
(Assignee)

Comment 10

4 years ago
Tim, 

Currently I take the similar approach as our current JS minify procedure.

For reference there's no node dependency here since the clean.js is modified (removed wrapper, same approach as JS minify) and is runned by xulrunner.
(In reply to Fred Lin [:gasolin] from comment #10)
> For reference there's no node dependency here since the clean.js is modified
> (removed wrapper, same approach as JS minify) and is runned by xulrunner.

Yes, my question was about the proper way to copy-paste this npm script and use it on XULRunner. It is certainly NOT comment out code that gets our way.
(Assignee)

Comment 12

4 years ago
I agree current JS/CSS minify way is pretty hack... because they only minify partial resources.


dump some other general launch latency improvement ideas:

1. change the way we copy the resources in share/ folder, let them can be optimized in the same way (haven't figure out how to make this works well yet)

in build time or commit time
2. run image optimizer to down size the images without sacrifice quality.
3. integrate compass to squash separate images to CSS sprite
Comment on attachment 759014 [details]
pull request redirect to github

The pull request doesn't work:

+Services.scriptloader.loadSubScript('file:///' + GAIA_DIR +
+    '/build/clean.js?reload=' + new Date().getTime(), scope);
+const { clean } = scope; 

Here, `clean` is undefined, as clean.js exports `CleanCSS`.

Otherwise, I'm wondering if we are seing a placebo effect and if this is really worth it. Some people already tried stripping and merging css files and haven't seen much improvement. If your load timings were done with the current patch, that would confirm this.
Attachment #759014 - Flags: review?(poirot.alex) → review-
(In reply to Fred Lin [:gasolin] from comment #3)
> email without any optimize
> 
> 1012 1291 1442 989 1660 973 1556 1009 972 975 avg: 1187
> 
> email + css optimize (3 files)
> 
> 1008 1018 955 1019 994 1052 971 974 949 983 953 avg: 988
> 
> It's clear that css minify can help shorten the loadtime.

The result is not clear to me. Did the first run is without the JS/CSS Optimizer and the second with JS optimizer AND CSS optimizer?

If yes it would be nice to have results for CSS only.

In all cases I don't think its going to hurt.
(Assignee)

Comment 15

4 years ago
No first run is without the JS/CSS Optimizer and the second with CSS optimizer only.

The current PR can just comment out JS optimize and see the result with CSS optimizer only.

// optimize_aggregateJsResources(win.document, webapp, newFile);
(Assignee)

Comment 16

4 years ago
Comment on attachment 759014 [details]
pull request redirect to github

Thanks for review. Sorry I made mistake while changing the cleancss modification, which give you wrong impression about the result.

The above estimation is done by old implementation(April), since then I've update the cleancss and try to align the origin code, and not deliver it correctly.

I'd like to run some bench mark to make sure it does improve b2g app loadtime,
such as the pref-o-metric (https://datazilla.mozilla.org/b2g/) .

Do you know any tool that can do it locally?
Attachment #759014 - Flags: review?(poirot.alex)
Comment on attachment 759014 [details]
pull request redirect to github

The patch looks good, but I don't see a real performance gain.
On a Unagi device with recent gecko master and with your commit (a4787076f9d3eaec8737557ed30d607a8728bd23) or without it on the previous revision of your branch (8a74913b1175b852f02a90413cba0475cb73f45a), I see these "time to load" numbers:

### without - 8a74913b1175b852f02a90413cba0475cb73f45a

# settings:
1302 1295 1300 1292 1281 1296 1396 1289

# mail:
1156 1144 1093 1074 1141 1100 1067

### with - a4787076f9d3eaec8737557ed30d607a8728bd23

# settings:
1298 1266 1293 1286 1268

# mail:
1110 1073 1095 1065 1159 1073

Here is how I got these numbers:

$ git checkout $REVISION
$ GAIA_OPTIMIZE=1 make reset-gaia
* Launch Settings app to toggle "show time to load" setting
* Close the settings app
* Launch and close multiple times Settings and then Mail apps.

Do you get similar results with your current branch or am I doing something wrong somewhere?
Attachment #759014 - Flags: review-
(Assignee)

Comment 18

4 years ago
Thanks for review, I'd take another test with current patch and report the results.
(Assignee)

Comment 19

4 years ago
Alexandre is right...

The css optimization did not gain much performance gain after applied packaging(zip). I think the most gain in comment 3 is based on the packaging work?


comment out 461~478 (JS/CSS optimization), got:

1132, 1135, 1129, 1285, 1121, 1117, 1142, 1202, 1129, 1274

comment out 461~468 (JS optimization), got:

1127, 1130, 1557, 1092, 1261, 1115, 1138, 1104, 1272


reproduce steps:

install email app with following command:

$ GAIA_OPTIMIZE=1 make install-gaia APP=email

Main steps:

open email and watch load time.

close it, long press home button to close email app. Then open email again.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
Attachment #759014 - Flags: review?(poirot.alex)

Updated

4 years ago
blocking-b2g: koi? → ---
See Also: → bug 1048843
You need to log in before you can comment on or make changes to this bug.