Fix stream overconsumption by undoing readahead via seek when possible #50
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
from
986aef5 to
f6f036a
Compare
smheidrich
changed the title
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
2 times, most recently
from
dfd74cc to
7396e2e
Compare
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
2 times, most recently
from
699faf8 to
3ce611a
Compare
smheidrich
added a commit
to smheidrich/json-stream
that referenced
this pull request
Dec 25, 2022
Proof of concept only. Maybe there is a more elegant solution that doesn't require putting `level` everywhere to find out when the top-level document ends. Requires smheidrich/py-json-stream-rs-tokenizer#50 to be merged to actually work.
This was referenced Dec 25, 2022
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
2 times, most recently
from
16a61fd to
3172869
Compare
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
3 times, most recently
from
173789e to
c22bff9
Compare
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
from
c22bff9 to
a5225c0
Compare
Pretty horrible fix that slows things down tremendously (speedup lowered from ~10 to ~2-3).
These issues remain: - seeking from the chunk start position instead of the very beginning of the stream doesn't work yet as there is nothing that resets the char counter when a new read() is performed - there must be some bug regarding re-initializing the BufReader after a readahead-undo (park_cursor() in the code) has been performed, because subsequent uses of this BufReader will yield wrong chars (visible e.g. in tests on random data like in the benchmarks but there should be an explicit test inside Rust instead) - only Python text streams are supported, none of this garbage will work for Python byte streams
Remaining issues: - get it to work for byte streams - clean up Rust warnings - remove unused code - look into degraded performance (speedup now only 8...), e.g. try to zero-copy strings from Python by putting them in refcells or sth. - handle case of non-seekable stream
Fixes performance degradation issue (lol...)
No switch to choose text/bytes yet => up next.
Also add unseekable text stream file I missed before
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
from
a5225c0 to
ea44f7e
Compare
Avoids collisions with parameters introduced by json-stream's Python tokenizer.
smheidrich
changed the title
smheidrich
force-pushed
the
fix-stream-overconsumption-seek-if-possible
branch
from
b0f2123 to
890fffa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #47 when done