python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.95k stars 194 forks source link

Replace ujson with orjson #591

Open valrus opened 3 months ago

valrus commented 3 months ago

This PR is a small update to @rumpelsepp's here: https://github.com/python-lsp/python-lsp-server/pull/579 It just points the python-lsp-jsonrpc dependency at the branch I have PR'd at https://github.com/python-lsp/python-lsp-jsonrpc/pull/29, to demonstrate the tests pass.

ccordoba12 commented 3 months ago

It seems everything is here too, thanks @valrus and @rumpelsepp!

I'll release 1.12.0 first and then 2.0.0 a couple of weeks after that with this improvement.

simon-liebehenschel commented 2 months ago

@valrus @ccordoba12 Are there any existing tests to benchmark the python-lsp-server memory usage in the long run? The orjson library has some known "features" related to the memory usage and I really do not want to get OOM (Out of memory) errors or high memory usage after this pull request. I do not mean that this is guaranteed to get for python-lsp-server, but better to check to be sure.

valrus commented 2 months ago

@simon-liebehenschel I'm not aware of any tests covering memory usage, but I might take a stab at writing one. Can you provide any pointers to the issue(s) you're concerned about?

simon-liebehenschel commented 2 months ago

@simon-liebehenschel I'm not aware of any tests covering memory usage, but I might take a stab at writing one. Can you provide any pointers to the issue(s) you're concerned about?

https://github.com/ijl/orjson/issues/504

But I do not know if it will have any impact on python-lsp-server because depends on a specific usage. If you think this PR is safe, let's try it and see if anyone notices any regression in memory usage.