Skip to content

Commit 5419713

Browse files
jakobnissenvtjnash
andauthored
Refactor IOBuffer code (#57570)
I've been a little frustrated with the IOBuffer code. It contains a whole bunch of implicit invariants, and is poorly commented. It also has several bugs that ultimately stems from the code being unclear about its own assumptions. This is a refactoring of IOBuffer. The primary goals are: * Comment the code more heavily * Test the code more thoroughly The secondary goals are * Fix a few outstanding bugs * Add some minor performance improvements This is a purely internal refactoring with be no change in behaviour of `IOBuffer`, except straight up bugfixes. However, note that previous code may have relied on buggy behaviour. Fixing bugs may therefore cause breakage. ## Current changes ### **BEHAVIOUR CHANGES** * The following code used to not throw an error, but now does: `IOBuffer(b"abc"; maxsize=2)`. I consider this a bugfix. It should not be possible to construct an IOBuffer with a buffersize larger than `maxsize`. * It used to be possible to write to indices higher than `maxindex`, which could trigger a bug causing data loss. The bug has been fixed, but as a result, some IOBuffers may reach full capacity faster (really: reach it at the correct point), changing writing behaviour. ### Bugfixes * Do not corrupt data on `copyline` on a non-appending buffer * Respect `maxsize` even after `take!` (fix #57549) * Fix bug when copying from an appending iobuffer to itself * Fix bug where re-allocating the buffer may cause it to shrink, discarding data. * Fix bug where `truncate` may throw a wrong BoundsError * Fix a bug where truncating a buffer may not correctly removed mark at position that has been deleted * Fix a bug where initializing an IOBuffer without an explicit buffer and with `truncate=false` makes it contain the full buffer * Current behaviour for `reset` and `position` did not work for `PipeBuffer`. Fix that ### Changes to brittle code * Removed some tests that explicitly tested internal code and internal behaviour. Some of that behaviour has changed. * Changed some internal `PipeBuffer` behaviour, which did not respect writable IOBuffer's ownership of their buffer and therefore failed spuriously ### Performance improvements * Writing to dense IOBuffer now uses memmove and is up to 10x faster for long writes. * Minor optimisations (about ten percent) for writing to IOBuffers in general Closes #57549 Co-authored-by: Jameson Nash <[email protected]>
1 parent 2c7527b commit 5419713

File tree

3 files changed

+925
-308
lines changed

3 files changed

+925
-308
lines changed

0 commit comments

Comments
 (0)