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.
This is very much WIP. Essentially the strategy is to have a small wrapper around the
Vector{UInt8}
that is the memory map that implements enough of theString
interface so that the parsing from that works.I also tried to do this with
WeakRefString
(by pinning the buffer withGC.@preserve
), but that consistently segfaulted during tests...I also think that this is actually not the ideal use-case for
WeakRefString
: the main reason for that package is that one can stack-allocate that type, but we don't need that at all. In fact, in this case here, nothing unsafe, or weak has to happen. So my sense is that using a different type for the string here, that is much safer to use, is a better option.There is another reason I think a custom string type makes sense here: I think one strategy for eventually implementing #24 is to just have another thin wrapper like the one here that does the same thing, but for a different encoding. So in my mind, these small string wrappers might be able to solve that issue eventually as well.
This PR does pass all tests right now, and when I run https://github.com/davidanthoff/csv-comparison, I get another small increase in performance (I think, I might have mixed up baselines, but I am certain that it is either neutral or an improvement).
Things still todo: