Skip to content

Use orjson to improve JSON marshalling performance - #691

Merged
amotl merged 2 commits into
mainfrom
orjson
Jan 30, 2025
Merged

amotl merged 2 commits into
mainfrom
orjson

Conversation

@amotl amotl changed the title Start using orjson to improve JSON marshalling performance Use orjson to improve JSON marshalling performance Jan 15, 2025
@amotl
amotl requested review from kneth and seut January 15, 2025 23:08
@amotl
amotl force-pushed the orjson branch 4 times, most recently from 2a43b26 to c92e27b Compare January 15, 2025 23:19
@amotl
amotl marked this pull request as ready for review January 15, 2025 23:27
Comment thread tests/client/test_http.py Outdated
Comment thread tests/client/test_http.py Outdated

@amotl amotl left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again. Thanks for the quick review. I've just added a few thoughts about the consequences of removing the traditional CrateJsonEncoder.

Comment thread src/crate/client/http.py Outdated
Comment thread src/crate/client/http.py Outdated
Comment thread src/crate/client/http.py Outdated
@amotl
amotl force-pushed the orjson branch 3 times, most recently from 5b8b18c to 402cdde Compare January 16, 2025 12:51
Comment thread tests/client/test_http.py Outdated
@amotl
amotl requested a review from seut January 16, 2025 12:57

@kneth kneth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this performance fix. Thanks to @amotl and @seut!

@seut seut left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

Comment thread src/crate/client/http.py Outdated
@surister

Copy link
Copy Markdown
Contributor

Do we have a benchmark?

Comment thread src/crate/client/http.py Outdated
@amotl

amotl commented Jan 16, 2025

Copy link
Copy Markdown
Contributor Author

Do we have a benchmark?

It's not a benchmark, but @widmogrod shared a flamegraph before/after over at #689 (comment).

Comment thread src/crate/client/http.py
Comment thread CHANGES.rst
Comment thread src/crate/client/http.py Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants