Verify ruby-lsp is working during run

[noahgibbs]

I'm seeing results from @eightbitraptor that look a lot like the ruby-lsp benchmark is failing, rapidly, and just not noticing, so it returns really fast, really unstable results.

So I figure I'll add some verification, to ensure that it's generating the right results, for some definition of that phrase. Probably "save the result from the last iteration, do a few basic checks on the value" sort of thing.

[maximecb]

Good idea to add these checks 👍

[noahgibbs]

Added a check to make sure it was working. On GHActions, like on Matt's machine, it was not.

Went down a rabbit hole with @vinistock to figure out what was going wrong. Looks like Rubocop is sometimes getting a LoadError for racc/parser which (for complicated reasons) ruby-lsp normally swallows quietly as "no rubocop" and goes on about its day.

The LoadError is from the parser gem (version 3.1.2.1) which is declaring racc as a development dependency but not a runtime dependency. So my best current guess is that some versions of Bundler treat development dependencies as okay (but racc isn't in Gemfile.lock -- why would that work?) but other versions of Bundler presumably don't, and correctly give a LoadError when you try to use an undeclared gem.

So: good news is that it's really easy to detect this problem, and the simple first version of the code works great to detect this problem. Check size of responses, they should be 34 and 1160. (see: #240)

The bad news is that right now this fails for complicated reasons on GitHub CI, and it's no good adding the test to detect the failure until we fix it.

More recent versions of parser appear to correctly declare racc as a runtime dependency. So now we just need to find a version of the parser gem that declares the dependency properly and isn't too hard to update to.

[maximecb]

So now we just need to find a version of the parser gem that declares the dependency properly and isn't too hard to update to.

GitHub automatically closed the issue when I merged the PR. You've added the validation, but you still need to find the right version of the parser gem?

[noahgibbs]

I've updated the parser gem in the PR.

[eregon]

One thought, do we actually want to run RuboCop in the ruby-lsp benchmark here? Because that's not really benchmarking ruby-lsp itself so much then. Maybe even a majority of the time is spent in RuboCop and not ruby-lsp (just a guess, don't have time to check right now)?

[noahgibbs]

I think this was a compromise from @vinistock, who (IIRC) wrote the benchmark. We have a number of benchmarks where we intentionally add several different activities together this way.

But Vini might have a stronger opinion on whether running Rubocop this way is typical and representative or not.

[vinistock]

The reason I included it is because running RuboCop related features, such as diagnostics, is by far the slowest operation the Ruby LSP performs - and YJIT helps a lot there.

But I did raise the point back when I added the benchmark if it would be better to split it into separate things and we ended up going with everything in the same benchmark.

I'm okay with splitting if people think it makes more sense. We can keep the ruby-lsp one for our regular operations and add a ruby-lsp-rubocop exclusively for RuboCop related stuff.

Once we integrate YARP into the Ruby LSP, the performance will change significantly. We can take that opportunity to both upgrade the Ruby LSP and then I'll split the RuboCop part into another benchmark. Does that sound good?

[noahgibbs]

Sounds good to me. Initially we were trying not to split benchmarks out of worry that we'd overrun our benchmarking time on our worker. That's not as big a deal at this point.

[maximecb]

Once we integrate YARP into the Ruby LSP, the performance will change significantly. We can take that opportunity to both upgrade the Ruby LSP and then I'll split the RuboCop part into another benchmark. Does that sound good?

Yes, let's revisit it then 👍