GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Replace ujson with orjson by valrus · Pull Request #591 · python-lsp/python-lsp-server

Conversation

@valrus

@valrus valrus commented Aug 17, 2024

Copy link
Copy Markdown

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rumpelsepp reacted with heart emoji
All reactions
  • ❤️ 1 reaction
rumpelsepp and others added 2 commits August 11, 2024 10:18
@rumpelsepp @valrus
Drop ujson and switch to orjson
f3c11af
@valrus
Update jsonrpc dependency
6e1d5c6
@valrus valrus mentioned this pull request Aug 17, 2024
Replace ujson with orjson python-lsp/python-lsp-jsonrpc#29
Open
@valrus
need to use https for dep, of course
7084ed8
@ccordoba12 ccordoba12 added this to the v2.0.0 milestone Aug 18, 2024
@ccordoba12 ccordoba12 mentioned this pull request Aug 18, 2024
Drop ujson and switch to orjson #579
Closed
@ccordoba12

ccordoba12 commented Aug 18, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.
valrus and rumpelsepp reacted with hooray emoji
All reactions
  • 🎉 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ccordoba12 ccordoba12 added the enhancement New feature or request label Aug 18, 2024
@simon-liebehenschel

simon-liebehenschel commented Sep 2, 2024

Copy link
Copy Markdown

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@valrus

valrus commented Sep 3, 2024

Copy link
Copy Markdown
Author

@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?
ccordoba12 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@simon-liebehenschel

simon-liebehenschel commented Sep 3, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@valrus @ccordoba12 @simon-liebehenschel @rumpelsepp
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.