tools icon indicating copy to clipboard operation
tools copied to clipboard

🐛 46x performance regression in LSP server

Open strager opened this issue 2 years ago • 3 comments

Environment information

CLI:
  Version:              0.0.0
  Color support:        true

Platform:
  CPU Architecture:     x86_64
  OS:                   linux

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 "xterm-256color"

Rome Configuration:
  Status:               loaded
  Formatter disabled:   false
  Linter disabled:      false

Workspace:
  Open Documents:       0

Discovering running Rome servers...

Running Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

ℹ The client isn't connected to any server but rage discovered this running Rome server.

Server:
  Version:              0.0.0
  Name:                 rome_lsp
  CPU Architecture:     x86_64
  OS:                   linux

Workspace:
  Open Documents:       0

Other Active Server Workspaces:

Workspace:
  Open Documents:       1

Rome Server Log:

⚠ Please review the content of the log file before sharing it publicly as it may contain sensitive information:
  * Path names that may reveal your name, a project name, or the name of your employer.
  * Source code

[logs are too long for GitHub]

Incompatible Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

ℹ Rage discovered this running server using an incompatible version of Rome.

Server:
  Version:              <=10.0.0

Incompatible Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

ℹ Rage discovered this running server using an incompatible version of Rome.

Server:
  Version:              12.1.3

What happened?

I have a suite of JS LSP server benchmarks (https://quick-lint-js.com/benchmarks/). About a year ago, Rome performed well. Recent versions of Rome perform about 46x worse! Rome is 5x as slow as ESLint in my benchmarks.

I bisected the Rome repo. There appear to be at least three big regressions:

  • 9x: somewhere between commits 7d15325c2ca82bcc4b7b2c8efcd29ebfdd7bd696 (~2.22 ms) and 6345f9703e9eab80b42e77d9037ecbe019770098 (~19.82 ms)
  • 4x: commit 5809d01193db33e3d21c88bcfdb0f252da00e5ec (~19.66 ms -> ~84.35 ms)
  • 30%: somewhere between commits c3fb1a8145417b4e9af7e97c6c1e7d08aa3708f5 (~79.90 ms) and d31c9cf0ae4f825c3e4bbdc0f72e361fe33a474a (~103.85 ms) (likely multiple smaller regressions)

For newer versions, my benchmarks stop the Rome server between samples. Each sample includes 1 warmup iteration and 10 timed iterations.

The benchmark basically lints this file 10 times: https://github.com/quick-lint/quick-lint-js/blob/1433b434f37c4dae212c35307a55849cfb633586/benchmark/benchmark-lsp/corpus/express-router.js

Benchmark framework: https://github.com/quick-lint/quick-lint-js/tree/1433b434f37c4dae212c35307a55849cfb633586/benchmark/benchmark-lsp

Here is performance data at various commits on my 5950X on Linux for quick-lint-js-benchmark-lsp-servers Rome/full-change-wait/express-router.js --iterations 10 --samples 3:

Rome 9be13cac83320929a949201ca462015d5a446dbc -- published benchmarks 2.36 ms per iteration 2.24 ms per iteration 2.32 ms per iteration

Rome 8b80b134475fd61564c62efed66ea44dbe9e637f 2.19 ms per iteration 2.28 ms per iteration 2.18 ms per iteration

Rome 7d15325c2ca82bcc4b7b2c8efcd29ebfdd7bd696 2.22 ms per iteration 2.27 ms per iteration 2.24 ms per iteration

Rome c2927dcc5b3bd497cffae96df3e7db1f44a99d5e started using socket LSP; hard to test

Rome b567557c69 hard to test

Rome 6345f9703e9eab80b42e77d9037ecbe019770098 added lsp-proxy command 19.82 ms per iteration 26.40 ms per iteration 23.58 ms per iteration

Rome 145c38bad92a946627db8ea9aec5a2a804323692 20.62 ms per iteration 19.35 ms per iteration 21.32 ms per iteration

Rome 519df0b61ff40bc655d6a7a8ec7db9529a915581 19.47 ms per iteration 18.72 ms per iteration 18.94 ms per iteration

Rome 452aedefdce8983d90dedb70c1947f70591b4120 19.30 ms per iteration 19.23 ms per iteration 19.97 ms per iteration

Rome 5248e2b5d5f37fd3319566241be51ec01ad568da 21.68 ms per iteration 22.64 ms per iteration 19.66 ms per iteration

Rome 5809d01193db33e3d21c88bcfdb0f252da00e5ec big performance regression 84.35 ms per iteration 85.85 ms per iteration 87.99 ms per iteration

Rome 58a8e381b7fb7955174f9c79a46d81a85f94977b 88.39 ms per iteration 89.44 ms per iteration 89.41 ms per iteration

Rome 8b6b6d01d632039d912b1545ebbaa98a9becf451 87.18 ms per iteration 89.08 ms per iteration 87.97 ms per iteration

Rome a090d6023ced996dce0636ac3031d2a13777a350 84.87 ms per iteration 82.63 ms per iteration 80.02 ms per iteration

Rome c3fb1a8145417b4e9af7e97c6c1e7d08aa3708f5 79.90 ms per iteration 82.71 ms per iteration 82.43 ms per iteration

Rome 6da14752379a60c229468ffca5bdc048174cb404 113.45 ms per iteration 116.18 ms per iteration 119.82 ms per iteration

Rome 6bd0a69f68fd81deb51ad59d875a3c866d92aa4c 99.98 ms per iteration 102.63 ms per iteration 97.38 ms per iteration

Rome b77547d5b2fac4f06bc4a9b5e10eaab79dac77c1 97.02 ms per iteration 101.20 ms per iteration 98.93 ms per iteration

Rome d31c9cf0ae4f825c3e4bbdc0f72e361fe33a474a 105.01 ms per iteration 105.91 ms per iteration 103.85 ms per iteration

Rome 817d5ac1eae7f5b5a9972105ab8ac2b9e5f08f01 -- latest commit tested 104.27 ms per iteration 108.58 ms per iteration 109.32 ms per iteration

Expected result

Rome's LSP server is fast :rocket:

Code of Conduct

  • [X] I agree to follow Rome's Code of Conduct

strager avatar Aug 22 '23 05:08 strager

The regression impacts only the LSP?

Conaclos avatar Aug 22 '23 22:08 Conaclos

The regression impacts only the LSP?

I don't know. I only measured LSP.

strager avatar Aug 22 '23 22:08 strager

flamegraph1 I believe I've found the problem: https://github.com/rome/tools/blob/964a039f31b5b6bbd5e301e5f18682104866fbff/crates/rome_rowan/src/ast/batch.rs#L306-L332

TextEdit::from_unicode_words(&old, &new); is the bottleneck. We can try reducing the range of the string for calculating the text diff, because currently, we use the entire strings both before and after mutation.

denbezrukov avatar Aug 28 '23 19:08 denbezrukov