Run embedded compilations across multiple isolates #1981
Conversation
|
I just tested this with my host implementation at https://github.com/ntkme/sass-embedded-host-ruby/tree/dart-sass All tests passed. However, there is a behavior difference causing ruby process to hang after everything is finished - the problem turns out to be that after closing stdin, stdout, and stderr for the child process, the dart-sass process somehow did not exit, and ruby process was waiting for dart-sass to exit before it could exit. In the code, it got stuck here: https://github.com/ntkme/sass-embedded-host-ruby/blob/eb3e10cfaf18814663c3aac0b0cf3033a5e07d9b/lib/sass/embedded/compiler.rb#L43 This is different from the previous version where the dart-sass-embedded process would exit after stdin is closed and all inflight requests are completed. |
|
Ruby host can be tested with: git clone https://github.com/ntkme/sass-embedded-host-ruby.git --branch dart-sass
cd sass-embedded-host-ruby
export DART_SASS=/full/path/to/dart-sass-1.63.0-dev-macos-arm64.tar.gz
export EMBEDDED_SASS_PROTOCOL=https://github.com/sass/sass/raw/embedded-2-impl/spec/embedded_sass.proto
bundle exec rake |
|
@jathak The remaining failures should be fixed once I have an embedded host PR up and running. |
|
@ntkme The latest commit should fix your issue. |
|
Not very scientific benchmark result: benchmark coderequire_relative 'lib/sass-embedded'
require 'benchmark'
puts Sass.info
n = ARGV.shift.to_i
input = ARGV.shift
Benchmark.bm do |x|
x.report('1 thread ') do
n.times do
Sass.compile(input).css
end
end
(2..9).each do |t|
x.report("#{t} threads") do
tl = []
t.times do
tl << Thread.new do
(n / t).times do
Sass.compile(input).css
end
end
end
tl.each(&:join)
end
end
x.report('1 compiler ') do
n.times do
Sass.compile(input).css
end
end
(2..9).each do |t|
compilers = Array.new(t) do
Sass::Embedded.new
end
x.report("#{t} compilers") do
tl = []
t.times do |i|
tl << Thread.new do
(n / t).times do
compilers[i].compile(input).css
end
end
end
tl.each(&:join)
end
compilers.each(&:close)
end
endtiny input4kb inputlarge input |
|
Another benchmark with high round trips benchmark coderequire_relative 'lib/sass-embedded'
require 'benchmark'
puts Sass.info
n = ARGV.shift.to_i
rt = ARGV.shift
input = '
@for $i from 1 through ' + rt + ' {
ul:nth-child(#{$i}) {
content: foo($i);
}
}
'
functions = {
'foo($i)': lambda do |*|
Sass::Value::String.new('foo')
end
}
Benchmark.bm do |x|
x.report('1 thread ') do
n.times do
Sass.compile_string(input, functions: functions).css
end
end
(2..9).each do |t|
x.report("#{t} threads") do
tl = []
t.times do
tl << Thread.new do
(n / t).times do
Sass.compile_string(input, functions: functions).css
end
end
end
tl.each(&:join)
end
end
x.report('1 compiler ') do
n.times do
Sass.compile_string(input, functions: functions).css
end
end
(2..9).each do |t|
compilers = Array.new(t) do
Sass::Embedded.new
end
x.report("#{t} compilers") do
tl = []
t.times do |i|
tl << Thread.new do
(n / t).times do
compilers[i].compile_string(input, functions: functions).css
end
end
end
tl.each(&:join)
end
compilers.each(&:close)
end
end10 function call requests per compilation |
|
Am I interpreting those results correctly to indicate that this is almost universally slower than the previous version, even when making heavy use of parallelism? |
|
The x-threads test means using a single persisted compiler that each runs x-threads (if supported). We are always slower when using 1 persisted compiler with 1 thread in new version comparing to old version. - This is expected due to isolate overhead. |
|
Oh I see, I missed the "X threads" readout. In that case, given that the additional single-threaded overhead is relatively low, I think it's acceptable. There are some further possibilities for improvement here. In dart-lang/sdk#52577 I'm asking the Dart team for better ways to limit copies when sending messages across isolates. We could also look into making the compiler able to switch between parallel and serial modes—either with an explicit switch from the caller, or by detecting when the caller attempts to run multiple compilations at once and switching modes after the active compilation finishes. |
Or we can introduce a command line flag like |
|
That's definitely another option, although all things being equal I think it'd probably be better to avoid punting the decision about when/whether to be parallel to the host. After all, even if the host can submit parallel requests, it's not always going to be called in a way that causes them. |
|
I just change the way threading works on the ruby host side. Performance of 1.63.0-dev has greatly improved in all cases comparing to the previous result of 1.63.0-dev. Specifically, now multi-threading is consistently faster than multi-processes for lots of tiny input/output, and getting closer to multi-processes for large input/output. |
|
What's interesting is that offloading the protobuf decoding from the dispatcher I/O thread on the ruby host side improved the single compiler single thread performance so much for medium and large size input/output, for which it makes up for the isolate overhead and becomes faster overall. For tiny input/output test with single compiler single thread, it was slower but only 0.02ms per compilation on average. So from ruby host perspective, this is a win. It would get even better with future improvement on reducing dart isolate communication overhead. |
|
Sorry never mind with my previous comment, my local branch state was not synced correctly. The compiler does appear to shutdown correctly. |
* main: Run embedded compilations across multiple isolates (sass#1981)
Closes #1980
Closes #1959
See sass/sass#3599
See sass/embedded-host-node#224