-
Notifications
You must be signed in to change notification settings - Fork 357
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
Comments
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.
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 |
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. |
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 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 @nex3 do you mean that I tried check if there was a difference in memory usage by replacing this part from
to
But got a bunch of errors of this type:
So I guess there's more to it. |
/cc @filipesilva feel free to open issue in sass-loader for tracking and investigation |
It should always be using
The Webpack importer seems to be written asynchronously right now, so it makes sense that it doesn't work with |
@evilebottnawi opened an issue in |
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 |
@vikerman If you pass the |
Heya @nex3 👋
Recently at Angular CLI we flipped our default from using
node-sass
todart-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. Withnode-sass
I see a total of 580mb used, and withdart-sass
it's 760mb instead.In Angular CLI we use a Webpack compilation with
sass-loader
, that can use bothdart-sass
andnode-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 tosass-loader
.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.
The text was updated successfully, but these errors were encountered: