Skip to content
This repository was archived by the owner on May 15, 2023. It is now read-only.

Replace waitFor with async/await #136

Closed
wants to merge 2 commits into from
Closed

Replace waitFor with async/await #136

wants to merge 2 commits into from

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented Dec 24, 2022

This PR replaces waitFor with async/await.

There's some trade off in terms of performance, see benchmark below. Personally I think this trade off is worth it as in most of the cases the performance is getting better.

Update: Added a commit to fix regression. Performance is now either nearly equal or way better in every case. The code logic isn't really changed from the first attempt, but dart's compiler seems to have trouble optimize the old one correctly due to how type inference works.

Update 2: still regress a lot in another case, see comment below.

Baseline 1.57.1 waitFor:

       user     system      total        real
 1c start up cost  0.000184   0.000801   0.000985 (  0.022070)
1t/100c bootstrap  0.021838   0.021640   0.043478 ( 15.298093)
1t/100000c w/  func w/  url  4.891331   1.683478   6.574809 ( 21.760007)
4t/100000c w/  func w/  url  4.863213   1.674371   6.537584 ( 21.683645)
1t/100000c w/  func w/o url  4.709930   1.602782   6.312712 ( 19.915833)
4t/100000c w/  func w/o url  4.722000   1.590509   6.312509 ( 19.854262)
1t/100000c w/o func w/  url  2.131774   0.488197   2.619971 ( 12.001358)
4t/100000c w/o func w/  url  2.124635   0.487802   2.612437 ( 11.982604)
1t/100000c w/o func w/o url  2.073813   0.497656   2.571469 ( 10.536980)
4t/100000c w/o func w/o url  2.082869   0.506721   2.589590 ( 10.585789)
ruby test.rb  104.23s user 39.80s system 100% cpu 2:23.72 total

This PR 1.57.1 async/await:

       user     system      total        real
 1c start up cost  0.000259   0.000981   0.001240 (  0.024843)           # (smaller is better)
1t/100c bootstrap  0.021841   0.021375   0.043216 ( 15.391264)           # 1.006x 
1t/100000c w/  func w/  url  4.081550   1.060135   5.141685 ( 18.881023) # 0.868x
4t/100000c w/  func w/  url  4.082645   1.058004   5.140649 ( 18.870283) # 0.870x
1t/100000c w/  func w/o url  4.037197   1.063087   5.100284 ( 17.276619) # 0.867x
4t/100000c w/  func w/o url  4.040086   1.064021   5.104107 ( 17.267876) # 0.870x
1t/100000c w/o func w/  url  2.149704   0.511046   2.660750 ( 12.137423) # 1.011x
4t/100000c w/o func w/  url  2.144795   0.512078   2.656873 ( 12.132177) # 1.012x
1t/100000c w/o func w/o url  2.098397   0.505022   2.603419 ( 10.608703) # 1.006x
4t/100000c w/o func w/o url  2.088623   0.505967   2.594590 ( 10.606887) # 1.002x
ruby test.rb  99.62s user 27.62s system 95% cpu 2:13.28 total            # 0.927x

Both tests are compiled locally with same dart version:

pkg-compile-native
  dart compile aot-snapshot bin/dart_sass_embedded.dart -Dversion=1.57.1 -Ddart-version=2.18.6 -Dprotocol-version=1.2.0 -Dcompiler-version=1.57.1 -Dimplementation-version=1.57.1 --output build/dart-sass-embedded.native

Benchmark code:

require_relative './lib/sass-embedded'
require 'benchmark'

c = 100000

Benchmark.bm do |x|
  x.report ' 1c start up cost' do
    Sass.info
  end
  x.report '1t/100c bootstrap' do
    100.times do
      Sass.compile('./node_modules/bootstrap/scss/bootstrap.scss').css
    end
  end
  [true, false].each do |f|
    [true, false].each do |u|
      [1, 4].each do |t|
        x.report "#{t}t/#{c}c #{f ? 'w/ ' : 'w/o'} func #{u ? 'w/ ' : 'w/o'} url" do
          input = f ? 'a {b: foo()}' : 'a {b: c}'
          args = ({})
            .merge!(f ? {functions: {
              'foo()': ->(_) { Sass::Value::String.new('bar') }
            }} : {})
            .merge!(u ? {url: 'file:///tmp/test.scss'} : {})

          t.times do
            threads = []
            threads << Thread.new do
              (c / t).times do
                Sass.compile_string(input, **args).css
              end
            end
            threads.each(&:join)
          end
        end
      end
    end
  end
end

@ntkme
Copy link
Contributor Author

ntkme commented Dec 25, 2022

This actually turned out to regress if a function is passed but never used on a large user input due to most part of the compiler is running in event loop because dart simply doesn't optimize synchronously returned Value for FutureOr<Value> and still put it on event loop: dart-lang/language#2033

@ntkme
Copy link
Contributor Author

ntkme commented Dec 26, 2022

Closing for now.

@ntkme ntkme closed this Dec 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrent requests block each other and Stack Overflow under high concurrency
1 participant