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

sass-loader performance #296

Closed
mmase opened this issue Nov 4, 2016 · 43 comments

Comments

Projects
None yet
@mmase
Copy link

commented Nov 4, 2016

We have a fairly expansive suite of sass files (around 600 files total). When running our suite directly with libsass, behind node-sass, it takes less than 5 seconds. Yet, sass-loader through webpack takes over 15 seconds.

I am wondering if others have run into this or if we can debug this to find where the performance lag lies.

@khankuan

This comment has been minimized.

Copy link

commented Nov 7, 2016

I experienced great improvements after switching to node-sass. But doing that would go in opposite direction with component-based styling. @mmase do you have lots of variables and mixins that you import with @import? Also, are you importing .sass files multiple times, i.e, every component imports its own sass file?

@wildeyes

This comment has been minimized.

Copy link

commented Nov 25, 2016

"Also, are you importing .sass files multiple times, i.e, every component imports its own sass file?" would doing this improve or worsen the compiliation speed?

@jhnns

This comment has been minimized.

Copy link
Member

commented Nov 29, 2016

Related to: #307

@khankuan

This comment has been minimized.

Copy link

commented Nov 29, 2016

@wildeyes i think so. I have 2 sass files, variables.sass and componentA.sass. The variables file is used by multiple components. Incremental rebuild speed for 700 components took 8 secs on a variable.sass change while 2 secs for componentA.sass.

@jhnns

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

Based on this discussion, I started profiling an example project yesterday and had some interesting insights. First of all, the sass-loader performance (49.2s) is completely unacceptable compared to fast-sass-loader (5.29s) or even bare node-sass (2.25s) from the command line. All three toolchains compile the same source code and produce similar output. We can expect some overhead coming from webpack since we need to initialize it, set up the loader pipeline, etc., but even the fast-sass-loader is unacceptable slow compared to bare node-sass in my opinion.

Overview

So let's take a look at the flame graphs:

sass-loader-overview

First of all, as you can see, node-sass is spending most of the time outside of Node.js which is what we expected, since the hard work is done by Libsass.

Now, let's take a look at both sass-loaders. The execution can roughly be divided into five phases (indicated as black boxes in the graph) that I will discuss in detail later:

  1. webpack initialization
  2. first execution sass-loader
  3. first execution css-loader
  4. second execution sass-loader (this block is not visible in the fast-sass-loader graph, reasons below)
  5. second execution css-loader

So, the first surprise here is that we have two separate compilations going on. This is due to the extract-text-webpack-plugin which initiates a child compilation. Tbh, I'm not sure why this is necessary at all. @sokra can give us more insights here.

The second surprise is, that the second compilation doesn't use the cache from the first compilation. In the sass-loader graph, you can clearly see that the second pass takes almost exactly the same amount of time. This could be improved a little bit because the sass-loader is currently not using webpack's fs cache, but that would only reduce the second sass-loader execution time (4.), which is not where most of the time is spent.

The most time, however, is spent—and that's the third surprise—inside the css-loader. You can clearly see that in both graphs. It takes up half of the total time in the fast-sass-loader setup, and even more in the sass-loader setup.

It might also be surprising that node-sass and webpack + fast-sass-loader take almost the same amount of time to compile. In the flame graphs, you can clearly see that node-sass finishes almost at the same time where the 2. block of the fast-sass-loader graph ends. However, we need to remember that the fast-sass-loader is deduping all files before compilation, so there's far less to compile.


Now, let's take a look at all the phases in detail.

Phase 1: Webpack initialization & loader execution start

sass-loader-phase-1

No big surprise here: The initialization of both setups takes almost the same amount of time. Differences here are probably due to I/O or general OS flakyness because there is no actual difference between both loaders here.

You can clearly see that during the first 1000ms, most of the time is spent requiring modules. That's why webpack almost always takes at least 1s to compile—even with the simplest setup. Maybe we could win something here with lazy requiring ala lazy-req.

Phase 2: Import resolving & sass compilation (1. compilation)

sass-loader-phase-2

Now that looks different!

First, let's look at the sass-loader: The sass-loader registers a custom importer and kicks off the compilation. Now, we're essentially waiting for Libsass to call back. You can clearly see that there is a lot of back-and-forth going on with recurring breaks (grey bars) where the process is just sitting and waiting for Libsass. This graph led me to the assumption that Libsass might be doing sequential I/O. Concurrent I/O would look different, I suppose. After all the resolving is done, Libsass is performing the actual compilation and we're just waiting for Libsass to call back. Nothing to optimize here.

Now, let's look at the fast-sass-loader: The fast-sass-loader preparses the source code with regexps for import statements and passes all imports to webpack's resolver. After the import paths have been resolved, the imported files are read from disk via node's fs module and then replaced with their import statements. Duplicated imports are simply omitted, only the first occurence gets included. This is way faster than the sass-loader, which shows that webpack's resolver is not the bottleneck. In fact, it's really fast and almost negligible in the graph.

The actual Sass compilation is also a lot faster since a lot of imports have been deduped. It's just that Libsass has less to parse.

Phase 3: css-loader (1. compilation)

sass-loader-phase-3

This phase is the most interesting one because:

  1. it takes up most of the time
  2. there is nothing the sass-loader or the fast-sass-loader can do

The only difference is that the amount of data is very different. The fast-sass-loader produces a string with length 627067, the sass-loader with 6914854. This is 11 times longer. And this translates roughly to the amount of time. The css-loader takes 11 times longer processing the output from the sass-loader.

There are two things that I found surprising:

  1. Why does the css-loader take so long? In this case, it is only required to translate the CSS into a CommonJS module while transforming all url() statements and remaining @import into require(). To me, this task seems a lot more trivial than the Sass compilation. The funny thing is: We're spending 9.6s just inside postcss-modules-local-by-default during the first compilation. In total, we're spending 16.29s there – and we didn't even ask for CSS modules... And that's just the first postcss plugin. The thread is completely busy for 13s in the first compilation while processing postcss plugins without any chance for other tasks. In total: 26s just inside postcss, hogging the CPU.

  2. Why is postcss busy with generating source maps? We didn't ask for source maps... then I took a look at the css-loader: This call applies the map option irregardless of an inputMap. In fact, inputMap is undefined in this case, but we're still telling postcss to generate a map. As an experiment, I set map: false and was able to shave off 10s in total—just by setting one flag. It's still crazy that source maps take up 10s. Switching source maps on and off with node-sass makes a difference of 150ms.

Phase 4: sass-loader & css-loader (2. compilation)

sass-loader-phase-4

The sass-loader performs exactly the same compilation again. No results from the first compilation are re-used.

The fast-sass-loader skips the second Sass compilation because it uses its own cache. That's a nice shortcut, but this should be fixed in webpack.

I don't understand why we need a second compilation. And if we need the second one, can't we just skip the first one? This seems redundant.


Conclusion

  • We spend 1s just requiring webpack modules and loaders and initializing the loader pipeline. This could possibly be improved with a tool like lazy-req.
  • The sass-loader could be improved by:
    • preparsing and resolving all dependencies. While this might seem like a good idea, there are also problems: 1. The source maps will be incorrect. 2. Errors won't be reported at the correct location. It all boils down to the fact that Libsass doesn't know the original location of the source anymore.
    • using the webpack cache when resolving the file. A custom importer is also allowed to pass the actual source to Libsass. This way, we could use webpack's internal fs caching layer.
    • deduping dependencies. This could be achieved by passing an empty string if the custom importer has resolved a resource for the second time. While this operation is not entirely safe, it will most likely do more good than bad in most cases. I'd say: if the specificity relies on the correct source order, there's a bigger problem.
  • The css-loader could be improved by:
    • using a simpler pipeline for CSS files that are not CSS modules. This performance penalty is not acceptable.
    • only generating source maps when they are actually requested
    • "somehow" improving the postcss pipeline. Blocking the process for 13s is not acceptable, even for CSS modules. I don't know how we can improve that. Maybe postcss is applying the plugins in an unfortunate way?
  • The extract-text-webpack-plugin could be improved by:
    • getting rid of the second compilation
    • or using cached loader results where possible

Here are both CPU profiles. Load them into your Chrome Developer Tools if you want to take a look for yourself.

cpuprofiles.zip


This was bothering me for a long time because I think we can do a lot better than that 😁

@jhnns

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

As comparison, the sass-loader setup with css-loader@0.14 takes 10.1s and looks like this:

bildschirmfoto 2017-03-22 um 18 13 36
sass-loader-old-css-loader.cpuprofile.zip

@jhnns

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

As @sokra pointed out: With the allChunks: true option...

    new ExtractTextPlugin({
      filename: '[name].css',
      allChunks: true
    })

the second compilation can be skipped:

bildschirmfoto 2017-03-22 um 18 40 34

thus reducing the built time to 23.9s

@feng-xiao

This comment has been minimized.

Copy link

commented Mar 27, 2017

@jhnns Hi jhnns, may I ask what tools you were using to generate those profiling screenshots? I need to troubleshoot webpack issues as well, I need a tool to help me pinpoint the problem.

@jhnns

This comment has been minimized.

Copy link
Member

commented Mar 27, 2017

Run

node --inspect-brk ./node_modules/.bin/webpack

to start the debugging process. The process will halt at the first line and print a debugger URL. Copy the debugger URL into the Chrome Browser and the developers tools will initialize.

Then go to the JavaScript profiler tab and start profiling :)
After you've pressed stop, the flame graph will be generated.

If you don't want to copy debugger URLs around, you can also use the NIM chrome extension. It discovers debuggable node processes automatically.

@feng-xiao

This comment has been minimized.

Copy link

commented Mar 28, 2017

@jhnns thank you very much!!!

@jhnns

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

Not directly related to this thread, but also interesting: Looks like the most recent v8 version (with their new optimizing compiler TurboFan) gives a startup performance boost of 160% (1150ms vs 720ms).

Using v8/node, phase 1 looks like this:

bildschirmfoto 2017-03-29 um 15 23 55

The overall build time decreased from 49.2s to 41.35s (~119% faster).

TurboFan doesn't bail out on try/catch which will probably improve the startup performance of all node applications because node is using try/catch to initialize the modules.

@sokra

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

awesome 👍

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Apr 8, 2017

Postcss issue about source map: postcss/postcss-loader#195

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Apr 11, 2017

After disabling generation of source maps using my PR webpack-contrib/css-loader#478 i get about ~1.5x-2x speedup my build

@evilebottnawi evilebottnawi changed the title sass-loader speed vs. using node-sass directly sass-loader performance May 23, 2017

@mikesherov

This comment has been minimized.

Copy link

commented Jul 11, 2017

hello friends. Just as an FYI, I'm looking into this. With any luck, we'll see if there is any low hanging fruit to go after besides for the allChunks and sourcemap: false optimizations.

@levin-du

This comment has been minimized.

Copy link

commented Aug 6, 2017

any new updates? @mikesherov

@mikesherov

This comment has been minimized.

Copy link

commented Aug 6, 2017

@levin-du, several of css-loaders dependencies are extremely slow but have been rewritten to be much much faster, but are semver major. css-loader is currently undergoing a rewrite along with the rest of the CSS module loading architecture, so I asked if a patch would be accepted. Due to the fact that it's a major bump, I was asked to wait for the rewrite to be completed instead.

@sentience

This comment has been minimized.

Copy link

commented Aug 6, 2017

That's a shame, @mikesherov, but thanks for the update!

@jhnns

This comment has been minimized.

Copy link
Member

commented Apr 13, 2018

Maybe dart-sass #435 is faster. I need to try that out.

@xzyfer

This comment has been minimized.

Copy link

commented Jun 5, 2018

I also want to take this opportunity to say we're more than happy to help out where ever we can. We <3 the Sass and surrounding tooling communities.

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 5, 2018

@xzyfer will be great if you review code in master and maybe add advice how we can solve something better than we do right now (or send PR 👍 )

@xzyfer

This comment has been minimized.

Copy link

commented Jun 5, 2018

We've discussed some improvements custom importers on the Node Sass side that might be looked at in not too distant future. However the profiling in #296 (comment) seems to suggest (to me) that improvements to Node Sass will have a negligible affect.

review code in master and maybe add advice how we can solve something better than we do right now

I've had a look in the past and nothing jumps out at me. As for webpack specifics that's really out of my expertise.

The fast-sass-loader produces a string with length 627067, the sass-loader with 6914854.

I think this is something that deserves attention though. I don't anything that explains this 11x difference in output from Node Sass. Dependency deduping was suggested to be the cause but since @imports are cached internally in LibSass I'm don't believe that to be the case (assuming I'm correctly interpreting dependency deduping).

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@xzyfer We have some perf problem with resolving @import, because we don't know should be @import handle by node-sass (i.e. includesPath option) or we should handle this inside own importer, maybe node-sass can provide this function (like nodeSass.resolveImport(pathToImport) and return path if yes)? It is solve many problems.

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@xzyfer problem not only in ~, webpack support aliases, we should first check can node-sass handle @import if not, try to resolve using aliases.

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

@evilebottnawi importers have to work the other way. Try to resolve ~ or aliases then defer to node-sass if nothing is found. This is because importers can be chained like middleware. The next importer in the chain will be executed if the current importer returns nothing.

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@xzyfer Right now we do this, but problem it is perf (this issue was created due this logic), we should check all possible alias variants (they can be many, more infromation https://webpack.js.org/configuration/resolve/) and when we can't find any aliases send file path to node-sass, it is spends a lot of cpu/memory usage.

Also some people can use includePaths and aliasing together and in some cases this leads to incorrect resolving. If we could check should @import handle node-sass we would solve a lot of problems and incompatibilities. It's really difficult to solve technically?

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

This isn't possible in Sass a moment, and there aren't any plans to added the ability to defer an importer.

The only solution I can think off is to register a second importer in front of your expensive importer. Have that importer mimic the existing Sass importer algorithm. If it fails to find anything then your importer can run.

It would be a waste to move logic from c++ to JavaScript but if the performance problems are that significant it might worth it.

The Node Sass team is working an official importer that does exactly this so if you wait a little bit the work will be done for free.

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@xzyfer

The only solution I can think off is to register a second importer in front of your expensive importer. Have that importer mimic the existing Sass importer algorithm. If it fails to find anything then your importer can run.

Where i can look on this (link or spec)?

Thanks for discussion, ping me when i can look on official importer

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

I couldn't find where the import algorithm is officially documented but it is roughly (not exactly)

@import "foo";
candidates = []
if cwd + foo.sass then candidates.push(cwd + foo.sass)
if cwd + foo.scss then candidates.push(cwd + foo.scss)
if cwd + _foo.sass then candidates.push(cwd + _foo.sass)
if cwd + _foo.scss then candidates.push(cwd + _foo.scss)
for each path in loadpaths
  if path + foo.sass then candidates.push(cwd + foo.sass)
  if path + foo.scss then candidates.push(cwd + foo.scss)
  if path + _foo.sass then candidates.push(cwd + _foo.sass)
  if path + _foo.scss then candidates.push(cwd + _foo.scss)
end

if candidates.empty throw file not found error
if candidates.length > 1 throw ambigious import error
return candidates.first
@mikesherov

This comment has been minimized.

Copy link

commented Jun 8, 2018

@evilebottnawi @xzyfer thanks so much for looking into this. I wonder though if replicating the resolution algorithm is focusing in on the wrong part of the problem. #296 (comment) shows basically all the ways that sass-loader are slower than fast-sass-loader, and import resolving doesn't seem to be the biggest part. If master has fixes for this, then great!

Thing's that do seem to stand out, as @xzyfer mentions:

  • The only difference is that the amount of data is very different. The fast-sass-loader produces a string with length 627067, the sass-loader with 6914854. This is 11 times longer. And this translates roughly to the amount of time. The css-loader takes 11 times longer processing the output from the sass-loader.

  • This happens if lots of importers are being resolved in parallel and they're additionally doing asynchronous work that requires the thread pool i.e. fs.readFile. This is well documented in sass/node-sass#857. Using some kind in memory fs cache could certainly work around this problem.

  • The funny thing is: We're spending 9.6s just inside postcss-modules-local-by-default during the first compilation. In total, we're spending 16.29s there – and we didn't even ask for CSS modules... And that's just the first postcss plugin. The thread is completely busy for 13s in the first compilation while processing postcss plugins without any chance for other tasks. In total: 26s just inside postcss, hogging the CPU.

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

@xzyfer

This comment has been minimized.

Copy link

commented Jun 8, 2018

I'm happy to contribute on improving performance - sass-loader is an important part of our ecosystem.

These performance problems may be more pronounced after #573 lands due to the speed differences between the two engines.

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@mikesherov it is old perf screenshots, we need do this again, feel free to do this 👍

@mikesherov

This comment has been minimized.

Copy link

commented Jun 8, 2018

@evilebottnawi I should try out of master?

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

@mikesherov

This comment has been minimized.

Copy link

commented Jun 18, 2018

updated waterfalls

See attached .cpuprofiles:
Archive.zip

latest sass-loader

image

latest fast-sass-loader

image

Main difference at this point is that fast-sass-loader does deduping (which can break correctness, a known caveat) and foregoes advanced resolving, so a lot more time is spent in node-sass for sass-loader than it is for fast-sass-loader.

@jhnns

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Hey @xzyfer 👋, thanks for sharing some insights (#296 (comment)).

Internally LibSass will only resolve the first unique instance of an @import. So this de-duping behaviour is already happening.

How does that actually work? What exactly is de-duped? The generated CSS?

The IO is 100% controlled by Node Sass, and the libuv event loop. When render is called Node Sass queue a LibSass job in the uv event loop, and uv pings Node Sass back when the work is done with the output (as JSON blog is the output and statistics). This is how all callbacks work, nothing special going.

When a custom importer is used an additional step happens. When LibSass encounters an @import it calls each registered custom importer sequentially until one of them returns with a value. Executing a custom function (when defined in JavaScript) means the LibSass process "pauses" compilation to tell Node Sass to execute the custom importer. Node pushes the import function into the uv event loop, uv eventually executes it, and passes the resulting value to Node Sass. Node Sass then passes the value back to LibSass which continues parsing.

This back and fourth is what you're seeing in that flame graph. Everything is happening asynchronously. Each LibSass process is isolated, and Node is in change of how parallel the work is.

I'd describe this behavior as sequential I/O because LibSass pauses the compilation until the custom importer call backs. That may be a restriction of Sass itself, but if there are a lot of imports at the beginning, this would mean that all the files cannot be resolved in parallel.

I'm happy to contribute on improving performance - sass-loader is an important part of our ecosystem.

I really appreciate that 👍. And thank you for your hard work.

Webpack (and the sass-loader) is adding a lot of unnecessary complexity here.

Over the past years, I came to the conclusion that the Sass compilation should be completely opaque to webpack. In the beginning, we had the idea that every Sass module is just a module in our dependency graph. But Sass (and Less) work differently, they don't have modules, they "just" have files that produce some output. Treating them as modules in the module graph was probably a mistake.

Now I'm thinking about removing the custom importer feature from sass-loader, because it's the cause for a lot of trouble and complexity. I think that Sass' includePath is probably enough for most use-cases. In the end, people shouldn't deviate from regular Sass (without webpack) anyway.

What do you think?

@xzyfer

This comment has been minimized.

Copy link

commented Jul 11, 2018

I'd describe this behavior as sequential I/O because LibSass pauses the compilation until the custom importer call backs.

For that Sass file yes, but multiple async render calls can happen in parallel. Keep in mind a render call takes a single Sass files and produces a CSS file. Multiple files can be compiled in parallel.

Now I'm thinking about removing the custom importer feature from sass-loader, because it's the cause for a lot of trouble and complexity. I think that Sass' includePath is probably enough for most use-cases. In the end, people shouldn't deviate from regular Sass (without webpack) anyway.

I disagree (in part), keeping in mind I don't know how this load currently works.

There is steady stream of demand from the community for @importing from node_modules to be baked into Node Sass. People love this feature!

includePaths are fairly insufficient for the task because npm can nest packages. Technically I'd imagine people expect node_module imports to emulate the npm require algorithm (i.e. piggy backing off require.resolve). Eyeglass was really the first project to make npm modules first class Sass citizens although it did it by adding metadat to the packages.

The Node Sass team has been talking about maintaining an offical node_modules importer.

I'm not entire sure whether that would be sufficient for use in a webpack loader. For instance I don't know how much you respect webpack config (include paths, modules, alias, etc..). This aspect of the loader I hear less comments about but that could very well be because of my local sphere.

@xzyfer

This comment has been minimized.

Copy link

commented Jul 13, 2018

I quickly put together a proof of concept loader for first class node module imports. It'll probably need some API changes to work within the webpack context but it's worth a look.

https://www.npmjs.com/package/@node-sass/node-module-importer

@manniL

This comment has been minimized.

Copy link

commented Nov 7, 2018

Hey folks 👋
Are there any news/successes on the perf topic? ☺️

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Dec 14, 2018

@manniL no,

We already cache many things and need improve speed on node-sass/dart-sass side, anyway you can look our code and send a PR with code where you can improve speed. Close issue because no places for optimization on our side. Also you can try cache-loader, it is allow to caching styles which don't changed, webpack@5 will support caching out of box and it is speed your build. Thanks!

@scottnath scottnath referenced this issue Feb 27, 2019

Closed

Export sass bundler #372

0 of 1 task complete

@asudoh asudoh referenced this issue Apr 19, 2019

Merged

Carbon Monorepo #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.