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

Implement sass --embedded in pure JS mode #2413

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,24 +126,40 @@ jobs:
working-directory: sass-spec

sass_spec_js_embedded:
name: 'JS API Tests | Embedded | Node ${{ matrix.node-version }} | ${{ matrix.os }}'
name: "JS API Tests | Embedded ${{ matrix.js && 'Pure JS' || 'Dart' }} | Node ${{ matrix.node-version }} | ${{ matrix.os }}"
runs-on: ${{ matrix.os }}
if: "github.event_name != 'pull_request' || !contains(github.event.pull_request.body, 'skip sass-embedded')"

strategy:
fail-fast: false
matrix:
js: [true, false]
os: [ubuntu-latest, windows-latest, macos-latest]
node-version: ['lts/*']
include:
# Test older LTS versions
- os: ubuntu-latest
- js: true
os: ubuntu-latest
dart_channel: stable
node-version: lts/-1
- os: ubuntu-latest
- js: true
os: ubuntu-latest
dart_channel: stable
node-version: lts/-2
- os: ubuntu-latest
- js: true
os: ubuntu-latest
dart_channel: stable
node-version: lts/-3
- js: false
os: ubuntu-latest
dart_channel: stable
node-version: lts/-1
- js: false
os: ubuntu-latest
dart_channel: stable
node-version: lts/-2
- js: false
os: ubuntu-latest
dart_channel: stable
node-version: lts/-3

Expand All @@ -166,19 +182,13 @@ jobs:
- name: Initialize embedded host
run: |
npm install
npm run init -- --compiler-path=.. --language-path=../build/language
npm run init -- --compiler-path=.. --language-path=../build/language --compiler-js=${{ matrix.js && 'true' || 'false' }}
npm run compile
mv {`pwd`/,dist/}lib/src/vendor/dart-sass
working-directory: embedded-host-node

- name: Version info
run: |
path=embedded-host-node/dist/lib/src/vendor/dart-sass/sass
if [[ -f "$path.cmd" ]]; then "./$path.cmd" --version
elif [[ -f "$path.bat" ]]; then "./$path.bat" --version
elif [[ -f "$path.exe" ]]; then "./$path.exe" --version
else "./$path" --version
fi
run: node dist/bin/sass.js --version
working-directory: embedded-host-node

- name: Run tests
run: npm run js-api-spec -- --sassPackage ../embedded-host-node --sassSassRepo ../build/language
Expand Down
4 changes: 1 addition & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,7 @@ an API for users to invoke Sass and define custom functions and importers.
* `sass --embedded --version` prints `versionResponse` with `id = 0` in JSON and
exits.

The `--embedded` command-line flag is not available when you install Dart Sass
as an [npm package]. No other command-line flags are supported with
`--embedded`.
No other command-line flags are supported with `--embedded`.

[npm package]: #from-npm

Expand Down
3 changes: 1 addition & 2 deletions bin/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import 'package:sass/src/io.dart';
import 'package:sass/src/stylesheet_graph.dart';
import 'package:sass/src/utils.dart';
import 'package:sass/src/embedded/executable.dart'
// Never load the embedded protocol when compiling to JS.
if (dart.library.js) 'package:sass/src/embedded/unavailable.dart'
if (dart.library.js) 'package:sass/src/embedded/js/executable.dart'
as embedded;

Future<void> main(List<String> args) async {
Expand Down
27 changes: 14 additions & 13 deletions lib/src/embedded/compilation_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@
// https://opensource.org/licenses/MIT.

import 'dart:convert';
import 'dart:io';
import 'dart:isolate';
import 'dart:io' if (dart.library.js) 'js/io.dart';
import 'dart:isolate' if (dart.library.js) 'js/isolate.dart';
import 'dart:typed_data';

import 'package:native_synchronization/mailbox.dart';
import 'package:path/path.dart' as p;
import 'package:protobuf/protobuf.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:sass/sass.dart' as sass;
import 'package:sass/src/importer/node_package.dart' as npi;

import '../io.dart' show FileSystemException;
import '../logger.dart';
import '../value/function.dart';
import '../value/mixin.dart';
Expand All @@ -23,6 +23,7 @@ import 'host_callable.dart';
import 'importer/file.dart';
import 'importer/host.dart';
import 'logger.dart';
import 'sync_receive_port.dart';
import 'util/proto_extensions.dart';
import 'utils.dart';

Expand All @@ -35,8 +36,8 @@ final _outboundRequestId = 0;
/// A class that dispatches messages to and from the host for a single
/// compilation.
final class CompilationDispatcher {
/// The mailbox for receiving messages from the host.
final Mailbox _mailbox;
/// The synchronous receive port for receiving messages from the host.
final SyncReceivePort _receivePort;

/// The send port for sending messages to the host.
final SendPort _sendPort;
Expand All @@ -52,8 +53,8 @@ final class CompilationDispatcher {
late Uint8List _compilationIdVarint;

/// Creates a [CompilationDispatcher] that receives encoded protocol buffers
/// through [_mailbox] and sends them through [_sendPort].
CompilationDispatcher(this._mailbox, this._sendPort);
/// through [_receivePort] and sends them through [_sendPort].
CompilationDispatcher(this._receivePort, this._sendPort);

/// Listens for incoming `CompileRequests` and runs their compilations.
void listen() {
Expand Down Expand Up @@ -366,14 +367,14 @@ final class CompilationDispatcher {
message.writeToCodedBufferWriter(protobufWriter);

// Add one additional byte to the beginning to indicate whether or not the
// compilation has finished (1) or encountered a fatal error (2), so the
// [IsolateDispatcher] knows whether to treat this isolate as inactive or
// close out entirely.
// compilation has finished (1) or encountered a fatal error (exitCode), so
// the [IsolateDispatcher] knows whether to treat this isolate as inactive
// or close out entirely.
var packet = Uint8List(
1 + _compilationIdVarint.length + protobufWriter.lengthInBytes);
packet[0] = switch (message.whichMessage()) {
OutboundMessage_Message.compileResponse => 1,
OutboundMessage_Message.error => 2,
OutboundMessage_Message.error => exitCode,
_ => 0
};
packet.setAll(1, _compilationIdVarint);
Expand All @@ -384,9 +385,9 @@ final class CompilationDispatcher {
/// Receive a packet from the host.
Uint8List _receive() {
try {
return _mailbox.take();
return _receivePort.receive();
} on StateError catch (_) {
// The [_mailbox] has been closed, exit the current isolate immediately
// The [SyncReceivePort] has been closed, exit the current isolate immediately
// to avoid bubble the error up as [SassException] during [_sendRequest].
Isolate.exit();
}
Expand Down
10 changes: 10 additions & 0 deletions lib/src/embedded/concurrency.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'dart:ffi';

/// More than MaxMutatorThreadCount isolates in the same isolate group
/// can deadlock the Dart VM.
/// See https://github.com/sass/dart-sass/pull/2019
int get concurrencyLimit => sizeOf<IntPtr>() <= 4 ? 7 : 15;
30 changes: 7 additions & 23 deletions lib/src/embedded/executable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,19 @@
// https://opensource.org/licenses/MIT.

import 'dart:io';
import 'dart:convert';

import 'package:stream_channel/stream_channel.dart';

import 'isolate_dispatcher.dart';
import 'options.dart';
import 'util/length_delimited_transformer.dart';

void main(List<String> args) {
switch (args) {
case ["--version", ...]:
var response = IsolateDispatcher.versionResponse();
response.id = 0;
stdout.writeln(
JsonEncoder.withIndent(" ").convert(response.toProto3Json()));
return;

case [_, ...]:
stderr.writeln(
"sass --embedded is not intended to be executed with additional "
"arguments.\n"
"See https://github.com/sass/dart-sass#embedded-dart-sass for "
"details.");
// USAGE error from https://bit.ly/2poTt90
exitCode = 64;
return;
if (parseOptions(args)) {
IsolateDispatcher(
StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false)
.transform(lengthDelimited),
gracefulShutdown: false)
.listen();
}

IsolateDispatcher(
StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false)
.transform(lengthDelimited))
.listen();
}
62 changes: 35 additions & 27 deletions lib/src/embedded/isolate_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@
// https://opensource.org/licenses/MIT.

import 'dart:async';
import 'dart:ffi';
import 'dart:io';
import 'dart:isolate';
import 'dart:io' if (dart.library.js) 'js/io.dart';
import 'dart:typed_data';

import 'package:native_synchronization/mailbox.dart';
import 'package:pool/pool.dart';
import 'package:protobuf/protobuf.dart';
import 'package:stream_channel/stream_channel.dart';

import 'compilation_dispatcher.dart';
import 'concurrency.dart' if (dart.library.js) 'js/concurrency.dart';
import 'embedded_sass.pb.dart';
import 'reusable_isolate.dart';
import 'isolate_main.dart' if (dart.library.js) 'js/isolate_main.dart';
import 'reusable_isolate.dart' if (dart.library.js) 'js/reusable_isolate.dart';
import 'util/proto_extensions.dart';
import 'utils.dart';

Expand All @@ -25,6 +23,10 @@ class IsolateDispatcher {
/// The channel of encoded protocol buffers, connected to the host.
final StreamChannel<Uint8List> _channel;

/// Whether to wait for all worker isolates to exit before exiting the main
/// isolate or not.
final bool _gracefulShutdown;

/// All isolates that have been spawned to dispatch to.
///
/// Only used for cleaning up the process when the underlying channel closes.
Expand All @@ -38,16 +40,13 @@ class IsolateDispatcher {

/// A pool controlling how many isolates (and thus concurrent compilations)
/// may be live at once.
///
/// More than MaxMutatorThreadCount isolates in the same isolate group
/// can deadlock the Dart VM.
/// See https://github.com/sass/dart-sass/pull/2019
final _isolatePool = Pool(sizeOf<IntPtr>() <= 4 ? 7 : 15);
final _isolatePool = Pool(concurrencyLimit);

/// Whether [_channel] has been closed or not.
var _closed = false;

IsolateDispatcher(this._channel);
IsolateDispatcher(this._channel, {bool gracefulShutdown = true})
: _gracefulShutdown = gracefulShutdown;

void listen() {
_channel.stream.listen((packet) async {
Expand Down Expand Up @@ -96,8 +95,12 @@ class IsolateDispatcher {
}, onError: (Object error, StackTrace stackTrace) {
_handleError(error, stackTrace);
}, onDone: () {
_closed = true;
_allIsolates.stream.listen((isolate) => isolate.kill());
if (_gracefulShutdown) {
_closed = true;
_allIsolates.stream.listen((isolate) => isolate.kill());
} else {
exit(exitCode);
}
});
}

Expand All @@ -112,7 +115,7 @@ class IsolateDispatcher {
isolate = _inactiveIsolates.first;
_inactiveIsolates.remove(isolate);
} else {
var future = ReusableIsolate.spawn(_isolateMain,
var future = ReusableIsolate.spawn(isolateMain,
onError: (Object error, StackTrace stackTrace) {
_handleError(error, stackTrace);
});
Expand All @@ -124,11 +127,11 @@ class IsolateDispatcher {
var fullBuffer = message as Uint8List;

// The first byte of messages from isolates indicates whether the entire
// compilation is finished (1) or if it encountered an error (2). Sending
// this as part of the message buffer rather than a separate message
// avoids a race condition where the host might send a new compilation
// request with the same ID as one that just finished before the
// [IsolateDispatcher] receives word that the isolate with that ID is
// compilation is finished (1) or if it encountered an error (exitCode).
// Sending this as part of the message buffer rather than a separate
// message avoids a race condition where the host might send a new
// compilation request with the same ID as one that just finished before
// the [IsolateDispatcher] receives word that the isolate with that ID is
// done. See sass/dart-sass#2004.
var category = fullBuffer[0];
var packet = Uint8List.sublistView(fullBuffer, 1);
Expand All @@ -142,9 +145,14 @@ class IsolateDispatcher {
_inactiveIsolates.add(isolate);
resource.release();
_channel.sink.add(packet);
case 2:
default:
_channel.sink.add(packet);
exit(exitCode);
exitCode = category;
if (_gracefulShutdown) {
_channel.sink.close();
} else {
exit(exitCode);
}
}
});

Expand All @@ -168,7 +176,11 @@ class IsolateDispatcher {
{int? compilationId, int? messageId}) {
sendError(compilationId ?? errorId,
handleError(error, stackTrace, messageId: messageId));
_channel.sink.close();
if (_gracefulShutdown) {
_channel.sink.close();
} else {
exit(exitCode);
}
}

/// Sends [message] to the host.
Expand All @@ -179,7 +191,3 @@ class IsolateDispatcher {
void sendError(int compilationId, ProtocolError error) =>
_send(compilationId, OutboundMessage()..error = error);
}

void _isolateMain(Mailbox mailbox, SendPort sendPort) {
CompilationDispatcher(mailbox, sendPort).listen();
}
12 changes: 12 additions & 0 deletions lib/src/embedded/isolate_main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2024 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'dart:isolate' show SendPort;

import 'compilation_dispatcher.dart';
import 'sync_receive_port.dart';

void isolateMain(SyncReceivePort receivePort, SendPort sendPort) {
CompilationDispatcher(receivePort, sendPort).listen();
}
10 changes: 10 additions & 0 deletions lib/src/embedded/js/concurrency.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2024 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'dart:js_interop';

@JS('os.cpus')
external JSArray _cpus();

int get concurrencyLimit => _cpus().length;
Loading
Loading