Help understanding performance implications of certain Sass features when using compileAsync

[aaronlademann-wf]

Hi Natalie!

I have been tasked with troubleshooting some substandard pub build_runner build times within Workiva's (massive) Dart application ecosystem as we try to finish our transition to the Dart 2 SDK.

One of the things that takes a very long time is compiling CSS in packages that depend on sass_builder. I have ruled out sass_builder itself as the culprit by confirming that similarly sluggish .scss compile times can be reproduced in our ecosystem by calling sass.compileAsync() directly instead of relying on sass_builder to make that call.

Unfortunately, none of the Workiva .scss source code is open sourced - but I will do my best to explain the situation in hopes that you might be able to give me some clues about which features we might be using in excess that would be most likely to result in such an enormous compile time delta compared to sass.compile().

Raw Numbers

As a control for my troubleshooting, I have set up a little project locally that has all of the Twitter Bootstrap .scss files, and then I compile bootstrap.scss using both sass.compile() and sass.compileAsync().

Compile Time For Control: bootstrap.scss

File Name	compile()	compileAsync()
bootstrap.scss	791ms	1009ms

As you can see, while slightly slower (~1.25x) - its certainly not earth-shaking.

Compile Time For an Internal Workiva Sass File Similar to Bootstrap

File Name	compile()	compileAsync()
workiva-bootstrap.scss	2930ms	15462ms

As you can see, async compilation for Workiva's "bootstrap" that we use is 5x slower.

Differences Between Workiva's "Bootstrap" and the original Twitter Bootstrap

1. The biggest difference I can speak to with certainty when comparing Workiva's "bootstrap" to Twitter bootstrap is that we make extensive use of Sass Maps for variables / configuration. Many of them have nested Maps - and we utilize map-get / map-get-deep extensively to recursively access nested key/value pairs.

2. The other difference is that there is significantly more granularity when it comes to the number of .scss partials in Workiva's "bootstrap" source.

---

Since the extent of our Sass Map usage is such that even just spiking out the performance implications of removing them would take many days (possibly weeks) - not hours, I'm trying to take a very measured approach to any further efforts to reduce compilation time.

And so, what I'm hoping to get from you is your expert opinion on the likelihood of those two things having a clear causal relationship with the significant increase in compile time when calling compileAsync vs. compile.

Your opinion(s) will help me "sell" with a reasonable amount of confidence any potential refactors / spikes to internal stakeholders.

Thank you so much for your time @nex3 !!!

cc/ @evanweible-wf @robbecker-wf @greglittlefield-wf @joelleibow-wf

[nex3]

I suspect the real issue here isn't so much that something in particular is wrong with maps in asynchronous mode, but that working asynchronously is slow in general (to what degree this is an issue with the Dart VM's implementation of async/await versus a fundamental issue with the asynchronous paradigm, I don't know). Map operations likely just involve a higher number of function calls for whatever reason, which are substantially more expensive asynchronously.

I think the best solution here is to just not call compileAsync(). The Dart VM provides the waitFor() function, and I strongly recommend that performance-sensitive users that need asynchronous behavior in importers or functions use it to call compile() instead.

[aaronlademann-wf]

@nex3 the problem we're facing is that we're utilizing the sass_builder package so that we can use package:<some_dependency>/<some_scss_file> imports, and sass_builder uses compileAsync.

I am in the process of attempting to write a standalone dart script that calls compile, but I am currently stuck on figuring out why relative imports within the .scss file imported by another package via a package: import don't work.

I wonder if the sass_builder could utilize waitFor instead of compileAsync?

[aaronlademann-wf]

@nex3 I have an implementation of sass_builder locally that is now calling sass.compile() after only a single await within its Builder.build() method:

final buildStepAsString = await buildStep.readAsString(inputId);
final cssOutput = sass.compileString(
        buildStepAsString,
        syntax: sass.Syntax.forPath(inputId.path),
        importers: [new BuildImporter(buildStep, buildStepAsString)],
        style: _getValidOutputStyle());

Unfortunately, the performance implications are the same.

So I guess it begs the question... if async is really this slow at IO operations, why would the new Dart build system be asynchronous? It seems - unless I'm missing something - that there are some fairly major performance issues within the build system pertaining to parsing / locating Uris. This also makes me think that its not a matter of any particular Sass construct that is slow (as you correctly alluded to) - but rather the sheer number of / the granularity of our @imports that could be what is causing such an extreme performance degradation.

cc/ @kevmoo @robbecker-wf @evanweible-wf

[nex3]

the problem we're facing is that we're utilizing the sass_builder package so that we can use package:<some_dependency>/<some_scss_file> imports, and sass_builder uses compileAsync.

There may be other reasons to use sass_builder, but for what it's worth, Dart Sass does natively support package: imports (when running from Dart source). All you have to do is pass the packageResolver option to compile() or compileAsync().

I have an implementation of sass_builder locally that is now calling sass.compile() after only a single await within its Builder.build() method:

final buildStepAsString = await buildStep.readAsString(inputId);
final cssOutput = sass.compileString(
        buildStepAsString,
        syntax: sass.Syntax.forPath(inputId.path),
        importers: [new BuildImporter(buildStep, buildStepAsString)],
        style: _getValidOutputStyle());

Unfortunately, the performance implications are the same.

Something doesn't add up here. If I understand you right, you're saying that:

• The Sass Dart API with compileStringAsync() takes about 15s.
• The Sass Dart API with compileString() takes about 3s.
• sass_builder with compileStringAsync() takes about 15s.
• sass_builder with compileString() still takes about 15s.

Something must be strange about your sass_builder test setup, because even if BuildImporter causes compileString() to be very slow, compileStringAsync() should be yet slower. Asynchrony adds substantial overhead even in the relatively quick case of the Bootstrap tests, and that should be even more visible if you're doing a bunch of map operations. I wonder if somehow your Builder.build() edit didn't get picked up by the build system?

As an aside: I know you said that the Workiva Sass files aren't open source, but if you'd ever be willing to open source them (or a portion of them that uses maps heavily), they'd be very valuable as a benchmark resource that we can use to test the effect of changes on real-world Sass stylesheets.

[aaronlademann-wf]

@nex3

Dart Sass does natively support package: imports (when running from Dart source)

I've tried creating a standalone compiler that utilizes packageResolver - but it seems that relative paths are broken once the compiler enters a file that was imported via a package: import.

// package_two/sass/app.scss

@import 'package:package_one/sass/api';

// package_one/sass/_api.scss
.foo {
  color: rebeccapurple;
}

// Throws "Can't find stylesheet to import." when _api.scss is imported by package_two
// > https://github.com/sass/dart-sass/blob/a0e63ac704c6c98c8a9123872e3a8c1f312bb2ed/lib/src/visitor/evaluate.dart#L859
@import '../some/relative/path/_some_partial.scss'; 

[nex3]

Can you file a separate issue for that? Relative imports are definitely supposed to work there.

[aaronlademann-wf]

@nex3 I filed #631 for the relative path issue along with a reduced test case you can pull down locally.

[nex3]

@aaronlademann-wf Do you have any more questions I can answer, or should I close this issue?