Shopify / yjit-bench

Set of benchmarks for the YJIT CRuby JIT compiler and other Ruby implementations.
MIT License
87 stars 22 forks source link

The ruby-lsp benchmark fails on ruby master #271

Closed rwstauner closed 6 months ago

rwstauner commented 10 months ago

With a fresh ruby-dev I get this error:

$ ./run_benchmarks.rb ruby-lsp --once
Running benchmark "ruby-lsp" (1/1)
/Users/rwstauner/.rubies/ruby-dev/bin/ruby -I harness /Users/rwstauner/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb
ruby 3.4.0dev (2024-01-18T15:35:46Z master 00814fd672) [arm64-darwin23]
Command: bundle check 2> /dev/null || bundle install
The Gemfile's dependencies are satisfied
/Users/rwstauner/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb:24:in `block in <main>': undefined method `size' for nil (NoMethodError)

  rc_last = rc.size
              ^^^^^
        from /Users/rwstauner/src/github.com/Shopify/yjit-bench/harness/harness.rb:29:in `block in run_benchmark'
        from /Users/rwstauner/.rubies/ruby-dev/lib/ruby/3.4.0+0/benchmark.rb:313:in `realtime'
        from /Users/rwstauner/src/github.com/Shopify/yjit-bench/harness/harness.rb:29:in `run_benchmark'
        from /Users/rwstauner/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb:18:in `<main>'
Command "/Users/rwstauner/.rubies/ruby-dev/bin/ruby -I harness /Users/rwstauner/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb" failed in directory /Users/rwstauner/src/github.com/Shopify/yjit-bench
./run_benchmarks.rb:46:in `check_call': RuntimeError (RuntimeError)
        from ./run_benchmarks.rb:280:in `block in run_benchmarks'
        from ./run_benchmarks.rb:226:in `each'
        from ./run_benchmarks.rb:226:in `each_with_index'
        from ./run_benchmarks.rb:226:in `run_benchmarks'
        from ./run_benchmarks.rb:428:in `block in <main>'
        from ./run_benchmarks.rb:427:in `each'
        from ./run_benchmarks.rb:427:in `<main>'

We are currently using ruby-lsp 0.4.1. If I upgrade to the latest 0.13.4 and then fixup the code to be able to run, the results are incorrect

diff --git a/benchmarks/ruby-lsp/Gemfile.lock b/benchmarks/ruby-lsp/Gemfile.lock
index adcb143..34b4766 100644
--- a/benchmarks/ruby-lsp/Gemfile.lock
+++ b/benchmarks/ruby-lsp/Gemfile.lock
@@ -17,7 +17,7 @@ GEM
     parser (3.2.2.3)
       ast (~> 2.4.1)
       racc
-    prettier_print (1.2.0)
+    prism (0.19.0)
     racc (1.7.1)
     rack (3.0.4.2)
     rainbow (3.1.1)
@@ -42,14 +42,12 @@ GEM
       activesupport (>= 4.2.0)
       rack (>= 1.1)
       rubocop (>= 1.33.0, < 2.0)
-    ruby-lsp (0.4.1)
+    ruby-lsp (0.13.4)
       language_server-protocol (~> 3.17.0)
-      sorbet-runtime
-      syntax_tree (>= 6, < 7)
+      prism (>= 0.19.0, < 0.20)
+      sorbet-runtime (>= 0.5.10782)
     ruby-progressbar (1.11.0)
-    sorbet-runtime (0.5.10679)
-    syntax_tree (6.0.0)
-      prettier_print (>= 1.2.0)
+    sorbet-runtime (0.5.11205)
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.3.0)
diff --git a/benchmarks/ruby-lsp/benchmark.rb b/benchmarks/ruby-lsp/benchmark.rb
index fe13954..d7fc514 100644
--- a/benchmarks/ruby-lsp/benchmark.rb
+++ b/benchmarks/ruby-lsp/benchmark.rb
@@ -17,19 +17,16 @@
 # These benchmarks are representative of the three main operations executed by the Ruby LSP server
 run_benchmark(200) do
   # File parsing
-  document = RubyLsp::Document.new(content)
+  document = RubyLsp::RubyDocument.new(source: content, version: 1, uri: URI(file_uri))

   # Running RuboCop related requests
-  rc = RubyLsp::Requests::Diagnostics.new(file_uri, document).run
+  rc = RubyLsp::Requests::Diagnostics.new(document).perform
   rc_last = rc.size

   # Running SyntaxTree visitor requests
-  hl = RubyLsp::Requests::SemanticHighlighting.new(
-    document,
-    encoder: RubyLsp::Requests::Support::SemanticTokenEncoder.new,
-  ).run
-  hl_last = hl.data.size
+  hl = RubyLsp::Requests::SemanticHighlighting.new(Prism::Dispatcher.new).perform
+  hl_last = hl.size
 end

-raise("ruby-lsp benchmark: the RuboCop diagnostics test is returning the wrong answer!") if rc_last != 34
-raise("ruby-lsp benchmark: the Semantic Highlighting test is returning the wrong answer!") if hl_last != 1160
+raise("ruby-lsp benchmark: the RuboCop diagnostics test is returning the wrong answer: #{rc_last}") if rc_last != 34
+raise("ruby-lsp benchmark: the Semantic Highlighting test is returning the wrong answer: #{hl_last}") if hl_last != 1160

raises with

ruby-lsp benchmark: the RuboCop diagnostics test is returning the wrong answer: 0 (RuntimeError)
maximecb commented 10 months ago

I'm not able to reproduce this bug on macOS with a dev build of the commit you pointed to:

./run_benchmarks.rb ruby-lsp --once
Running benchmark "ruby-lsp" (1/1)
/Users/maximecb/.rubies/ruby-yjit/bin/ruby -I harness /Users/maximecb/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb
ruby 3.4.0dev (2024-01-18T15:35:46Z master 00814fd672) [arm64-darwin23]
Command: bundle check 2> /dev/null || bundle install
The Gemfile's dependencies are satisfied
itr #1: 1118ms
RSS: 111.0MiB
MAXRSS: 113888.0MiB
Running benchmark "ruby-lsp" (1/1)
/Users/maximecb/.rubies/ruby-yjit/bin/ruby --yjit -I harness /Users/maximecb/src/github.com/Shopify/yjit-bench/benchmarks/ruby-lsp/benchmark.rb
ruby 3.4.0dev (2024-01-18T15:35:46Z master 00814fd672) +YJIT dev [arm64-darwin23]
Command: bundle check 2> /dev/null || bundle install
The Gemfile's dependencies are satisfied
itr #1: 6070ms
RSS: 178.4MiB
MAXRSS: 182912.0MiB
Total time spent benchmarking: 8s

interp: ruby 3.4.0dev (2024-01-18T15:35:46Z master 00814fd672) [arm64-darwin23]
yjit: ruby 3.4.0dev (2024-01-18T15:35:46Z master 00814fd672) +YJIT dev [arm64-darwin23]

--------  -----------  ----------  ---------  ----------  ------------  -----------
bench     interp (ms)  stddev (%)  yjit (ms)  stddev (%)  yjit 1st itr  interp/yjit
ruby-lsp  1118.8       0.0         6070.4     0.0         0.18          0.18       
--------  -----------  ----------  ---------  ----------  ------------  -----------
Legend:
- yjit 1st itr: ratio of interp/yjit time for the first benchmarking iteration.
- interp/yjit: ratio of interp/yjit time. Higher is better for yjit. Above 1 represents a speedup.

Output:
./data/output_132.csv
rwstauner commented 10 months ago

The changes in https://github.com/Shopify/yjit-bench/pull/255 pass for me on that commit of ruby

maximecb commented 10 months ago

No idea why it failed on the first place, but let's continue the discussion on #255 🤷‍♀️