Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

CSS minification in Neutrino 8 increases build duration by 3-4x #678

Closed
edmorley opened this issue Jan 20, 2018 · 10 comments · Fixed by #934
Closed

CSS minification in Neutrino 8 increases build duration by 3-4x #678

edmorley opened this issue Jan 20, 2018 · 10 comments · Fixed by #934
Assignees
Milestone

Comments

@edmorley
Copy link
Member

After updating to Neutrino 8, I'm seeing 10m30s build times, which are reduced down to 2m55s if I disable CSS minification like so:

  minify: {
    style: false,
  },

Is this kind of increase expected? It was pretty surprising to me, and makes me wonder if CSS minification should actually be changed to be opt-in rather than opt-out?

With CSS minification enabled:

$ node ./node_modules/neutrino/bin/neutrino build
√ Building project completed
Hash: 4ed83966259c4c536c62
Version: webpack 3.10.0
Time: 623700ms
                                                     Asset       Size                        Chunks                    Chunk Names
                         userguide.929e879dfd1ed42c7c67.js    5.23 kB                     userguide  [emitted]         userguide
  fontawesome-webfont.674f50d287a8c48dc19ba404d20fe713.eot     166 kB                                [emitted]
  fontawesome-webfont.b06871f281fee6b241d60582ae9369b9.ttf     166 kB                                [emitted]
fontawesome-webfont.af7ae505a9eed503f8b8e6982036873e.woff2    77.2 kB                                [emitted]
 fontawesome-webfont.fee66e712a8a08eef5805a46892932ad.woff      98 kB                                [emitted]
          dancing_cat.fa5552a59ba8993ec5c8e21d5704c275.gif    59.4 kB                                [emitted]
      treeherder-logo.3df97cffaebaf01c06e1e7dd87cc2c82.png    8.65 kB                                [emitted]
                     failureviewer.011689b66dd2ed9ebebc.js     359 kB                 failureviewer  [emitted]  [big]  failureviewer
                             index.8ff3629d4ed5d6361583.js     571 kB                         index  [emitted]  [big]  index
                         logviewer.8df78a2afbe749e6cb92.js     406 kB                     logviewer  [emitted]  [big]  logviewer
                              perf.610f29614eef24c549d4.js     535 kB                          perf  [emitted]  [big]  perf
                           runtime.7c4bb4b84423b7ce1b68.js    1.75 kB                       runtime  [emitted]         runtime
                          testview.792db22a7e491778b195.js     206 kB                      testview  [emitted]         testview
  fontawesome-webfont.912ec66d7572ff821749319396470bde.svg     444 kB                                [emitted]  [big]
                            vendor.58fbaf5eedaa0f8e001d.js    1.83 MB                        vendor  [emitted]  [big]  vendor
                index.6c4d60ee89e0d511c0a77c1753067e36.css     198 kB                  index, index  [emitted]         index, index
                 perf.689feed5ba666905ecb7e37cdacf5a79.css     180 kB                    perf, perf  [emitted]         perf, perf
            logviewer.5d73e70ed27d2c1fdb3bea4aa5e6ee3d.css     163 kB          logviewer, logviewer  [emitted]         logviewer, logviewer
        failureviewer.b11339f9deb9b7afd8ec159dc2dd38b7.css     162 kB  failureviewer, failureviewer  [emitted]         failureviewer, failureviewer
            userguide.f10930ddacff6d8e89f4efd4fa971baa.css     175 kB          userguide, userguide  [emitted]         userguide, userguide
             testview.b93fe3f0445b59fa9de673e2847e3100.css     159 kB            testview, testview  [emitted]         testview, testview
                                                index.html  583 bytes                                [emitted]
                                                 perf.html  581 bytes                                [emitted]
                                            logviewer.html  591 bytes                                [emitted]
                                        failureviewer.html  599 bytes                                [emitted]
                                            userguide.html  591 bytes                                [emitted]
                                             testview.html  589 bytes                                [emitted]
Done in 628.54s.

With it disabled:

$ node ./node_modules/neutrino/bin/neutrino build
√ Building project completed
Hash: e0ba159a6394854d8953
Version: webpack 3.10.0
Time: 169977ms
                                                     Asset       Size                        Chunks                    Chunk Names
                         userguide.422f8bf94776e00d83cc.js    5.23 kB                     userguide  [emitted]         userguide
  fontawesome-webfont.674f50d287a8c48dc19ba404d20fe713.eot     166 kB                                [emitted]
  fontawesome-webfont.b06871f281fee6b241d60582ae9369b9.ttf     166 kB                                [emitted]
fontawesome-webfont.af7ae505a9eed503f8b8e6982036873e.woff2    77.2 kB                                [emitted]
 fontawesome-webfont.fee66e712a8a08eef5805a46892932ad.woff      98 kB                                [emitted]
          dancing_cat.fa5552a59ba8993ec5c8e21d5704c275.gif    59.4 kB                                [emitted]
      treeherder-logo.3df97cffaebaf01c06e1e7dd87cc2c82.png    8.65 kB                                [emitted]
                     failureviewer.248a8468ad37c059edbe.js     359 kB                 failureviewer  [emitted]  [big]  failureviewer
                             index.85bda5a02338088b25fe.js     571 kB                         index  [emitted]  [big]  index
                         logviewer.f0a6adf7eb99184fd288.js     406 kB                     logviewer  [emitted]  [big]  logviewer
                              perf.d98484d8e05b52de9608.js     535 kB                          perf  [emitted]  [big]  perf
                           runtime.63cc0b7d712cdf209489.js    1.75 kB                       runtime  [emitted]         runtime
                          testview.7c51dfd849fc0afb8dd3.js     206 kB                      testview  [emitted]         testview
  fontawesome-webfont.912ec66d7572ff821749319396470bde.svg     444 kB                                [emitted]  [big]
                            vendor.58fbaf5eedaa0f8e001d.js    1.83 MB                        vendor  [emitted]  [big]  vendor
                index.6c4d60ee89e0d511c0a77c1753067e36.css     250 kB                  index, index  [emitted]  [big]  index, index
                 perf.689feed5ba666905ecb7e37cdacf5a79.css     228 kB                    perf, perf  [emitted]         perf, perf
            logviewer.5d73e70ed27d2c1fdb3bea4aa5e6ee3d.css     207 kB          logviewer, logviewer  [emitted]         logviewer, logviewer
        failureviewer.b11339f9deb9b7afd8ec159dc2dd38b7.css     206 kB  failureviewer, failureviewer  [emitted]         failureviewer, failureviewer
            userguide.f10930ddacff6d8e89f4efd4fa971baa.css     220 kB          userguide, userguide  [emitted]         userguide, userguide
             testview.b93fe3f0445b59fa9de673e2847e3100.css     168 kB            testview, testview  [emitted]         testview, testview
                                                index.html  583 bytes                                [emitted]
                                                 perf.html  581 bytes                                [emitted]
                                            logviewer.html  591 bytes                                [emitted]
                                        failureviewer.html  599 bytes                                [emitted]
                                            userguide.html  591 bytes                                [emitted]
                                             testview.html  589 bytes                                [emitted]
Done in 174.59s.
@jaridmargolin
Copy link
Contributor

@edmorley I was seeing similar issues with build times (even prior to v8). I switched from Babili, which is what is set by default in Neutrino, to Uglify, and saw a substantial decrease in build times.

@edmorley
Copy link
Member Author

edmorley commented Jan 21, 2018

JS minification is also a significant proportion of the build above (with that turned off the build drops from 2m55s to ~20s), albeit not related to the issue here. In another issue it would be good to:

  • compare babel-minify with uglifyjs2 - now that the latter supports ES6 (via uglify-es), which was the blocker previously
  • experiment with adjusting the babel-minify/uglifyjs2 settings to see if there is a better minify speed vs size tradeoff than the defaults (perhaps neutrino or babel-minify needs a "mode: {fastest, fast, ...}" that makes the choices simple?)
  • add docs around "speeding up the build" with links to other posts people have written (eg https://slack.engineering/keep-webpack-fast-a-field-guide-for-better-build-performance-f56a5995e8f1), ways to profile, and tips for quick wins (eg turning off minify)

@jaridmargolin
Copy link
Contributor

@edmorley I have no idea how I missed reference to CSS, which is referenced both in the title and three times in the description 😂

I'll open an additional issue.

@timkelty
Copy link
Contributor

@edmorley That seems crazy…maybe try disabling some specific cssnano optimizations to narrow it down?

  minify: {
    style: {
        cssProcessorOptions: { discardDuplicates: false },
    },
  },

@edmorley
Copy link
Member Author

Using @neutrinojs/react from master with the same project as the OP, I get:

  • fully warm cache (two builds in a row):
    • CSS minification disabled: 37s
    • CSS minification enabled: 62s
  • cold cache (rm -rf dist/ node_modules/.cache/):
    • CSS minification disabled: 84s
    • CSS minification enabled: 107s

The size reduction varied between 9KB and 30KB depending on entrypoint:
https://emorley.pastebin.mozilla.org/9085151

...so for our case probably still not enough to warrant an almost doubling in warm-cache build times (and possible issues from any CSS minification bugs). A quicker win for us would be to just switch from importing font-awesome.css to font-awesome.min.css (37KB -> 31KB), or even better to move from Font Awesome 4 (webfonts with CSS) to Font Awesome 5's SVG with JS + treeshaking support.

However I realise for smaller projects the build time difference may not be as significant, plus they may not be importing the minified versions of bootstrap/... so have more to benefit. (For what it's worth, I had the Reactstrap docs updated to promote minified bootstrap.)

It's worth noting since the OP:

  • Neutrino is now using webpack 4 (with its improved chunking logic that reduces overall duplication), plus has switched to mini-css-extract-plugin (which is more fit for purpose than extract-text-webpack-plugin, and I think may avoid some of the previous CSS duplication that is what made optimize-css-assets-webpack-plugin a necessity)
  • the project in question has:
    • unified the bootstrap imports so they all import the minified file (previously a mixture of minified and not - presumably resulting in double the content to minify)
    • updated from bootstrap 4.0.0-beta.2 to 4.1.1

@edmorley
Copy link
Member Author

edmorley commented May 11, 2018

For comparison, there is only 11s difference in the warm-cache build when JS minification is enabled/disabled, even though there is a lot more JS to minify. From a quick glance it appears that optimize-css-assets-webpack-plugin isn't caching anything, and why such a penalty.

Hopefully if/when webpack adds native support for CSS minfication in v5/whenever, support for caching/parallism will be added to optimize-css-assets-webpack-plugin, similar to how it was for uglifyjs-webpack-plugin.

@artola
Copy link

artola commented May 15, 2018

@edmorley I would like to migrate CRA to neutrino, the result must be quite similar. After some attempts with the basic config in a preset on top of @n/react I started to compare the build time.

  • CRA with min = 22secs
  • N with minify = 50secs
  • N without minify = 17secs

Then I tried just enabling 1 at a time:

  • with minify.babel= 44secs
  • with minify.style = 21secs

What is this big time difference? Somebody has a side by side comparison to CRA?

@edmorley
Copy link
Member Author

edmorley commented May 16, 2018

@artola, hi!

Neutrino 8 uses webpack 3 (same as latest CRA) but babel-minify for JS minification, whereas CRA uses uglify-es. babel-minify is quite a bit slower (in part due to it not having uglify-es' parallisation or caching functionality) - and is only used since in the past uglify-es did not exist, and uglify-js (note the one letter difference) does not support ES6.

However for Neutrino 9 (the version on master, not yet released), we've switched to uglify-es too, plus have upgraded to webpack 4, which is much faster. (For some of Mozilla's apps, warm cache build times that used to take 7 minutes with Neutrino 8, now only take 30 seconds with Neutrino master!)

I'm hoping we can release an alpha/beta of Neutrino 9 soon.

@edmorley edmorley added this to the v9 milestone Jun 1, 2018
@edmorley
Copy link
Member Author

edmorley commented Jun 6, 2018

Another issue I've found with style minification, is that if I toggle it on/off (using minify: { style: false } etc), the hash in the CSS filename does not change even though we're correctly using [contenthash].

This appears to be due to optimize-css-assets-webpack-plugin not supporting [contenthash] correctly, which is a pretty serious issue - eg:
NMFR/optimize-css-assets-webpack-plugin#56

As such, I think we should stop enabling style minification by default, and possibly disconnect it from the web preset entirely (people can always use the preset alongside the web preset).

For example - using a react project created using @neutrino/create-project from master, the hashes can be seen to stay the same even though the filesize (and thus content) changed...

Style minification enabled (currently the default):

$ webpack --mode production
Hash: 8f80d96957844e87c145
Version: webpack 4.11.1
Time: 1654ms
Built at: 2018-06-06 18:48:27
              Asset       Size  Chunks             Chunk Names
runtime.62cb599a.js   1.42 KiB       0  [emitted]  runtime
      1.5aef7ef9.js   99.8 KiB       1  [emitted]
 index.96536b32.css   18 bytes       2  [emitted]  index
  index.26d30bbc.js  625 bytes       2  [emitted]  index
         index.html  518 bytes          [emitted]
 [6] ./src/App.jsx 458 bytes {2} [built]
 [9] ./src/App.css 39 bytes {2} [built]
[11] (webpack)/buildin/harmony-module.js 573 bytes {1} [built]
[20] ./src/index.jsx 160 bytes {2} [built]
[21] multi ./src/index 28 bytes {2} [built]
    + 17 hidden modules
Child html-webpack-plugin for "index.html":
     1 asset
    [0] (webpack)/buildin/module.js 497 bytes {0} [built]
    [1] (webpack)/buildin/global.js 489 bytes {0} [built]
        + 2 hidden modules
Child mini-css-extract-plugin ../neutrino-dev/node_modules/css-loader/index.js??ref--6-1!src/App.css:
    [1] ../neutrino-dev/node_modules/css-loader??ref--6-1!./src/App.css 204 bytes {0} [built]
        + 1 hidden module
Done in 4.82s.

Style minification disabled:

$ webpack --mode production
Hash: 8f80d96957844e87c145
Version: webpack 4.11.1
Time: 1554ms
Built at: 2018-06-06 18:54:58
              Asset       Size  Chunks             Chunk Names
runtime.62cb599a.js   1.42 KiB       0  [emitted]  runtime
      1.5aef7ef9.js   99.8 KiB       1  [emitted]
 index.96536b32.css   27 bytes       2  [emitted]  index
  index.26d30bbc.js  625 bytes       2  [emitted]  index
         index.html  518 bytes          [emitted]
 [6] ./src/App.jsx 458 bytes {2} [built]
 [9] ./src/App.css 39 bytes {2} [built]
[11] (webpack)/buildin/harmony-module.js 573 bytes {1} [built]
[20] ./src/index.jsx 160 bytes {2} [built]
[21] multi ./src/index 28 bytes {2} [built]
    + 17 hidden modules
Child html-webpack-plugin for "index.html":
     1 asset
    [0] (webpack)/buildin/module.js 497 bytes {0} [built]
    [1] (webpack)/buildin/global.js 489 bytes {0} [built]
        + 2 hidden modules
Child mini-css-extract-plugin ../neutrino-dev/node_modules/css-loader/index.js??ref--6-1!src/App.css:
    [1] ../neutrino-dev/node_modules/css-loader??ref--6-1!./src/App.css 204 bytes {0} [built]
        + 1 hidden module
Done in 4.27s.

(btw both are warm-cache times, in case looking at the durations)

@eliperelman
Copy link
Member

Agreed.

edmorley added a commit that referenced this issue Jun 6, 2018
Since:
* `optimize-css-assets-webpack-plugin` doesn't support `[contenthash]`
  properly, meaning that filename hashes aren't always updated even
  when the file content has changed.
* `optimize-css-assets-webpack-plugin` doesn't support caching or
  parallelisation (unlike `uglifyjs-webpack-plugin`), meaning that
  for large projects, style minification can double the build time.
* the file size reduction is often not significant (given that many
  stylesheets are already available in minified form, eg bootstrap),
  and so not worth the correctness/performance penalty.
* it's easy enough for people to add `@neutrinojs/style-minify` to
  their project if they still want minification.

webpack 5+ will likely be including CSS minification out of the box,
and so in the future hopefully these issues should be resolved and
so we can re-enable it by default.

Fixes #678.
Refs #713.
@edmorley edmorley self-assigned this Jul 13, 2018
SteveShaffer added a commit to veritone/veritone-app-boilerplate-react that referenced this issue Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

5 participants