Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow compilation of SCSS files in Rails apps #1671

Closed
kevindew opened this issue Dec 3, 2019 · 27 comments
Closed

Slow compilation of SCSS files in Rails apps #1671

kevindew opened this issue Dec 3, 2019 · 27 comments
Assignees
Labels
epic Epics are used in planning project boards to group related stories

Comments

@kevindew
Copy link
Member

kevindew commented Dec 3, 2019

Hello 👋 from GOV.UK Publishing. We've been having some problems with speed of CSS compilation of GOV.UK apps for a while and after doing some detective work we've traced that a large volume of the time is due to importing files from govuk-frontend.

Backstory

GOV.UK apps use the Rails asset pipline to transpile SCSS to CSS. They mostly continue to use the deprecated Ruby SASS library, but are likely to start switching to sassc in the forseeable future. Nearly all GOV.UK apps use the govuk_publishing_components gem which pulls in the govuk-frontend node module and imports all components.

Gradually over the last few months or years we've been seeing the time that is needed to compile assets for GOV.UK has increased and once we switched to a docker based development environment we started to investigate as it seemed abnormally poor. Developers are finding that frequently an app won't seem to work the first time you try run it locally as the CSS could be taking more than a minute to compile. We did start to wonder why other Rails devs weren't going crazy about the slowness of CSS precompilation as it felt increasingly broken.

Looking into this further we found that most of the time was spent around the process of using @import to include files. Looking at a particular app, Finder Frontend, we could see that 7054 files were being imported, despite less than 400 distinct files being used, and every one of these files requires processing which seemed to be where the majority of the time was spent. Following this further we could see that from the point where we import all govuk-frontend components was where we were seeing the big increase in imported files.

Some basic benchmarks from my MBP 2015 on time to generate Finder Frontend's application.css file:

Environment SASS Cache Time (s)
govuk-docker Ruby SASS Cold 59.983
govuk-docker Ruby SASS Warm 0.228
govuk-docker sassc Cold 32.693
govuk-docker sassc Warm 17.478
Local machine Ruby SASS Cold 31.926
Local machine Ruby SASS Warm 0.063
Local machine sassc Cold 11.805
Local machine sassc Warm 2.413

Cause and effect

The problem of slow CSS compilation for GOV.UK seems to stem from the @import approach used in govuk-frontend and how this can affect Rails apps.

If we follow govuk/all.scss we import "settings/all" (>= 13 files), "tools/all" (>= 9 files), "helpers/all" (>= 12 files). Then for each component the same files are then imported. As sass executes each @import whether or not it's been included before this can exponentially increase the amount of work done. Some basic maths would have the 30 components adding up to at least 384 files imported but I think the number ends up being quite significantly greater.

We're theorising that the sheer quantity of imported files affects the Rails asset pipeline particularly badly as everyone one of these files is checked to see if it uses any Rails specific functions (asset-url, asset-path). It could well be paired with IO operation pain too, as it probably searches a whole bunch of directories for every single file.

I put together some isolated test cases to show the effects in a Rails app free from the rest of the GOV.UK infrastructure. As a basis for comparison I used a version of govuk-frontend I put together that was optimised for the all.scss file where all other imports were removed.

Some benchmarks were:

SASS Cache Time (s)
sassc Cold 7.494180
sassc Warm 1.667473
Ruby SASS Cold 15.556202
Ruby SASS Warm 0.007234

Where we can see this performs relatively consistently with Finder Frontend app, just clearly has a lot less to do being isolated.

And compared to the version with duplicate @import calls removed:

SASS Cache Time (s) Decrease
sassc Cold 0.552520 92.6 %
sassc Warm 0.007781 99.5 %
Ruby SASS Cold 2.913921 81.3 %
Ruby SASS Warm 0.005758 20.4 %

Here we can see quite how dramatic the effects of @import can be.

I did ponder whether things would be quicker using webpacker with Rails. There I saw it take ~5s to compile with @imports duplicated and ~2.5s with them removed. So it seemed difference is still significant here but as the scale is much reduced the impact is less. I'll also add that I really didn't put much time into setting up webpacker so these numbers could be flawed.

Finally, I tried running the sassc command line utility to see if this is slow I found that it took 0.27s to compile with @imports duplicated against 0.21s with them removed. Which isn't a very significant difference.

Impact on GOV.UK

Some of the effects this has had on GOV.UK have been:

  • Apps never seem to work on first request for local dev, as Rails hangs waiting for assets to preload. Unaccustomed developers assume the app is broken, abort it and ask for help
  • First test was frequently failing on apps due to timeout. We switched to running a manual pre-compile before tests to resolve it.
  • Switching to sassc is concerning because of the heavy performance impact on even a warm cache.

In general we seem to have ended up with a perfect storm on GOV.UK by having some apps needing to precompile multiple files with the same contents, importing all of govuk-frontend and then switching to the slower docker environment for development.

We're currently pondering about switching away from the approach of including all govuk-frontend components which would likely offer some performance improvement. Though we'd still be paying a reasonable performance penalty for duplicate @import calls.

Although we could see performance improve a reasonable amount by switching to use webpacker with Rails this seems like a long way off for GOV.UK given the quantity of apps and the amount of existing asset-pipeline based code we have. We're still a reasonable distance from being able to migrate one app to webpacker

Things we're wondering about

  • Whether something is broken in sassc-rails for the warm cache performance to perform so badly
  • Quite what is going on with each import that impacts time so badly
  • @use seems the ideal solution for resolving this problem but it's not available on libsass.
  • Whether other teams on different stacks are experiencing same performance issues
  • Whether the govuk-frontend style of @import usage is common or not - I notice it's not an approach tachyons or bootstrap use - as to whether wider world is affected.

It'd be great if we can try work out if there are things that can be done to reduce this problem. Although aspects of this seem quite specific Rails it does seem to be a problem that is likely to affect other environments (as multiple execution is a property of @import) and is likely to only increase in severity as more components are added.

@36degrees 36degrees added the awaiting triage Needs triaging by team label Dec 4, 2019
@NickColley NickColley added Effort: days ⚠️ high priority Used by the team when triaging labels Dec 4, 2019
@NickColley NickColley removed the awaiting triage Needs triaging by team label Dec 4, 2019
@kr8n3r
Copy link

kr8n3r commented Dec 6, 2019

worth noting that both of those rely on variables being there implicitly, which works for them, as a user you never import just one component.

govuk-frontend enabled importing of individual components specifically to allow services to gradually move their apps to govuk-frontend. because of this, component partials needed to import all the things they need for successful compile

might we worth checking with Dfe team about webpacker performance if they observed any issues

https://github.com/DFE-Digital/govuk-rails-boilerplate

@tvararu
Copy link

tvararu commented Dec 10, 2019

Hello! Some stats from DfE, on the current master branch on apply-for-teacher-training:

Hackintosh, i7 7700K, 4 cores, ~2628ms over 5 builds.

Webpack log
➜  apply-for-postgraduate-teacher-training git:(master) bin/webpack
Hash: 55c3218c512c0aee5fb4
Version: webpack 4.41.2
Time: 2628ms
Built at: 10/12/2019 10:59:49
                                                   Asset       Size                    Chunks                         Chunk Names
                            css/application-2f317a4d.css    133 KiB               application  [emitted] [immutable]  application
                        css/application-2f317a4d.css.map    199 KiB               application  [emitted] [dev]        application
                  js/application-e4a051a7091304d2dcc2.js    165 KiB               application  [emitted] [immutable]  application
              js/application-e4a051a7091304d2dcc2.js.map    181 KiB               application  [emitted] [dev]        application
                js/cookie-banner-4949dcd8eb6aabc2f06a.js   5.84 KiB             cookie-banner  [emitted] [immutable]  cookie-banner
            js/cookie-banner-4949dcd8eb6aabc2f06a.js.map   5.83 KiB             cookie-banner  [emitted] [dev]        cookie-banner
     js/nationality-autocomplete-547e90be06a23e3ef4eb.js   60.5 KiB  nationality-autocomplete  [emitted] [immutable]  nationality-autocomplete
 js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map   69.7 KiB  nationality-autocomplete  [emitted] [dev]        nationality-autocomplete
                                           manifest.json   2.75 KiB                            [emitted]
            media/fonts/bold-affa96571d-v2-affa9657.woff   39.9 KiB                            [emitted]
           media/fonts/bold-b542beb274-v2-b542beb2.woff2   30.7 KiB                            [emitted]
          media/fonts/light-94a07e06a1-v2-94a07e06.woff2   32.6 KiB                            [emitted]
           media/fonts/light-f591b13f7d-v2-f591b13f.woff   42.4 KiB                            [emitted]
                       media/images/favicon-de7abc52.ico   6.17 KiB                            [emitted]
media/images/govuk-apple-touch-icon-152x152-40846d46.png   2.79 KiB                            [emitted]
media/images/govuk-apple-touch-icon-167x167-f636cb7d.png   4.13 KiB                            [emitted]
media/images/govuk-apple-touch-icon-180x180-a0f7e1b7.png   3.42 KiB                            [emitted]
        media/images/govuk-apple-touch-icon-3c45daa1.png   2.81 KiB                            [emitted]
                media/images/govuk-crest-2x-57a05afc.png   8.68 KiB                            [emitted]
                   media/images/govuk-crest-bcd5768b.png    3.5 KiB                            [emitted]
          media/images/govuk-logotype-crown-b5b9956b.png  952 bytes                            [emitted]
               media/images/govuk-mask-icon-2afd125b.svg   3.01 KiB                            [emitted]
         media/images/govuk-opengraph-image-f86f1d0d.png     15 KiB                            [emitted]
Entrypoint application = css/application-2f317a4d.css js/application-e4a051a7091304d2dcc2.js css/application-2f317a4d.css.map js/application-e4a051a7091304d2dcc2.js.map
Entrypoint cookie-banner = js/cookie-banner-4949dcd8eb6aabc2f06a.js js/cookie-banner-4949dcd8eb6aabc2f06a.js.map
Entrypoint nationality-autocomplete = js/nationality-autocomplete-547e90be06a23e3ef4eb.js js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map
[./app/frontend/packs/application.js] 513 bytes {application} [built]
[./app/frontend/packs/cookie-banner.js] 1.8 KiB {cookie-banner} {application} [built]
[./app/frontend/packs/nationality-autocomplete.js] 1 KiB {nationality-autocomplete} {application} [built]
[./app/frontend/styles/application.scss] 39 bytes {application} [built]
[./node_modules/govuk-frontend/govuk/assets sync recursive ^\.\/.*$] ./node_modules/govuk-frontend/govuk/assets sync ^\.\/.*$ 1010 bytes {application} [built]
[./node_modules/webpack/buildin/global.js] (webpack)/buildin/global.js 878 bytes {application} [built]
[./node_modules/webpack/buildin/module.js] (webpack)/buildin/module.js 552 bytes {application} {nationality-autocomplete} [built]
    + 19 hidden modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--6-1!node_modules/postcss-loader/src/index.js??ref--6-2!node_modules/accessible-autocomplete/dist/accessible-autocomplete.min.css:
    Entrypoint mini-css-extract-plugin = *
       2 modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--7-1!node_modules/postcss-loader/src/index.js??ref--7-2!node_modules/sass-loader/dist/cjs.js??ref--7-3!app/frontend/styles/application.scss:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/postcss-loader/src/index.js?!./node_modules/sass-loader/dist/cjs.js?!./app/frontend/styles/application.scss] ./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss 383 KiB {mini-css-extract-plugin} [built] [2 warnings]
        + 8 hidden modules
WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4424:9) start value has mixed support, consider using flex-start instead

WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4589:9) start value has mixed support, consider using flex-start instead

MacBook Pro 13" 2018, i7 8559U, 4 cores, ~3168ms over 5 builds.

Webpack log
Hash: 55c3218c512c0aee5fb4
Version: webpack 4.41.2
Time: 3168ms
Built at: 12/10/2019 11:04:51 AM
                                                   Asset       Size                    Chunks                         Chunk Names
                            css/application-2f317a4d.css    133 KiB               application  [emitted] [immutable]  application
                        css/application-2f317a4d.css.map    199 KiB               application  [emitted] [dev]        application
                  js/application-e4a051a7091304d2dcc2.js    165 KiB               application  [emitted] [immutable]  application
              js/application-e4a051a7091304d2dcc2.js.map    181 KiB               application  [emitted] [dev]        application
                js/cookie-banner-4949dcd8eb6aabc2f06a.js   5.84 KiB             cookie-banner  [emitted] [immutable]  cookie-banner
            js/cookie-banner-4949dcd8eb6aabc2f06a.js.map   5.83 KiB             cookie-banner  [emitted] [dev]        cookie-banner
     js/nationality-autocomplete-547e90be06a23e3ef4eb.js   60.5 KiB  nationality-autocomplete  [emitted] [immutable]  nationality-autocomplete
 js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map   69.7 KiB  nationality-autocomplete  [emitted] [dev]        nationality-autocomplete
                                           manifest.json   2.75 KiB                            [emitted]
            media/fonts/bold-affa96571d-v2-affa9657.woff   39.9 KiB                            [emitted]
           media/fonts/bold-b542beb274-v2-b542beb2.woff2   30.7 KiB                            [emitted]
          media/fonts/light-94a07e06a1-v2-94a07e06.woff2   32.6 KiB                            [emitted]
           media/fonts/light-f591b13f7d-v2-f591b13f.woff   42.4 KiB                            [emitted]
                       media/images/favicon-de7abc52.ico   6.17 KiB                            [emitted]
media/images/govuk-apple-touch-icon-152x152-40846d46.png   2.79 KiB                            [emitted]
media/images/govuk-apple-touch-icon-167x167-f636cb7d.png   4.13 KiB                            [emitted]
media/images/govuk-apple-touch-icon-180x180-a0f7e1b7.png   3.42 KiB                            [emitted]
        media/images/govuk-apple-touch-icon-3c45daa1.png   2.81 KiB                            [emitted]
                media/images/govuk-crest-2x-57a05afc.png   8.68 KiB                            [emitted]
                   media/images/govuk-crest-bcd5768b.png    3.5 KiB                            [emitted]
          media/images/govuk-logotype-crown-b5b9956b.png  952 bytes                            [emitted]
               media/images/govuk-mask-icon-2afd125b.svg   3.01 KiB                            [emitted]
         media/images/govuk-opengraph-image-f86f1d0d.png     15 KiB                            [emitted]
Entrypoint application = css/application-2f317a4d.css js/application-e4a051a7091304d2dcc2.js css/application-2f317a4d.css.map js/application-e4a051a7091304d2dcc2.js.map
Entrypoint cookie-banner = js/cookie-banner-4949dcd8eb6aabc2f06a.js js/cookie-banner-4949dcd8eb6aabc2f06a.js.map
Entrypoint nationality-autocomplete = js/nationality-autocomplete-547e90be06a23e3ef4eb.js js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map
[./app/frontend/packs/application.js] 513 bytes {application} [built]
[./app/frontend/packs/cookie-banner.js] 1.8 KiB {cookie-banner} {application} [built]
[./app/frontend/packs/nationality-autocomplete.js] 1 KiB {nationality-autocomplete} {application} [built]
[./app/frontend/styles/application.scss] 39 bytes {application} [built]
[./node_modules/govuk-frontend/govuk/assets sync recursive ^\.\/.*$] ./node_modules/govuk-frontend/govuk/assets sync ^\.\/.*$ 1010 bytes {application} [built]
[./node_modules/webpack/buildin/global.js] (webpack)/buildin/global.js 878 bytes {application} [built]
[./node_modules/webpack/buildin/module.js] (webpack)/buildin/module.js 552 bytes {application} {nationality-autocomplete} [built]
    + 19 hidden modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--6-1!node_modules/postcss-loader/src/index.js??ref--6-2!node_modules/accessible-autocomplete/dist/accessible-autocomplete.min.css:
    Entrypoint mini-css-extract-plugin = *
       2 modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--7-1!node_modules/postcss-loader/src/index.js??ref--7-2!node_modules/sass-loader/dist/cjs.js??ref--7-3!app/frontend/styles/application.scss:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/postcss-loader/src/index.js?!./node_modules/sass-loader/dist/cjs.js?!./app/frontend/styles/application.scss] ./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss 383 KiB {mini-css-extract-plugin} [built] [2 warnings]
        + 8 hidden modules
WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4424:9) start value has mixed support, consider using flex-start instead

WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4589:9) start value has mixed support, consider using flex-start instead

@kevindew
Copy link
Member Author

Thanks @tvararu and hello 👋

Would you be able to see if it makes a difference to your build time if you use a govuk-frontend build without imports (e.g. https://github.com/kevindew/govuk-frontend-rails-performance/blob/ada4f4c00ae0d5ef93343d6989d9d4e34ea2d382/package.json#L9). I think that'll work for you as you use import all but it is govuk-frontend 3.2 rather than 3.4.

@aliuk2012
Copy link
Contributor

I remember when I worked on Finder-frontend almost a year ago it was painfully slow. It was the first time that I've had noticeable issues with sass compilation.

@kevindew have you tried removing the @import 'govuk_publishing_components/all_components' to see if it's any quicker?

@kevindew
Copy link
Member Author

I remember when I worked on Finder-frontend almost a year ago it was painfully slow. It was the first time that I've had noticeable issues with sass compilation.

@kevindew have you tried removing the @import 'govuk_publishing_components/all_components' to see if it's any quicker?

Yes, we tried that a long time ago and found it made a huge difference. We hadn't quite traced the issues down to govuk-frontend at that point so haven't tried it in a while.

As govuk_publishing_components pulls in govuk-frontend dependencies at various points (for example: https://github.com/alphagov/govuk_publishing_components/blob/cd014428a873588701c05e25d93f57d9bed46f80/app/assets/stylesheets/govuk_publishing_components/components/_back-link.scss#L1) we likely compound the performance issue further by causing further cascades of @import

@tvararu
Copy link

tvararu commented Dec 13, 2019

@kevindew that does make it faster!

➜  apply-for-postgraduate-teacher-training git:(master) ✗ gd
diff --git a/package.json b/package.json
index b2076302..dc209d43 100644
--- a/package.json
+++ b/package.json
@@ -4,7 +4,7 @@
   "dependencies": {
     "@rails/webpacker": "^4.2.2",
     "accessible-autocomplete": "^2.0.1",
-    "govuk-frontend": "^3.4.0"
+    "govuk-frontend": "https://github.com/kevindew/govuk-frontend-import-test"
   },
   "devDependencies": {
     "webpack-dev-server": "^3.9.0"
diff --git a/yarn.lock b/yarn.lock
index e68089fa..28dc426c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3078,10 +3078,9 @@ globule@^1.0.0:
     lodash "~4.17.10"
     minimatch "~3.0.2"

-govuk-frontend@^3.4.0:
-  version "3.4.0"
-  resolved "https://registry.yarnpkg.com/govuk-frontend/-/govuk-frontend-3.4.0.tgz#f6694245fbec2dd476d8e161c36be2a4ea785ce3"
-  integrity sha512-rmYPtcCtWgz92QBejYwOnfSxbPGYfvSruLwB4CBk/yJtySHRY0whG1e2/iFRRSj0pMx1Bu+zh/IqCTo+84hbFw==
+"govuk-frontend@https://github.com/kevindew/govuk-frontend-import-test":
+  version "3.2.0"
+  resolved "https://github.com/kevindew/govuk-frontend-import-test#6ea8703f269686fed99c72d066f1414c45ebb1c6"

 graceful-fs@^4.1.11, graceful-fs@^4.1.15, graceful-fs@^4.1.2:
   version "4.2.0"

Results in:

Hackintosh, i7 7700K, 4 cores, ~1475ms over 5 builds.

Webpack log
➜  apply-for-postgraduate-teacher-training git:(master) ✗ bin/webpack
Hash: 1b1c3c730dbd9f7be511
Version: webpack 4.41.2
Time: 1475ms
Built at: 13/12/2019 17:27:00
                                                   Asset       Size                    Chunks                         Chunk Names
                            css/application-8d28032c.css    136 KiB               application  [emitted] [immutable]  application
                        css/application-8d28032c.css.map    332 KiB               application  [emitted] [dev]        application
                  js/application-b3edc771a19e339c7d07.js    165 KiB               application  [emitted] [immutable]  application
              js/application-b3edc771a19e339c7d07.js.map    181 KiB               application  [emitted] [dev]        application
                js/cookie-banner-4949dcd8eb6aabc2f06a.js   5.84 KiB             cookie-banner  [emitted] [immutable]  cookie-banner
            js/cookie-banner-4949dcd8eb6aabc2f06a.js.map   5.83 KiB             cookie-banner  [emitted] [dev]        cookie-banner
     js/nationality-autocomplete-547e90be06a23e3ef4eb.js   60.5 KiB  nationality-autocomplete  [emitted] [immutable]  nationality-autocomplete
 js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map   69.7 KiB  nationality-autocomplete  [emitted] [dev]        nationality-autocomplete
                                           manifest.json   2.75 KiB                            [emitted]
            media/fonts/bold-affa96571d-v2-affa9657.woff   39.9 KiB                            [emitted]
           media/fonts/bold-b542beb274-v2-b542beb2.woff2   30.7 KiB                            [emitted]
          media/fonts/light-94a07e06a1-v2-94a07e06.woff2   32.6 KiB                            [emitted]
           media/fonts/light-f591b13f7d-v2-f591b13f.woff   42.4 KiB                            [emitted]
                       media/images/favicon-de7abc52.ico   6.17 KiB                            [emitted]
media/images/govuk-apple-touch-icon-152x152-40846d46.png   2.79 KiB                            [emitted]
media/images/govuk-apple-touch-icon-167x167-f636cb7d.png   4.13 KiB                            [emitted]
media/images/govuk-apple-touch-icon-180x180-a0f7e1b7.png   3.42 KiB                            [emitted]
        media/images/govuk-apple-touch-icon-3c45daa1.png   2.81 KiB                            [emitted]
                media/images/govuk-crest-2x-57a05afc.png   8.68 KiB                            [emitted]
                   media/images/govuk-crest-bcd5768b.png    3.5 KiB                            [emitted]
          media/images/govuk-logotype-crown-b5b9956b.png  952 bytes                            [emitted]
               media/images/govuk-mask-icon-2afd125b.svg   3.01 KiB                            [emitted]
         media/images/govuk-opengraph-image-f86f1d0d.png     15 KiB                            [emitted]
Entrypoint application = css/application-8d28032c.css js/application-b3edc771a19e339c7d07.js css/application-8d28032c.css.map js/application-b3edc771a19e339c7d07.js.map
Entrypoint cookie-banner = js/cookie-banner-4949dcd8eb6aabc2f06a.js js/cookie-banner-4949dcd8eb6aabc2f06a.js.map
Entrypoint nationality-autocomplete = js/nationality-autocomplete-547e90be06a23e3ef4eb.js js/nationality-autocomplete-547e90be06a23e3ef4eb.js.map
[./app/frontend/packs/application.js] 513 bytes {application} [built]
[./app/frontend/packs/cookie-banner.js] 1.8 KiB {cookie-banner} {application} [built]
[./app/frontend/packs/nationality-autocomplete.js] 1 KiB {nationality-autocomplete} {application} [built]
[./app/frontend/styles/application.scss] 39 bytes {application} [built]
[./node_modules/govuk-frontend/govuk/assets sync recursive ^\.\/.*$] ./node_modules/govuk-frontend/govuk/assets sync ^\.\/.*$ 1010 bytes {application} [built]
[./node_modules/webpack/buildin/global.js] (webpack)/buildin/global.js 878 bytes {application} [built]
[./node_modules/webpack/buildin/module.js] (webpack)/buildin/module.js 552 bytes {application} {nationality-autocomplete} [built]
    + 19 hidden modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--6-1!node_modules/postcss-loader/src/index.js??ref--6-2!node_modules/accessible-autocomplete/dist/accessible-autocomplete.min.css:
    Entrypoint mini-css-extract-plugin = *
       2 modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--7-1!node_modules/postcss-loader/src/index.js??ref--7-2!node_modules/sass-loader/dist/cjs.js??ref--7-3!app/frontend/styles/application.scss:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/postcss-loader/src/index.js?!./node_modules/sass-loader/dist/cjs.js?!./app/frontend/styles/application.scss] ./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss 524 KiB {mini-css-extract-plugin} [built] [2 warnings]
        + 8 hidden modules
WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4415:9) start value has mixed support, consider using flex-start instead

WARNING in ./app/frontend/styles/application.scss (./node_modules/css-loader/dist/cjs.js??ref--7-1!./node_modules/postcss-loader/src??ref--7-2!./node_modules/sass-loader/dist/cjs.js??ref--7-3!./app/frontend/styles/application.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(4580:9) start value has mixed support, consider using flex-start instead

So ~80% faster?

kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Jan 15, 2020
This removes all the places where we're importing files from
govuk-frontend which are needed because we import govuk/all. Theses are all
swapped to be comments. These are removed because there is a pretty
significant performance impact to importing files from govuk-frontend,
see: alphagov/govuk-frontend#1671 and
resolving this seems to add more value than what we have from the exact
files

This is removed to offer a performance boost. Running on my 2015 MBP
this chance was able to reduce the time to compile the application.css
for the dummy app:

Before:

➜  ~ time curl -s -o /dev/null http://localhost:3212/assets/application.css
curl -s -o /dev/null http://localhost:3212/assets/application.css  0.01s user 0.01s system 0% cpu 19.149 total

After:

➜  ~ time curl -s -o /dev/null http://localhost:3212/assets/application.css
curl -s -o /dev/null http://localhost:3212/assets/application.css  0.01s user 0.01s system 0% cpu 12.149 total

So a 7 second reduction.

Applying this to a different GOV.UK application (Content Publisher) was
more dramatic (for reasons I don't fully understand):

Before:

➜  content-publisher git:(master) time curl -s -o /dev/null http://localhost:3000/assets/application.css
curl -s -o /dev/null http://localhost:3000/assets/application.css  0.01s user 0.01s system 0% cpu 37.841 total

After:

➜  content-publisher git:(master) ✗ time curl -s -o /dev/null http://localhost:3000/assets/application.css
curl -s -o /dev/null http://localhost:3000/assets/application.css  0.01s user 0.01s system 0% cpu 24.273 total
kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Jan 15, 2020
This removes all the places where we're importing files from
govuk-frontend which are needed because we import govuk/all. Theses are all
swapped to be comments. These are removed because there is a pretty
significant performance impact to importing files from govuk-frontend,
see: alphagov/govuk-frontend#1671 and
resolving this seems to add more value than what we have from the exact
files

This is removed to offer a performance boost. Running on my 2015 MBP
this chance was able to reduce the time to compile the application.css
for the dummy app:

Before:

➜  ~ time curl -s -o /dev/null http://localhost:3212/assets/application.css
curl -s -o /dev/null http://localhost:3212/assets/application.css  0.01s user 0.01s system 0% cpu 19.149 total

After:

➜  ~ time curl -s -o /dev/null http://localhost:3212/assets/application.css
curl -s -o /dev/null http://localhost:3212/assets/application.css  0.01s user 0.01s system 0% cpu 12.149 total

So a 7 second reduction.

Applying this to a different GOV.UK application (Content Publisher) was
more dramatic (for reasons I don't fully understand):

Before:

➜  content-publisher git:(master) time curl -s -o /dev/null http://localhost:3000/assets/application.css
curl -s -o /dev/null http://localhost:3000/assets/application.css  0.01s user 0.01s system 0% cpu 37.841 total

After:

➜  content-publisher git:(master) ✗ time curl -s -o /dev/null http://localhost:3000/assets/application.css
curl -s -o /dev/null http://localhost:3000/assets/application.css  0.01s user 0.01s system 0% cpu 24.273 total
@36degrees
Copy link
Contributor

When we're making changes off the back of this, we should be thinking about how it'll impact an eventual move to @uses.

@36degrees 36degrees self-assigned this Feb 14, 2020
@NickColley
Copy link
Contributor

NickColley commented Feb 20, 2020

When I build the project using Node sass (node wrapper around libsass) it builds in half a second, does anyone know why there's such a high increase when using sassc which should be a wrapper around libsass?

time node-sass ./package/govuk/all.scss

Outputs:
node-sass ./package/govuk/all.scss 0.40s

@36degrees
Copy link
Contributor

I have a WIP branch that tries to provide a non breaking-change way of reducing the number of imports when importing all:

https://github.com/alphagov/govuk-frontend/tree/sass-perf

@36degrees
Copy link
Contributor

(the commented out imports in the layer files are just me trying to profile the compilation time of individual components)

@36degrees
Copy link
Contributor

When I build the project using Node sass (node wrapper around libsass) it builds in half a second, does anyone know why there's such a high increase when using sassc which should be a wrapper around libsass?

Is there any sort of caching going on that needs clearing for true timings?

@NickColley
Copy link
Contributor

Yeah I'm not sure I tried to find documentation to find out if there was some caching going on but couldnt find it, welcome any help.

@kevindew
Copy link
Member Author

I have a WIP branch that tries to provide a non breaking-change way of reducing the number of imports when importing all:

https://github.com/alphagov/govuk-frontend/tree/sass-perf

This looks great 👍

When I build the project using Node sass (node wrapper around libsass) it builds in half a second, does anyone know why there's such a high increase when using sassc which should be a wrapper around libsass?

time node-sass ./package/govuk/all.scss

Outputs:
node-sass ./package/govuk/all.scss 0.40s

I think my info in the top description above has the times for bare CLI sass compilation wrong by a decimal point (0.27 and 0.21) - sorry! My raw result on https://github.com/kevindew/govuk-frontend-rails-performance#with-govuk-frontend-using-sassc has a much similar time to what you got with node-sass:

time sassc app/assets/stylesheets/application.scss -I node_modules
=> sassc app/assets/stylesheets/application.scss -I node_modules  0.19s user 0.03s system 83% cpu 0.271 total

@36degrees 36degrees added this to the v4.0.0 milestone Mar 16, 2020
@36degrees
Copy link
Contributor

We've now raised a proposal for this here: alphagov/govuk-design-system-architecture#21

@kevindew
Copy link
Member Author

Thanks for the update @36degrees - I'll take a look at the proposal and encourage other GOV.UK devs to do so too.

@36degrees
Copy link
Contributor

Based on feedback from the initial proposal, we've iterated and raised a new proposal with an alternative approach: alphagov/govuk-design-system-architecture#22

kevindew added a commit to alphagov/govuk_publishing_components that referenced this issue Apr 30, 2020
This removes the sass depedency of this gem and instead replaces it with
a sassc-rails dependency. This is set as a development dependency as
this is only required for the dummy version of this app and this gem
should not force a particular version of sass on consuming apps without
good reason.

This should help GOV.UK gradually move towards a world without Ruby
SASS. Prior to this change introducing govuk_publishing_components into
an application forced Ruby SASS in as a dependency, this has the effect
in Sprockets of choosing Ruby SASS over sassc.

A downside of doing this is that it will make the warm cache run slower
for requests. Until GOV.UK Frontend resolves some performance issues [1]
we may find each page of the dummy app loads a reasonable amount slower
in a development environment. Although not ideal, I don't think this
performance issue will cause anyone significant problems developing on
this app (I get about 2.5s on my local machine) and it'll be nice for us
to have already started the sassc journey.

[1]: alphagov/govuk-frontend#1671
@36degrees
Copy link
Contributor

36degrees commented May 5, 2020

Off the back of the proposal, we've created four issues that represent the changes that need to be made:

@36degrees 36degrees added epic Epics are used in planning project boards to group related stories and removed breaking change sass / css ⚠️ high priority Used by the team when triaging 🕔 days labels May 5, 2020
@36degrees
Copy link
Contributor

@kevindew Sorry to repeat myself, we've published a pre-release of the new changes in #1804 – is there any chance you might be able to benchmark it and see how it compares?

npm install --save alphagov/govuk-frontend#1699f846

(It should be very similar to what you saw with #1752, so really just looking for a sanity check at this point!)

@kevindew
Copy link
Member Author

kevindew commented May 11, 2020

Thanks @36degrees - really exciting that we're making progress still.

I've given 1699f84 a test run and it seems to have a few performance regressions compared to 6735d43. Here's some comparisons that I ran just now on my machine (I also added in 3.6 to testing in case that was a factor):

SASS Cache 3.4.0 Time (s) 3.6.0 Time (s) 6735d43 Time (s) 1699f84 Time (s)
sassc Cold 5.056535 5.753195 0.997573 3.811318
sassc Warm 1.810953 1.424013 0.018379 0.622007
Ruby SASS Cold 14.236387 14.817171 5.773475 10.718535
Ruby SASS Warm 0.008104 0.007467 0.010392 0.007001
Individual runs with branches

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-npm-sassc

➜  govuk-frontend-test-harness git:(govuk-frontend-npm-sassc) rails tmp:clear && rails c
Running via Spring preloader in process 803
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  2.691000   2.263217   4.954217 (  5.056535)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.360359   0.427982   0.788341 (  1.810953)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-npm

➜  govuk-frontend-test-harness git:(govuk-frontend-npm) rails tmp:clear && rails c
Running via Spring preloader in process 96727
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  9.287733   3.378479  12.666212 ( 14.236387)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.005946   0.002045   0.007991 (  0.008104)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-npm-3.6-sassc

➜  govuk-frontend-test-harness git:(govuk-frontend-npm-3.6-sassc) rails tmp:clear && rails c
Running via Spring preloader in process 2300
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  2.820635   2.481661   5.302296 (  5.753195)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.342728   0.412966   0.755694 (  1.424013)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-npm-3.6

➜  govuk-frontend-test-harness git:(govuk-frontend-npm-3.6) rails tmp:clear && rails c
Running via Spring preloader in process 3531
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  9.569619   3.314844  12.884463 ( 14.817171)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.005483   0.001878   0.007361 (  0.007467)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-6735d43a-sassc

➜  govuk-frontend-test-harness git:(govuk-frontend-6735d43a-sassc) rails tmp:clear && rails c
Running via Spring preloader in process 5720
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.637290   0.352559   0.989849 (  0.997573)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.011042   0.003874   0.014916 (  0.018379)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-6735d43a-ruby-sass

➜  govuk-frontend-test-harness git:(govuk-frontend-6735d43a-ruby-sass) rails tmp:clear && rails c
Running via Spring preloader in process 5195
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  4.238595   1.039007   5.277602 (  5.773475)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.004981   0.001917   0.006898 (  0.010392)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-1699f846-sassc

➜  govuk-frontend-test-harness git:(govuk-frontend-1699f846-sassc) rails tmp:clear && rails c
Running via Spring preloader in process 8897
Loading development environment (Rails 6.0.1)
irb(main):001:0>  puts Benchmark.measure { Rails.application.assets["application.css"] }
  1.981663   1.758010   3.739673 (  3.811318)
=> nil
irb(main):002:0>  puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.296399   0.316943   0.613342 (  0.622007)
=> nil

https://github.com/kevindew/govuk-frontend-rails-performance/tree/govuk-frontend-1699f846-ruby-sass

➜  govuk-frontend-test-harness git:(govuk-frontend-1699f846-ruby-sass) rails tmp:clear && rails c
Running via Spring preloader in process 7000
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  7.168966   2.374849   9.543815 ( 10.718535)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.004869   0.001524   0.006393 (  0.007001)
=> nil

So it looks like the time savings we make with sassc via sprockets have gone up about 400% since the last pre-release but still a good 20% reduction on previous times

I did a bit of tinkering myself to remove @import statements to try work out the cause and it seems to lie mostly in the importing of supporting components with forms. After a bit of hacking I got similar results to before (though this was rather crude):

➜  govuk-frontend-test-harness git:(govuk-frontend-1699f846-sassc) rails tmp:clear && rails c
Running via Spring preloader in process 21115
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.488338   0.221433   0.709771 (  0.724322)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.007836   0.002387   0.010223 (  0.010261)
=> nil

I think there might be a mistake in components importing other components:

@import "../error-message/error-message";
@import "../hint/hint";
@import "../label/label";
@import "../textarea/textarea";

These are importing the non-index version of the components which means that base is included for each one of these, and thus cascades multiple imports.

If I change all of those to reference index files and make no other changes I get this result with Ruby and Sassc:

➜  govuk-frontend-test-harness git:(govuk-frontend-1699f846-sassc) ✗ rails tmp:clear && rails c
Running via Spring preloader in process 29075
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.757306   0.470081   1.227387 (  1.261366)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.016180   0.004629   0.020809 (  0.021007)
=> nil

Otherwise, the time savings I got (to take it down to 0.7s) was removing the various @imports of base from core/settings/etc files. I guess though with the 4.0 release we might be able to reduce those further by defining exactly what belongs in base or not.

Thanks again for continuing to move this forward 👍

@36degrees
Copy link
Contributor

These are importing the non-index version of the components which means that base is included for each one of these, and thus cascades multiple imports.

Great spot – I'll fix that now.

@36degrees
Copy link
Contributor

@kevindew with the changes in 3e471f8 I'm now seeing these results using your test repo (also on a MBP 15" 2015, so these should be ~comparable):

SASS Cache ee7fe13 Time (s)
sassc Cold 1.259280
sassc Warm 0.039461
Ruby SASS Cold 5.702858
Ruby SASS Warm 0.007272
Individual runs with branches

Ruby Sass

➜ rails tmp:clear && rails c
Running via Spring preloader in process 8985
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  4.229962   1.103654   5.333616 (  5.702858)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.005551   0.001726   0.007277 (  0.007272)

Sassc

➜ rails tmp:clear && rails c
Running via Spring preloader in process 12404
Loading development environment (Rails 6.0.1)
irb(main):001:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.729329   0.508032   1.237361 (  1.259280)
=> nil
irb(main):002:0> puts Benchmark.measure { Rails.application.assets["application.css"] }
  0.033088   0.005693   0.038781 (  0.039461)

I think that's probably as good as we're going to get it without breaking changes.

There's another pre-release available if you want to do any further testing:

npm install --save alphagov/govuk-frontend#ee7fe13b

@kevindew
Copy link
Member Author

That's awesome @36degrees - I can confirm I get times in the same ballpark as you there. Thanks again.

@36degrees 36degrees removed this from the v4.0.0 milestone May 18, 2020
@36degrees
Copy link
Contributor

With these changes going out in 4.0:

I believe we've now done everything we said we were going to do in the proposal, so we can mark this epic as done and close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Epics are used in planning project boards to group related stories
Projects
None yet
Development

No branches or pull requests

8 participants