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

dart-sass uses a lot more memory than node-sass #744

Open
filipesilva opened this issue Jun 28, 2019 · 8 comments
Open

dart-sass uses a lot more memory than node-sass #744

filipesilva opened this issue Jun 28, 2019 · 8 comments
Labels
JavaScript Issues particular to the Node.js distribution needs info

Comments

@filipesilva
Copy link

Heya @nex3 👋

Recently at Angular CLI we flipped our default from using node-sass to dart-sass as part of our version 8 release. We started getting a lot of reports of CLI8 hitting the memory limit when building projects that didn't hit the memory limit in CLI7.

I've tried to profile where memory is being used in angular/angular-cli#13734 (comment). For this particular project, using dart-sass uses 180mb extra memory. With node-sass I see a total of 580mb used, and with dart-sass it's 760mb instead.

In Angular CLI we use a Webpack compilation with sass-loader, that can use both dart-sass and node-sass. It's hard to extract the exact sass compilation logic from that setup, but I can give you the source Sass files.

You can get them by cloning https://github.com/filipesilva/awesome-angular-workshop, and checking out commit SHA 9076a3d.

Inside this repository, the following .scss files are compiled individually, and included as part of the same webpack compilation. I obtained this list by logging all the requests to sass-loader.

./src/3-rxjs-begin/shared/filter/filter.component.scss
./src/3-rxjs-end/shared/filter/filter.component.scss
./src/4-reactive/shared/filter/filter.component.scss
./src/5-ngrx-begin/shared/filter/filter.component.scss
./src/5-ngrx-end/shared/filter/filter.component.scss
./src/0-awesome/app.component.scss
./src/1-routing-begin/app.component.scss
./src/1-routing-end/app.component.scss
./src/1-routing-guards-begin/app.component.scss
./src/1-routing-guards-end/app.component.scss
./src/2-modules-begin/app.component.scss
./src/2-modules-end/app.component.scss
./src/3-rxjs-begin/app.component.scss
./src/3-rxjs-end/app.component.scss
./src/4-reactive/app.component.scss
./src/5-ngrx-begin/app.component.scss
./src/5-ngrx-end/app.component.scss
./src/7-deploy/app.component.scss
./src/3-rxjs-begin/play-ops/play-ops.component.scss
./src/3-rxjs-begin/play-subject/play-subject.component.scss
./src/3-rxjs-end/play-ops/play-ops.component.scss
./src/3-rxjs-end/play-subject/play-subject.component.scss
./src/3-rxjs-begin/bus/color.component.scss
./src/3-rxjs-end/bus/color.component.scss
./src/styles/theme.scss
./src/styles/mixin.scss
./src/styles/styles.scss
./src/0-awesome/heroes/hero-detail/hero-detail.component.scss
./src/0-awesome/villains/villain-detail/villain-detail.component.scss
./src/1-routing-begin/heroes/hero-detail/hero-detail.component.scss
./src/1-routing-begin/villains/villain-detail/villain-detail.component.scss     
./src/1-routing-end/heroes/hero-detail/hero-detail.component.scss
./src/1-routing-end/villains/villain-detail/villain-detail.component.scss       
./src/1-routing-guards-begin/heroes/hero-detail/hero-detail.component.scss      
./src/1-routing-guards-begin/villains/villain-detail/villain-detail.component.scss
./src/1-routing-guards-end/heroes/hero-detail/hero-detail.component.scss       
./src/1-routing-guards-end/villains/villain-detail/villain-detail.component.scss
./src/2-modules-begin/heroes/hero-detail/hero-detail.component.scss
./src/2-modules-begin/villains/villain-detail/villain-detail.component.scss     
./src/2-modules-end/heroes/hero-detail/hero-detail.component.scss
./src/2-modules-end/villains/villain-detail/villain-detail.component.scss       
./src/3-rxjs-begin/heroes/hero-detail/hero-detail.component.scss
./src/3-rxjs-begin/villains/villain-detail/villain-detail.component.scss       
./src/3-rxjs-end/heroes/hero-detail/hero-detail.component.scss
./src/3-rxjs-end/villains/villain-detail/villain-detail.component.scss
./src/4-reactive/heroes/hero-detail/hero-detail.component.scss
./src/4-reactive/villains/villain-detail/villain-detail.component.scss
./src/5-ngrx-begin/heroes/hero-detail/hero-detail.component.scss
./src/5-ngrx-begin/villains/villain-detail/villain-detail.component.scss       
./src/7-deploy/heroes/hero-detail/hero-detail.component.scss
./src/7-deploy/villains/villain-detail/villain-detail.component.scss
./src/5-ngrx-end/heroes/hero-detail/hero-detail.component.scss
./src/5-ngrx-end/villains/villain-detail/villain-detail.component.scss
./src/0-awesome/heroes/heroes/heroes.component.scss
./src/0-awesome/villains/villains/villains.component.scss
./src/1-routing-begin/heroes/heroes/heroes.component.scss
./src/1-routing-begin/villains/villains/villains.component.scss
./src/1-routing-end/heroes/heroes/heroes.component.scss
./src/1-routing-end/villains/villains/villains.component.scss
./src/1-routing-guards-begin/heroes/heroes/heroes.component.scss
./src/1-routing-guards-begin/villains/villains/villains.component.scss
./src/1-routing-guards-end/heroes/heroes/heroes.component.scss
./src/1-routing-guards-end/villains/villains/villains.component.scss
./src/2-modules-begin/heroes/heroes/heroes.component.scss
./src/2-modules-begin/villains/villains/villains.component.scss
./src/2-modules-end/heroes/heroes/heroes.component.scss
./src/2-modules-end/villains/villains/villains.component.scss
./src/3-rxjs-begin/heroes/heroes/heroes.component.scss
./src/3-rxjs-begin/villains/villains/villains.component.scss
./src/3-rxjs-end/heroes/heroes/heroes.component.scss
./src/3-rxjs-end/villains/villains/villains.component.scss
./src/4-reactive/heroes/heroes/heroes.component.scss
./src/4-reactive/villains/villains/villains.component.scss
./src/5-ngrx-begin/heroes/heroes/heroes.component.scss
./src/5-ngrx-begin/villains/villains/villains.component.scss
./src/7-deploy/heroes/heroes/heroes.component.scss
./src/7-deploy/villains/villains/villains.component.scss
./src/5-ngrx-end/heroes/heroes/heroes.component.scss
./src/5-ngrx-end/villains/villains/villains.component.scss
./src/1-routing-end/core/toolbar/toolbar.component.scss
./src/1-routing-guards-end/core/toolbar/toolbar.component.scss
./src/2-modules-end/core/toolbar/toolbar.component.scss
./src/4-reactive/core/toolbar/toolbar.component.scss
./src/5-ngrx-end/core/toolbar/toolbar.component.scss
./src/0-awesome/core/toolbar/toolbar.component.scss
./src/1-routing-begin/core/toolbar/toolbar.component.scss
./src/1-routing-guards-begin/core/toolbar/toolbar.component.scss
./src/2-modules-begin/core/toolbar/toolbar.component.scss
./src/5-ngrx-begin/core/toolbar/toolbar.component.scss
./src/7-deploy/core/toolbar/toolbar.component.scss
./src/3-rxjs-end/core/toolbar/toolbar.component.scss
./src/3-rxjs-begin/core/toolbar/toolbar.component.scss
./src/0-awesome/heroes/hero-list/hero-list.component.scss
./src/0-awesome/villains/villain-list/villain-list.component.scss
./src/1-routing-begin/heroes/hero-list/hero-list.component.scss
./src/1-routing-begin/villains/villain-list/villain-list.component.scss
./src/1-routing-end/heroes/hero-list/hero-list.component.scss
./src/1-routing-end/villains/villain-list/villain-list.component.scss
./src/1-routing-guards-begin/heroes/hero-list/hero-list.component.scss
./src/1-routing-guards-begin/villains/villain-list/villain-list.component.scss 
./src/1-routing-guards-end/heroes/hero-list/hero-list.component.scss
./src/1-routing-guards-end/villains/villain-list/villain-list.component.scss    
./src/2-modules-begin/heroes/hero-list/hero-list.component.scss
./src/2-modules-begin/villains/villain-list/villain-list.component.scss
./src/2-modules-end/heroes/hero-list/hero-list.component.scss
./src/2-modules-end/villains/villain-list/villain-list.component.scss
./src/3-rxjs-begin/heroes/hero-list/hero-list.component.scss
./src/3-rxjs-begin/villains/villain-list/villain-list.component.scss
./src/3-rxjs-end/heroes/hero-list/hero-list.component.scss
./src/3-rxjs-end/villains/villain-list/villain-list.component.scss
./src/4-reactive/heroes/hero-list/hero-list.component.scss
./src/5-ngrx-begin/heroes/hero-list/hero-list.component.scss
./src/7-deploy/heroes/hero-list/hero-list.component.scss
./src/7-deploy/villains/villain-list/villain-list.component.scss
./src/5-ngrx-end/heroes/hero-list/hero-list.component.scss
./src/4-reactive/villains/villain-list/villain-list.component.scss
./src/5-ngrx-begin/villains/villain-list/villain-list.component.scss
./src/5-ngrx-end/villains/villain-list/villain-list.component.scss

I understand they are repeated and might be following bad practices (e.g. including a lot of toplevel css), but I don't think that has bearing on the comparative memory usage.

@nex3 nex3 added the JavaScript Issues particular to the Node.js distribution label Jun 28, 2019
@nex3
Copy link
Contributor

nex3 commented Jun 28, 2019

To some extent, increased memory usage relative to node-sass is expected due to the inherent difference in memory efficiency of JavaScript compared to handwritten C++. I'll do what I can to look into where Dart Sass is retaining memory, though, and see if I can trim it down or suggest a better way to load it.

To do that, though, I'd like some more information about how Angular is invoking the Sass compiler. I know you said extracting that logic was hard, but it would be tremendously helpful. Just plotting out the specific implementations of the Sass API methods and ideally creating a standalone JS file that replays them would make it much easier to narrow down the problem.

We expected dart-sass to not be as performant as node-sass so we let you go back to node-sass by installing it in your project (npm install node-sass --save-dev). As long as it's installed, we'll pick it up automatically. You can also use dart-sass with the fibers package for better performance by installing it in the same way as node-sass. Both node-sass and fibers use native bindings and might not work on all machines, which is why we don't install them by default.

I noticed this paragraph in angular/angular-cli#13734 (comment), which may indicate an easy way to get rid of some of the overhead. You shouldn't need to pass in the fibers package unless you're calling sass.render(), and you shouldn't need to call sass.render() (as opposed to sass.renderSync()) unless you're defining custom functions or importers that must be asynchronous. If you can call renderSync() instead, you can probably get rid of a bunch of asynchrony overhead for free, more or less.

@nex3
Copy link
Contributor

nex3 commented Jun 28, 2019

I should also mention #248, in which we're working on a way of packaging Dart Sass for npm as a Dart executable, but still exposing a full JavaScript API. That's likely to be substantially more efficient in both speed and memory, since it uses the Dart VM, while also taking its memory overhead out of the main CLI process. It's not likely to be ready for at least another quarter, but it's worth keeping an eye on.

@filipesilva
Copy link
Author

Thanks for getting back to me! We don't invoke the Sass compiler directly and instead use https://www.npmjs.com/package/sass-loader. The exact version used when I profiled it was sass-loader@7.1.0.

I'm not very familiar with how Sass is meant to be invoked by in https://github.com/webpack-contrib/sass-loader/blob/master/lib/loader.js I don't see any calls to renderSync. For the dart-sass case it seems to always be using render.

@nex3 do you mean that sass-loader should be using renderSync when fibers isn't provided? Looping in @evilebottnawi and @jhnns as they are sass-loader maintainers.

I tried check if there was a difference in memory usage by replacing this part from

render(options, (err, result) => {
  // callback code
});

to

var result = options.implementation.renderSync(options);
// callback code

But got a bunch of errors of this type:

ERROR in ./styles/styles.scss
Module build failed (from ../node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ../node_modules/sass-loader/lib/loader.js):
Error: Can't find stylesheet to import.
  ╷
4 │ @import '~@angular/material/theming';
  │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  src\styles\theme.scss 4:9  @import
  src\styles\mixin.scss 1:9  @import
  stdin 1:9                  root stylesheet
    at Object.BI (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5233:30)
    at Object.Ei (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5149:34)
    at nF.E8 (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5135:5)     
    at Object.GZ (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:463:69)
    at Object.hB (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2191:27)
    at HV (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2991:27)
    at Object.renderSync (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2980:42)
    at Object.sassLoader (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass-loader\lib\loader.js:48:39)        
    at runLoaders (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\webpack\lib\NormalModule.js:301:20)
    at D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:367:11
    at D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:233:18
    at runSyncOrAsync (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:143:3)   
    at iterateNormalLoaders (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:232:2)

So I guess there's more to it.

@alexander-akait
Copy link

/cc @filipesilva feel free to open issue in sass-loader for tracking and investigation

@nex3
Copy link
Contributor

nex3 commented Jul 1, 2019

Thanks for getting back to me! We don't invoke the Sass compiler directly and instead use https://www.npmjs.com/package/sass-loader. The exact version used when I profiled it was sass-loader@7.1.0.

I'm not very familiar with how Sass is meant to be invoked by in https://github.com/webpack-contrib/sass-loader/blob/master/lib/loader.js I don't see any calls to renderSync. For the dart-sass case it seems to always be using render.

@nex3 do you mean that sass-loader should be using renderSync when fibers isn't provided? Looping in @evilebottnawi and @jhnns as they are sass-loader maintainers.

It should always be using renderSync() with Dart Sass if at all possible. The only reason not to is if a custom importer or function absolutely can't avoid using asynchronous APIs (and in that case, it should use fibers if at all possible). I'm not familiar enough with Webpack's loader interface to know how easy this would be, but this page does seem to indicate that synchronous loaders are possible.

I tried check if there was a difference in memory usage by replacing this part from

render(options, (err, result) => {
  // callback code
});

to

var result = options.implementation.renderSync(options);
// callback code

But got a bunch of errors of this type:

ERROR in ./styles/styles.scss
Module build failed (from ../node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ../node_modules/sass-loader/lib/loader.js):
Error: Can't find stylesheet to import.
  ╷
4 │ @import '~@angular/material/theming';
  │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  src\styles\theme.scss 4:9  @import
  src\styles\mixin.scss 1:9  @import
  stdin 1:9                  root stylesheet
    at Object.BI (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5233:30)
    at Object.Ei (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5149:34)
    at nF.E8 (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:5135:5)     
    at Object.GZ (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:463:69)
    at Object.hB (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2191:27)
    at HV (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2991:27)
    at Object.renderSync (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass\sass.dart.js:2980:42)
    at Object.sassLoader (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\sass-loader\lib\loader.js:48:39)        
    at runLoaders (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\webpack\lib\NormalModule.js:301:20)
    at D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:367:11
    at D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:233:18
    at runSyncOrAsync (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:143:3)   
    at iterateNormalLoaders (D:\sandbox\memory-debug\awesome-angular-workshop\node_modules\loader-runner\lib\LoaderRunner.js:232:2)

So I guess there's more to it.

The Webpack importer seems to be written asynchronously right now, so it makes sense that it doesn't work with renderSync(), which only allows synchronous importers.

@filipesilva
Copy link
Author

@evilebottnawi opened an issue in sass-loader for using renderSync when applicable: webpack-contrib/sass-loader#701.

@vikerman
Copy link

Taking another look at this to see how to take this forward.

Looks like there is one change to be tried out on the CLI side. The sass-loader seems to use the async version (https://github.com/webpack-contrib/sass-loader/blob/master/src/getRenderFunctionFromSassImplementation.js#L15 ). We can try switching to the synchoronous renderSync (with a local mod to test out) and see how much that helps with the memory. If it helps we should upstream that into the sass-loader package probably behind a flag.

@nex3
Copy link
Contributor

nex3 commented Aug 30, 2019

@vikerman If you pass the fiber option, Dart Sass will use the synchronous code path even if the async render() is called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScript Issues particular to the Node.js distribution needs info
Projects
None yet
Development

No branches or pull requests

4 participants