search

python-lsp / python-lsp-server

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

Remove ujson dependency and use python's json module #508

Closed rumpelsepp closed 5 months ago

rumpelsepp commented 6 months ago

The ujson dependency ships a C-extension via a wheel which could cause unnecessary problems due to different versions of glibc in different environments, especially on NixOS:

import ujson as json
ImportError: /nix/store/9v5d40jyvmwgnq1nj8f19ji2rcc5dksd-glibc-2.37-45/lib/libc.so.6: version `GLIBC_2.38' not found (required by /nix/store/myw67gkgayf3s2mniij7zwd79lxy8v0k-gcc-12.3.0-lib/lib/libstdc++.so.6)

I propose removing this dependency, since I claim that the performance gain it did more than four years ago (added in 1c0c54079b093728ca6d3acb99d1c230708619e6) is not that much relevant any more. Here is a benchmark of several json implementations: https://github.com/TkTech/json_benchmark. The builtin json module is even faster than ujson in 50% of their benchmarks.

I claim, replacing ujson with the standard json module does not make any difference in practice; but it avoids compatability issues.

ccordoba12 commented 5 months ago

I'm sorry but I'm going to say no to this due to the following:

rumpelsepp commented 5 months ago

Thanks for your answer. For the sake of correctness:

We're not interested in supporting really old Linux distros, you are

This is wrong. I did not propose supporting really old Linux distros, but very recent ones by not relying on libstdc++. Anyway, switching to orjson should also fix those problems as it relies on Rust instead. From my experience this causes fewer compatability issues with libc than C++ dependencies.

ccordoba12 commented 5 months ago

Ok, sorry for the misunderstanding. I didn't know ujson is based on C++, which is causing the problem you reported.

Could you help us then to switch to orjson instead? I'll be happy to merge that PR.