Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ujson with orjson #591

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

valrus
Copy link

@valrus valrus commented Aug 17, 2024

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

@ccordoba12 ccordoba12 added this to the v2.0.0 milestone Aug 18, 2024
@ccordoba12
Copy link
Member

ccordoba12 commented Aug 18, 2024

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.

@ccordoba12 ccordoba12 added the enhancement New feature or request label Aug 18, 2024
@simon-liebehenschel
Copy link

@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
Copy link
Author

valrus commented Sep 3, 2024

@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
Copy link

simon-liebehenschel commented Sep 3, 2024

@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?

ijl/orjson#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants