Skip to content

Commit

Permalink
perf(@angular-devkit/build-angular): use Sass worker pool for Sass su…
Browse files Browse the repository at this point in the history
…pport in esbuild builder

When using the experimental esbuild-based browser application builder, Sass stylesheets will
now be processed using a worker pool that is currently also used by the default Webpack-based
builder. This allows up to four stylesheets to be processed in parallel and keeps the main thread
available for other build tasks. On projects with a large amount of Sass stylesheets, this change
provided up to a 25% improvement in build times based on initial testing.

(cherry picked from commit e1ca878)
  • Loading branch information
clydin authored and dgp1130 committed Oct 31, 2022
1 parent fb4ead2 commit 9d83fb9
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { SourceFileCache, createCompilerPlugin } from './compiler-plugin';
import { bundle, logMessages } from './esbuild';
import { logExperimentalWarnings } from './experimental-warnings';
import { NormalizedBrowserOptions, normalizeOptions } from './options';
import { shutdownSassWorkerPool } from './sass-plugin';
import { Schema as BrowserBuilderOptions } from './schema';
import { bundleStylesheetText } from './stylesheets';
import { ChangedFiles, createWatcher } from './watcher';
Expand Down Expand Up @@ -437,6 +438,8 @@ export async function* buildEsbuildBrowser(

// Finish if watch mode is not enabled
if (!initialOptions.watch) {
shutdownSassWorkerPool();

return;
}

Expand Down Expand Up @@ -476,6 +479,7 @@ export async function* buildEsbuildBrowser(
await watcher.close();
// Cleanup incremental rebuild state
result.dispose();
shutdownSassWorkerPool();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,52 @@
*/

import type { PartialMessage, Plugin, PluginBuild } from 'esbuild';
import { readFile } from 'node:fs/promises';
import { dirname, relative } from 'node:path';
import { fileURLToPath } from 'node:url';
import type { CompileResult } from 'sass';
import { fileURLToPath, pathToFileURL } from 'node:url';
import type { CompileResult, Exception } from 'sass';
import { SassWorkerImplementation } from '../../sass/sass-service';

let sassWorkerPool: SassWorkerImplementation;

function isSassException(error: unknown): error is Exception {
return !!error && typeof error === 'object' && 'sassMessage' in error;
}

export function shutdownSassWorkerPool(): void {
sassWorkerPool?.close();
}

export function createSassPlugin(options: { sourcemap: boolean; loadPaths?: string[] }): Plugin {
return {
name: 'angular-sass',
setup(build: PluginBuild): void {
let sass: typeof import('sass');

build.onLoad({ filter: /\.s[ac]ss$/ }, async (args) => {
// Lazily load Sass when a Sass file is found
sass ??= await import('sass');
sassWorkerPool ??= new SassWorkerImplementation();

const warnings: PartialMessage[] = [];
try {
const warnings: PartialMessage[] = [];
// Use sync version as async version is slower.
const { css, sourceMap, loadedUrls } = sass.compile(args.path, {
const data = await readFile(args.path, 'utf-8');
const { css, sourceMap, loadedUrls } = await sassWorkerPool.compileStringAsync(data, {
url: pathToFileURL(args.path),
style: 'expanded',
loadPaths: options.loadPaths,
sourceMap: options.sourcemap,
sourceMapIncludeSources: options.sourcemap,
quietDeps: true,
logger: {
warn: (text, _options) => {
warn: (text, { deprecation, span }) => {
warnings.push({
text,
text: deprecation ? 'Deprecation' : text,
location: span && {
file: span.url && fileURLToPath(span.url),
lineText: span.context,
// Sass line numbers are 0-based while esbuild's are 1-based
line: span.start.line + 1,
column: span.start.column,
},
notes: deprecation ? [{ text }] : undefined,
});
},
},
Expand All @@ -48,16 +67,17 @@ export function createSassPlugin(options: { sourcemap: boolean; loadPaths?: stri
warnings,
};
} catch (error) {
if (error instanceof sass.Exception) {
if (isSassException(error)) {
const file = error.span.url ? fileURLToPath(error.span.url) : undefined;

return {
loader: 'css',
errors: [
{
text: error.toString(),
text: error.message,
},
],
warnings,
watchFiles: file ? [file] : undefined,
};
}
Expand Down
26 changes: 25 additions & 1 deletion packages/angular_devkit/build_angular/src/sass/sass-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
Exception,
FileImporter,
Importer,
Logger,
SourceSpan,
StringOptionsWithImporter,
StringOptionsWithoutImporter,
} from 'sass';
Expand Down Expand Up @@ -48,6 +50,7 @@ interface RenderRequest {
id: number;
workerIndex: number;
callback: RenderCallback;
logger?: Logger;
importers?: Importers[];
previousResolvedModules?: Set<string>;
}
Expand All @@ -68,6 +71,12 @@ interface RenderResponseMessage {
id: number;
error?: Exception;
result?: Omit<CompileResult, 'loadedUrls'> & { loadedUrls: string[] };
warnings?: {
message: string;
deprecation: boolean;
stack?: string;
span?: Omit<SourceSpan, 'url'> & { url?: string };
}[];
}

/**
Expand Down Expand Up @@ -153,13 +162,14 @@ export class SassWorkerImplementation {
resolve(result);
};

const request = this.createRequest(workerIndex, callback, importers);
const request = this.createRequest(workerIndex, callback, logger, importers);
this.requests.set(request.id, request);

this.workers[workerIndex].postMessage({
id: request.id,
source,
hasImporter: !!importers?.length,
hasLogger: !!logger,
options: {
...serializableOptions,
// URL is not serializable so to convert to string here and back to URL in the worker.
Expand Down Expand Up @@ -200,6 +210,18 @@ export class SassWorkerImplementation {
this.requests.delete(response.id);
this.availableWorkers.push(request.workerIndex);

if (response.warnings && request.logger?.warn) {
for (const { message, span, ...options } of response.warnings) {
request.logger.warn(message, {
...options,
span: span && {
...span,
url: span.url ? pathToFileURL(span.url) : undefined,
},
});
}
}

if (response.result) {
request.callback(undefined, {
...response.result,
Expand Down Expand Up @@ -274,12 +296,14 @@ export class SassWorkerImplementation {
private createRequest(
workerIndex: number,
callback: RenderCallback,
logger: Logger | undefined,
importers: Importers[] | undefined,
): RenderRequest {
return {
id: this.idCounter++,
workerIndex,
callback,
logger,
importers,
};
}
Expand Down
83 changes: 64 additions & 19 deletions packages/angular_devkit/build_angular/src/sass/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import { Exception, StringOptionsWithImporter, compileString } from 'sass';
import { Exception, SourceSpan, StringOptionsWithImporter, compileString } from 'sass';
import { fileURLToPath, pathToFileURL } from 'url';
import { MessagePort, parentPort, receiveMessageOnPort, workerData } from 'worker_threads';

Expand All @@ -31,6 +31,10 @@ interface RenderRequestMessage {
* Indicates the request has a custom importer function on the main thread.
*/
hasImporter: boolean;
/**
* Indicates the request has a custom logger for warning messages.
*/
hasLogger: boolean;
}

if (!parentPort || !workerData) {
Expand All @@ -43,11 +47,20 @@ const { workerImporterPort, importerSignal } = workerData as {
importerSignal: Int32Array;
};

parentPort.on('message', ({ id, hasImporter, source, options }: RenderRequestMessage) => {
parentPort.on('message', (message: RenderRequestMessage) => {
if (!parentPort) {
throw new Error('"parentPort" is not defined. Sass worker must be executed as a Worker.');
}

const { id, hasImporter, hasLogger, source, options } = message;
let warnings:
| {
message: string;
deprecation: boolean;
stack?: string;
span?: Omit<SourceSpan, 'url'> & { url?: string };
}[]
| undefined;
try {
if (hasImporter) {
// When a custom importer function is present, the importer request must be proxied
Expand Down Expand Up @@ -75,10 +88,24 @@ parentPort.on('message', ({ id, hasImporter, source, options }: RenderRequestMes
...options,
// URL is not serializable so to convert to string in the parent and back to URL here.
url: options.url ? pathToFileURL(options.url) : undefined,
logger: hasLogger
? {
warn(message, { deprecation, span, stack }) {
warnings ??= [];
warnings.push({
message,
deprecation,
stack,
span: span && convertSourceSpan(span),
});
},
}
: undefined,
});

parentPort.postMessage({
id,
warnings,
result: {
...result,
// URL is not serializable so to convert to string here and back to URL in the parent.
Expand All @@ -91,22 +118,9 @@ parentPort.on('message', ({ id, hasImporter, source, options }: RenderRequestMes
const { span, message, stack, sassMessage, sassStack } = error;
parentPort.postMessage({
id,
warnings,
error: {
span: {
text: span.text,
context: span.context,
end: {
column: span.end.column,
offset: span.end.offset,
line: span.end.line,
},
start: {
column: span.start.column,
offset: span.start.offset,
line: span.start.line,
},
url: span.url ? fileURLToPath(span.url) : undefined,
},
span: convertSourceSpan(span),
message,
stack,
sassMessage,
Expand All @@ -115,9 +129,40 @@ parentPort.on('message', ({ id, hasImporter, source, options }: RenderRequestMes
});
} else if (error instanceof Error) {
const { message, stack } = error;
parentPort.postMessage({ id, error: { message, stack } });
parentPort.postMessage({ id, warnings, error: { message, stack } });
} else {
parentPort.postMessage({ id, error: { message: 'An unknown error has occurred.' } });
parentPort.postMessage({
id,
warnings,
error: { message: 'An unknown error has occurred.' },
});
}
}
});

/**
* Converts a Sass SourceSpan object into a serializable form.
* The SourceSpan object contains a URL property which must be converted into a string.
* Also, most of the interface's properties are get accessors and are not automatically
* serialized when sent back from the worker.
*
* @param span The Sass SourceSpan object to convert.
* @returns A serializable form of the SourceSpan object.
*/
function convertSourceSpan(span: SourceSpan): Omit<SourceSpan, 'url'> & { url?: string } {
return {
text: span.text,
context: span.context,
end: {
column: span.end.column,
offset: span.end.offset,
line: span.end.line,
},
start: {
column: span.start.column,
offset: span.start.offset,
line: span.start.line,
},
url: span.url ? fileURLToPath(span.url) : undefined,
};
}

0 comments on commit 9d83fb9

Please sign in to comment.