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

Make the JS version faster #113

Open
nex3 opened this issue Feb 3, 2017 · 37 comments
Open

Make the JS version faster #113

nex3 opened this issue Feb 3, 2017 · 37 comments
Labels
enhancement help wanted JavaScript Issues particular to the Node.js distribution

Comments

@nex3
Copy link
Contributor

nex3 commented Feb 3, 2017

The JS-compiled version of Dart Sass hovers around 2.5x and 3x slower than when running on the Dart VM. Some slowdown is inevitable, but this seems extreme. We should put dedicated effort into profiling the JS output and optimizing hot spots.

@ranquild
Copy link

ranquild commented Sep 22, 2017

Common thing with dart2js performance issues is dynamic dispatch. For example, in dynamic s = func(); s.method();. call of method will be compiled to something like J.getInterceptor(s).method$0();, where getInterceptor - is sort-of runtime prototype wrapper that allows to call non-existing methods on JS primitives. Types like List or String are not wrapped in compiled dart code, they are Array and String in JS. So, when you have dynamic variable and call method/property like first, interceptor returns correct prototype for object by doing types checks for String, Array, int and so on.

What that does mean is that usage of dynamic ruins compiled-to-js performance and should be avoided at all costs. Easy workaround is to use Object for variables that could contain objects of different types and doing is checks and calling method on variable of specific types. Using dynamic in VM does not have those performance issues and that's why there is so huge performance gap.

While using Object with type checks may sound like a quick win, better solution would be to refactor code to extract interfaces or something like that so all code will be statically typed without runtime checks.

@nex3 nex3 added the JavaScript Issues particular to the Node.js distribution label Jul 20, 2018
@nex3
Copy link
Contributor Author

nex3 commented Jul 20, 2018

See this test file as a test case where JS is vastly slower than the Dart VM.

@ogonkov
Copy link

ogonkov commented Aug 30, 2018

For me, full compilation time went to 2 minutes from 20 seconds

@nex3
Copy link
Contributor Author

nex3 commented Aug 30, 2018

@ogonkov Can you share the stylesheets that are running so much slower?

@ogonkov
Copy link

ogonkov commented Aug 30, 2018

I'm afraid no, i will try to reproduce the issue, by excluding sass files, but don't know when

@nex3
Copy link
Contributor Author

nex3 commented Sep 10, 2018

This comment also includes a zip file that's reported to have poor JS performance.

nex3 added a commit that referenced this issue Sep 11, 2018
nex3 added a commit that referenced this issue Sep 11, 2018
nex3 added a commit that referenced this issue Sep 11, 2018
@nex3
Copy link
Contributor Author

nex3 commented Sep 11, 2018

Dart Sass 1.13.4 should substantially improve performance for many of the worst cases. I've verified that it speeds up @alex-page's a11ycolor example from about 11s to about 3s on my computer, which puts it within the expected 2-3x slower than the Dart VM. If anyone has any more examples where the JS speed is 4x or more slower than LibSass or the Dart VM, please include them here.

I'm keeping this issue open because I still think there are likely to be code changes we can make that will further improve the JS speed.

@alex-page
Copy link
Contributor

Wow thanks for all your hard work @nex3 I will definitely do some experimentation on my end!

@ogonkov
Copy link

ogonkov commented Sep 14, 2018

Upgrading to dart-sass@1.13.4, seems make a trick, compilation time drops to 47s, much better than 1m 40s

@nex3
Copy link
Contributor Author

nex3 commented Jul 12, 2019

I've been doing some light profiling, and it seems that our dash-normalized maps cause a pretty hefty amount of overhead in logic-heavy benchmarks like Susy. We should consider alternatives, such as eagerly normalizing and storing the original names elsewhere or determining them from source spans. We could even go more aggressive and convert intra-file name references to indexes into a pre-allocated list of values, although this would require substantially more up-front complexity.

@nex3
Copy link
Contributor Author

nex3 commented Jul 12, 2019

We could even go more aggressive and convert intra-file name references to indexes into a pre-allocated list of values, although this would require substantially more up-front complexity.

I experimentally implemented this. My implementation was extremely hacky, but I wanted to see whether it would provide a substantial enough improvement to warrant investing in more heavily. I got to the point where about 80% of the variable accesses across the Susy benchmark were being resolved via index rather than via name, and while this did produce a measurable performance improvement, it was only about 1.06x faster on Node and 1.15x faster on the Dart VM.

Notably, this is slower than just replacing dash-normalized maps with normal Dart maps, likely because the latter also improves function and mixin look-up times, while the former is mostly useful for local variables. This leads me to think that index-based variables are not a particularly fertile avenue for further experimentation.

@ronnyek
Copy link

ronnyek commented Sep 23, 2020

I know angular at some point replaced node-sass with dart-sass (presumably the js version) and I'd say I went from having our builds spend negligible amount of time on scss compilation (honestly a handful of seconds) to at least 2 minutes.

I know you guys have nothing to do with angular side of things, but I'd say its a pretty significant increase in time.

Is there currently any way to increase verbosity of logging to get more of an idea where time is spent in a build? I'm aware of --trace, that seems to just expand stack traces better.

@nex3
Copy link
Contributor Author

nex3 commented Sep 23, 2020

Although improving the build times of the JS output is something we'd like to do, it's unlikely we'll be able to get it to approach Node Sass's build times simply because JS is inherently a substantially slower language than C++. However, we're working on a Node.js host for embedded Dart Sass which will run the natively-compiled Dart code as a subprocess and should substantially speed up compile times. Keep an eye on that repo!

@ronnyek
Copy link

ronnyek commented Dec 3, 2020

I've played with web assembly and know how you can compile things to webassembly targets and get almost native performance... is that something that could be utilized to achieve near native runtime peformance, without having to rely on distributing native binaries? (Not familiar with how this may or may not be supported with node or npm)

@ronnyek
Copy link

ronnyek commented Dec 3, 2020

Are there things we can turn on for verbose/debugging output (for sass the npm package)? I'd like to try and identify if we're doing something with our stylesheets that inherently causes massive delays. At this point I'm inclined to move style preprocessing stuff completely out of this project, because of the massive hit while compiling...

@nex3
Copy link
Contributor Author

nex3 commented Dec 4, 2020

I've played with web assembly and know how you can compile things to webassembly targets and get almost native performance... is that something that could be utilized to achieve near native runtime peformance, without having to rely on distributing native binaries? (Not familiar with how this may or may not be supported with node or npm)

Dart has started experimenting with support for WebAssembly. If/when that becomes production-ready, we'll certainly explore it as another possible avenue for improving performance.

Are there things we can turn on for verbose/debugging output (for sass the npm package)? I'd like to try and identify if we're doing something with our stylesheets that inherently causes massive delays. At this point I'm inclined to move style preprocessing stuff completely out of this project, because of the massive hit while compiling...

There's not a flag you can flip, ad I'm not sure exactly what information would be useful there. If you're trying to inspect the performance issues, you're probably better off using a profiling tool of some sort.

@gkatsanos
Copy link

gkatsanos commented Dec 9, 2020

Hi, I was wondering if meanwhile we can use the dart vm without JS based app somehow? I attempted to move from node-sass to sass and we went from 2 minutes to 5 minutes of compilation time. (I was actually hoping dart-sass will improve compilation time..) (which is obviously not going to happen anytime soon?) (the issue dates from 2017..)
Not to make this political or anything, but if Dart-Sass is supported by Google, arent there resources to dedicate to this issue? I just find it a little hard to believe.

@ogonkov
Copy link

ogonkov commented Dec 9, 2020

Do you use fibers package @gkatsanos ?

@nex3
Copy link
Contributor Author

nex3 commented Dec 11, 2020

@ronnyek I'm sorry to hear that. How much did you performance degrade, and what absolute timings are we talking about?

@ronnyek
Copy link

ronnyek commented Jan 25, 2021

Apologies about the delay, I spent quite a bit of time trying to get a better idea bout where the time is being spent. It looks like the meat of our time compiling scss is actually compiling elasticui's scss files (https://github.com/elastic/eui/tree/master/src). Before switching to dart-sass, it would literally be negligible... like I wouldn't even notice any time being spent compiling. Afterwards, it was at least a 45sec hit any time I would make changes that had anything to do with elastic's scss.

That being said, I'm no expert with scss, but didn't seem to notice any glaring problems with their scss usage. I didn't see much in the way of import or link or whatever that functionality was that some people reported caused their performance problems.

@nex3
Copy link
Contributor Author

nex3 commented Jan 28, 2021

When you say "anything to do with elastic's scss", what does that mean specifically? Are there mixins or functions you're calling that are causing problems, or is it just loading the library at all?

@nex3
Copy link
Contributor Author

nex3 commented Jan 28, 2021

Digging into this, it looks like the biggest issue is that EUI is implementing its own version of math.pow() (and an nth-root function that doesn't use math.pow()). If I fix those two functions, the compilation time of EUI's light_theme.scss goes from about 20s using the standalone executable to under 2s. I haven't measured the JS version, but I'd expect if anything an even more dramatic speed improvement.

To a degree, this is inevitable: Sass is never going to be a great language for heavy numeric computation, and it doesn't make a lot of sense for us to optimize for that use-case rather than encouraging users to use built-in functions that are much more efficient. That said, I'll take a look at the number implementation and see if there's any low-hanging fruit that can be easily optimized so even the manual pow() implementation gets a bit faster.

@ronnyek
Copy link

ronnyek commented Jan 28, 2021

That is great information, I really appreciate that you went above and beyond IMO to do even that much investigation. Not that I am anyone with a say, but I agree about the fact that Sass wasn't built to optimize computationally expensive stuff, or that it should be. I'll see if we can make changes manually here that net the same benefits.

@gkatsanos
Copy link

Why are you guys digging into a specific library? We run a VueJS app with a bunch of components with SCSS , no framework/library, and still the difference in compilation time was severals tens of percentage points apart.. I fail to see why the discussion diverted that much.

@ronnyek
Copy link

ronnyek commented Jan 28, 2021

Well it sounds like in this specific case, there are probably things that this library does, that actually amplifies the difference in speed. the dart-sass js library is slower, but it was exceptionally slower because of something this library was doing, and it sounds like fixing this problem in the library actually speeds up sass cli speeds too.

@gkatsanos
Copy link

Just for my own curiosity, there must a solid explanation as to why node-sass is so much faster compared to the JS implementation of dart-sass?

@nex3
Copy link
Contributor Author

nex3 commented Jan 29, 2021

Just for my own curiosity, there must a solid explanation as to why node-sass is so much faster compared to the JS implementation of dart-sass?

Node-Sass is written in C++, which is fundamentally a faster language than JS by virtue of being statically-typed and compiled to native machine code. Fortunately, it's not fundamentally faster than Dart, which is why we're working on Embedded Sass as a way of providing the Dart's speed with a JS API. Look for a beta launch in the next month or so!

nex3 added a commit that referenced this issue Feb 1, 2021
This allows us to use more efficient implementations in the common
case where numbers don't have complex units.

See #113
nex3 added a commit that referenced this issue Feb 3, 2021
This allows us to use more efficient implementations in the common
case where numbers don't have complex units.

See #113
@mattiagiuffrida-st
Copy link

mattiagiuffrida-st commented Aug 16, 2021

@nex3 with libsass now deprecated, my team (and I suspect many other people) are now moving to dart-sass.
This in the webpack world means using the sass npm package and the sass-loader.
However, people will be surprised at the difference in performance.

Our codebase is currently being built using libsass in around 14s, with dart-sass it takes 100s (that's roughly 6.5x slower, confirming other people reports I've found navigating through the issues).
This not only affects full compilation: a single file edit is refreshed on the browser in less than 2s under libsass, but it takes almost 9s under dart-sass, so this negatively impacts development as well, and quite heavily!

It would be great to:

  • Know if there's any quick-win my team and others can put in place, other than reverting to libsass (and thus losing all the latest changes to the codebase like @use and modules).
  • Have a better estimate of the roadmap to the Embedded Sass you mentioned above, which I can see has reached beta 8 and has a node host in beta as well, but it's still unclear on when it will be available for use in Webpacker and other tools.

I really like the Sass team vision and what they're doing, including all the language improvement brought by dart-sass. However speed of development will always take priority in any commercial team and thus I believe solving the performance issue should really be the Sass team number one priority right now.


EDIT: I should clarify that the comparison above is not between node-sass and dart-sass, as our current codebase is Rails-based and built by Sprockets, using this Ruby wrapper.
I just tried replacing dart-sass with node-sass in my webpack configuration, and compilation went down to ~70s (single file changes down to ~5s).
Although this is still faster than dart-sass, it's more in line with the expected 2x speed, rather than the 6.5x diff I previously saw.

But then a new question arise. How can sassc-rails be so blazing fast compared to both node-sass and dart-sass?
Ruby is definitely not the fastest language out there 😂

@nex3
Copy link
Contributor Author

nex3 commented Aug 17, 2021

@mattiagiuffrida-st We're still hard at work on the embedded host. Much of our time in that direction recently was focused on the new JS API, which we intend to launch concurrently with the embedded host.

It's worth noting that there is a beta release of the embedded host available today (although the Sass version it wraps is a little outdated).

@mattiagiuffrida-st
Copy link

mattiagiuffrida-st commented Aug 18, 2021

Thanks for the feedback @nex3 and great to hear things are moving forward.
I know this is a tricky question but do you have any estimate on when these will be publicly available for use?

Also, I'd be happy to give the beta a try (I also noticed an updated version was released), but the repo lacks any usage instruction at the moment. Should I simply add the npm package and pass it to sass-loader as the implementation?

EDIT: tried my luck, but that doesn't work 😅. I get an Unknown Sass implementation error

@nex3
Copy link
Contributor Author

nex3 commented Aug 18, 2021

We don't usually like to publicly commit to timelines because so many things can change so quickly. All I can say is that we're actively working on the embedded host, and you can follow that progress on its repo.

Also, I'd be happy to give the beta a try (I also noticed an updated version was released), but the repo lacks any usage instruction at the moment. Should I simply add the npm package and pass it to sass-loader as the implementation?

The embedded host doesn't yet support importer or function plugins, so it's not going to work with existing integration tools that rely on those. However, you can call render() manually.

@PaulVrugt
Copy link

so any progress on this? This issue is now 5 years old and dart-sass still seems way slower that node-sass. Our CI builds are significantly slower since we migrated from node-sass to dart-sass.

@nex3
Copy link
Contributor Author

nex3 commented May 25, 2022

Yes, the embedded host has been released and we recommend that for performance-critical Sass compilation.

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

No branches or pull requests

8 participants