-
Notifications
You must be signed in to change notification settings - Fork 79
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
relax bind! Vector{UInt8} -> AbstractVector{UInt8} #340
Conversation
This allows zero-copy writes e.g. for ReinterpretArrays.
Sorry for the slow review on this; I'd like to merge, but can you look at the failing test and see if we should update the test or if we need something else to maintain desired behavior? |
I'd be afraid that this would lose a requirement that the Vector be contiguous in memory (which is I believe true for the underlying SQLite C code). You might need to add a new method, that specifically takes |
Sooo: I'm trying to understand what happens in the test. Here Line 632 in 95131f8
Base.CodeUnits{UInt8, String} to a newly created table in a BLOB column. Before this change, the bytearray was written as a serialized julia object, including type information. So after reading it back in, its type is completely restored, but at the cost of storing it in a julia-only readable format (not as plain bytearray). After this patch, the bytes corresponding to the string "bytearray" are written. After reading them back in, it's just a plain vector. In consequence, the test would have to be changed.
Ok if that is true, we have to take it seriously. So far it has worked for me. Can you point to a resource for more information? |
Ok, I just implemented a non-contiguous I'll come up with sth along the lines you suggested. |
Can you trigger tests again? |
Looks like this is getting closer. There's a failure on the nightly CI though w/ trying to get a pointer to ReinterpretArray. I'm not sure if there's a valid way to do that after the Memory changes in Base julia? It might be worth asking around (public julia slack/zulip/discord) or snooping around the ReinterpretArray code in base to see what's possible. |
I now pass a pointer to the ccall directly, so it doesn't rely on the broken conversion. That seems to work with old and new versions. |
...aaand replaced pointer by Ref, because pointer is not safe from GC. |
@quinnj ping :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @maxfreu!
This allows zero-copy writes e.g. for ReinterpretArrays.
Do you want this to be tested explicitly?