kcl-lang / kcl

KCL Programming Language (CNCF Sandbox Project). https://kcl-lang.io
https://kcl-lang.io
Apache License 2.0
1.58k stars 113 forks source link

Improve LSP performance by skip compiling version behind current version #1515

Open logo749 opened 1 month ago

logo749 commented 1 month ago

Enhancement

When file changed, the LSP compile the code in multi thread. But all thread seem to run sequentially because of the acquiring of the db write lock: image

I try to move the lock close to the insertion of the db, but it won't work, because it seem that we change the gs in the compile_with_params.

I propose a tiny optimization which is checking the version after we get snapshot.db write lock. In situation when user enter continuously in a big kcl file, this can skip many useless compiling of version behind current version.

I have some question:

  1. Is my idea above correct?
  2. Should we add debouncing when file changed?
  3. Any plan to make the compile compile_with_params parallelizable by removing the db lock?
Peefy commented 1 month ago

cc @He1pa

He1pa commented 1 month ago

debouncing is needed, and the version check is also designed for this. But I dont think you optimization will work(if you mean check version before compile). The main problem is that the compilation process now takes quite a long time. The process is as follows: image

DB lock is indeed a tricky problem. We have made many changes before, but have not achieved very good results. So, if you have good suggestions and ideas, please continue to discuss.

He1pa commented 1 month ago

I have also tried moving the location of db.lock, but it seems that there will be thread safety restrictions. I think it is correct to acquire the lock when need writing(after compiling), but I don't know how to implement it :)

logo749 commented 1 month ago

Yes, when there two event, the result is the same. But when there is 3 or more event, only the first and the lastest event will be compile. And I want to add debounce to optimize the first event, so when a user press key continuely, only the lastest will handled. image

logo749 commented 1 month ago

I have also tried moving the location of db.lock, but it seems that there will be thread safety restrictions. I think it is correct to acquire the lock when need writing(after compiling), but I don't know how to implement it :)

Yes, I think this is very complicated and need deep understanding of the lsp code base...

He1pa commented 1 month ago

Yes, when there two event, the result is the same. But when there is 3 or more event, only the first and the lastest event will be compile. And I want to add debounce to optimize the first event, so when a user press key continuely, only the lastest will handled. image

Ok,can you provide pseudo code? If it is very simple, it is also OK to submit a PR directly. Thank you very much for your contribution

logo749 commented 1 month ago

Yes, when there two event, the result is the same. But when there is 3 or more event, only the first and the lastest event will be compile. And I want to add debounce to optimize the first event, so when a user press key continuely, only the lastest will handled. image

Ok,can you provide pseudo code? If it is very simple, it is also OK to submit a PR directly. Thank you very much for your contribution

Ok, I will submit a PR later :)

Peefy commented 3 weeks ago

Closed by #1550

He1pa commented 8 hours ago

closed by #1584