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

Allow TokenReader to allocate and introduce reset function #30

Closed
ianprime0509 opened this issue Nov 23, 2023 · 2 comments
Closed

Allow TokenReader to allocate and introduce reset function #30

ianprime0509 opened this issue Nov 23, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@ianprime0509
Copy link
Owner

Right now, TokenReader will only use its internal buffer to store one full token, and after the next call to next, that token is invalidated. This means that clients (such as Reader) have to copy around a lot of data, potentially unnecessarily.

If TokenReader used a growable buffer, it would be possible for it to offload the responsibility of deciding when to reset to the client (e.g. Reader might reset after handling a single event). However, there must still be a way to ensure that normal XML documents do not use unnecessarily large amounts of memory: specifically, there should be a configurable limit on the size of content tokens returned by the reader, so that the client has a chance to reset if desired in the middle of content.
@ianprime0509 ianprime0509 added the enhancement New feature or request label Nov 23, 2023
@ianprime0509 ianprime0509 mentioned this issue Nov 23, 2023
Closed
@ianprime0509
Copy link
Owner Author

One outcome which should naturally follow from this is that today's TokenReader should be easily reproducible from this proposed new TokenReader just by providing a FixedBufferAllocator.

ianprime0509 added a commit that referenced this issue Nov 24, 2023
@ianprime0509
feat: allow TokenReader to allocate
0b6b980 
Closes #30

This currently provides no benefit in itself based on my benchmarks, but
opens the door to further performance opportunities for `Reader`.

Since `TokenReader`'s buffer is now allowed to grow, it is no longer
necessary for the decoder interface to handle overflowing the encoding
buffer. As such, the parameter type of the buffer has been changed to
reflect the exact necessary size, and `error.Overflow` is no longer
returned.

BREAKING CHANGE: `TokenReader` now requires an allocator, and
`error.Overflow` is no longer returned from any reader function.
@ianprime0509 ianprime0509 mentioned this issue Nov 24, 2023
Closed
@ianprime0509
Copy link
Owner Author

No longer relevant with #36.

@ianprime0509 ianprime0509 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ianprime0509