Skip to content

jsontext: Preserve the internal buffer between Reset calls #174

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

Closed

Conversation

fpetkovski
Copy link

The jsontext decoder throws away the internal buffer on each Reset call. This makes decoding one message at a time pretty wasteful since each new message will cause at least one allocation for the buffer.

This commit makes sure the buffer gets reused when the decoder is reset with a new reader. If unbounded growth for the slice is a concern, I can add an option limiting the maxiumum size after which the bytes buffer will be thrown away and a new one can be allocated.

The jsontext decoder throws away the internal buffer on each Reset call.
This makes decoding one message at a time pretty wasteful since each new message
will cause at least one allocation for the buffer.

This commit makes sure the buffer gets reused when the decoder is reset with a new reader.
If unbounded growth for the slice is a concern, I can add an option limiting the maxiumum size
after which the bytes buffer will be thrown away and a new one can be allocated.
@dsnet
Copy link
Collaborator

dsnet commented Jun 10, 2025

Hi, thanks for the PR, but this repository is now frozen as future development has moved to the Go standard library.

That said, I don't think this is quite the right fix as this could unintentionally alias the internal buffer of a bytes.Buffer.

@dsnet
Copy link
Collaborator

dsnet commented Jun 10, 2025

I could make the proper fix upstream for you, or did you want to take a shot at it?

@dsnet dsnet closed this Jun 10, 2025
@fpetkovski
Copy link
Author

Thanks for the pointer, let me raise an upstream PR 👍

@fpetkovski
Copy link
Author

Upstream PR here golang/go#74120

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.

2 participants