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.

Sign up for GitHub

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

Jump to bottom

Replace PyAccu with PyUnicodeWriter #95005

Closed
aivarsk opened this issue Jul 19, 2022 · 4 comments
Closed

Replace PyAccu with PyUnicodeWriter #95005

aivarsk opened this issue Jul 19, 2022 · 4 comments
Labels
type-feature A feature request or enhancement

Comments

@aivarsk
Copy link
Contributor

aivarsk commented Jul 19, 2022 •
edited
Loading

PyAccu is used only in two places:

  • JSON encoder (PyUnicodeWriter is used already for the decoder)
  • StringIO (very minimal use of PyAccu)

The newer PyUnicodeWriter seems to be a better tool for aggregating strings and removing PyAccu gets rid of >100 lines of C code while keeping the same functionality.

P.S. The new JSON encoder also performed ~10% better on a completely synthetic test encoding a 20Mb JSON. That JSON in turn was created while investigating the lack of concurrent execution in FastAPI services. Which turned out to be due to JSON encoding.
@aivarsk aivarsk added the type-feature A feature request or enhancement label Jul 19, 2022
@bedevere-bot bedevere-bot mentioned this issue Jul 19, 2022
Merged
@mdboom
Copy link
Contributor

mdboom commented Jul 19, 2022 •
edited
Loading

pyperformance has some JSON-related benchmarks. It would be good to run those in our stable benchmarking environment to confirm the performance increase before merging. I'm happy to do that if you ping me when your PR is ready for that testing.

@aivarsk
Copy link
Contributor Author

aivarsk commented Jul 19, 2022

This is pyperformance before my changes:

### json_dumps ###
Mean +- std dev: 11.0 ms +- 0.1 ms

And after my changes:

### json_dumps ###
Mean +- std dev: 10.3 ms +- 0.1 ms

There is an improvement but not as big as in my 20Mb test case.

@aivarsk
Copy link
Contributor Author

aivarsk commented Jul 19, 2022

@mdboom the PR is ready, can you run the tests?

@mdboom
Copy link
Contributor

mdboom commented Jul 19, 2022

The results on the standard test machine corroborate your results.

Before your change:

### json_dumps ###
Mean +- std dev: 12.2 ms +- 0.1 ms

After your change:

### json_dumps ###
Mean +- std dev: 11.3 ms +- 0.1 ms

So, a solid improvement (and of course there are other reasons to do this besides just performance...)

corona10 pushed a commit that referenced this issue Jul 27, 2022
@aivarsk
gh-95005: Replace PyAccu with PyUnicodeWriter (gh-95006)
8c88e36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mdboom @aivarsk @corona10