unsafe read/write #14766
unsafe read/write #14766
Conversation
| @@ -104,7 +104,7 @@ function sendfile(dst::File, src::File, src_offset::Int64, bytes::Int) | |||
| nothing | |||
| end | |||
|
|
|||
| function write(f::File, buf::Ptr{UInt8}, len::Integer, offset::Integer=-1) | |||
| function unsafe_write(f::File, buf::Ptr{UInt8}, len::UInt, offset::Int64=Int64(-1)) | |||
There was a problem hiding this comment.
does this need to be tightened from Integer? this might break packages that are hitting code paths not tested in base
There was a problem hiding this comment.
i also renamed it.....
i'll attempt to put in a deprecations for this. the problem is that write(io, p, l) is supposed to be the same as write(io, p); write(io, l), but many IO streams were abusing some variant of this method signature to express the unsafe_write concept.
There was a problem hiding this comment.
right but the renamed version is getting called elsewhere. It looks like you have a more generic method that calls convert below.
vtjnash
force-pushed
the
jn/unsafe_write
branch
from
eff6204 to
e65891e
Compare
| @@ -178,23 +181,17 @@ function write(to::IO, from::IO) | |||
| end | |||
| end | |||
|
|
|||
| @inline unsafe_read{T}(s::IO, p::Ref{T}, n::Integer) = unsafe_read(s, unsafe_convert(Ref{T}, p)::Ptr, n) # mark noinline to ensure ref is gc-rooted somewhere (by the caller) | |||
There was a problem hiding this comment.
using @inline but comment says @noinline ?
|
seeing no comments, i assume this will be good to merge (once I fix tkelman's comment above) |
vtjnash
force-pushed
the
jn/unsafe_write
branch
from
e65891e to
60946e0
Compare
| if r < nb | ||
| throw(EOFError()) | ||
| readbytes!(s::LibuvStream, a::Vector{UInt8}, nb = length(a)) = readbytes!(s, a, Int(nb)) | ||
| function readbytes!(s::LibuvStream, a::Vector{UInt8}, nb::Int) |
There was a problem hiding this comment.
this is almost entirely copy-pasted from below, which is not great.
There was a problem hiding this comment.
yeah, i'm hoping to address this function in some future PR. right now, every implementation of it has slightly different behavior, but in significant ways.
…to end up in the hot-path
…:Uint) this better clarifies the behavior of this method (the fallback for write(io, xs...) usually calls write on each element of xs) and provides a fallback implementation for every subtype IO this also allows the deletion of a substantial amount of duplicated and special-case code that is now made redundant once Ref can be stack-allocated, this approach should gain an extra speed bump
vtjnash
force-pushed
the
jn/unsafe_write
branch
from
60946e0 to
8b97743
Compare
add unsafe read/write as the lowest-level IO functions to enable IO subtypes to implement efficient block-transfers
| end | ||
|
|
||
| if nb <= SZ_UNBUFFERED_IO # Under this limit we are OK with copying the array from the stream's buffer | ||
| wait_readnb(s, nb) |
There was a problem hiding this comment.
Should this be wait_readnb(s, Int(nb))?
Edit: Sorry, I confused the signature on line 904 with line 905. Nevermind.
There was a problem hiding this comment.
no. nb is already coerced to an Int in the call signature
as mentioned at #14678 (comment), this renames
write(io, ptr, nb)tounsafe_write(for consistency withunsafe_load, etc.) and ensures it exists for all IO subtypes. this makes the interface for creating an IO stream much simpler and a bit more efficient.i'll add the corresponding change todoneunsafe_readlater