Skip to content

Commit 426b15b

Browse files
authored
Avoid write(::IO, ::String) invalidations from Stream and IO
* Avoid write(::IO, ::String) invalidations from Stream and deadline IO Extending Base.write for String/AbstractString on a new IO type invalidates every abstractly-inferred write(::IO, ::String) call site in the ecosystem (~1380 instances each at using time, measured with @snoop_invalidations). Hook Base.unsafe_write instead: Base's generic string write funnels through it, so behavior is unchanged while the method-table impact disappears. * Remove invalidation-prone abstract edges from write and websocket paths - http1.jl / http_transport.jl: Int-assert length/write results where the stream is deliberately untyped, so loop arithmetic and comparisons stay concretely inferred instead of carrying >(::Any, ::Int) edges. - http_server_streams.jl: write concrete Vector{UInt8}/String in the server write path; the abstract write(io, ::AbstractVector{UInt8}) and write(::Stream, ::AbstractString) edges get invalidated whenever any package defines write methods for its own types (LaTeXStrings, ...). - http_websocket_codec.jl: use a Ref instead of a closure-reassigned local; the Core.Box capture infers Any and the resulting >(::UInt64, ::Any) edges get invalidated by packages defining broad comparison methods (SIMD.jl), recompiling the websocket receive path on first use. * Define write(::Stream, ::UInt8) so generic AbstractStrings don't throw String and SubString{String} writes route through unsafe_write, but Base's fallbacks for other AbstractStrings (and Char) write char-by-char and bottom out in write(io, ::UInt8), which previously threw "Stream does not support byte I/O". Adding the byte primitive causes no invalidations (measured with @snoop_invalidations against a loaded Bonito/Makie stack), since it is the method every IO subtype is expected to define.
1 parent d48b520 commit 426b15b

5 files changed

Lines changed: 90 additions & 15 deletions

File tree

src/http1.jl

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -597,15 +597,19 @@ end
597597
function _write_exact_bytes_body!(stream, body::BytesBody, expected_len::Int64)
598598
expected_len < 0 && throw(ArgumentError("expected_len must be >= 0"))
599599
expected_len == 0 && return nothing
600-
available = (length(body.data) - body.next_index) + 1
600+
# `body` is usually abstract here and `stream` is deliberately untyped;
601+
# the Int asserts keep the arithmetic/comparisons concretely inferred so
602+
# this method carries no invalidation-prone `>=(::Any, ::Int)` edges
603+
len = length(body.data)::Int
604+
available = (len - body.next_index) + 1
601605
available >= expected_len || throw(ProtocolError("body ended before expected Content-Length bytes were written"))
602606
stop_index = body.next_index + Int(expected_len) - 1
603-
chunk = if body.next_index == 1 && stop_index == length(body.data)
607+
chunk = if body.next_index == 1 && stop_index == len
604608
body.data
605609
else
606610
view(body.data, body.next_index:stop_index)
607611
end
608-
n = write(stream, chunk)
612+
n = Int(write(stream, chunk))
609613
n == expected_len || throw(ProtocolError("transport short write"))
610614
body.next_index = stop_index + 1
611615
return nothing

src/http_server_streams.jl

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,10 @@ function _server_write(stream::Stream, data::AbstractVector{UInt8})::Int
295295
if stream.write_mode == _ServerStreamWriteMode.CHUNKED
296296
io = IOBuffer()
297297
print(io, string(length(data), base=16), "\r\n")
298-
write(io, data)
298+
# concrete bytes: `write(io, ::AbstractVector{UInt8})` is an
299+
# invalidation-prone edge (any package adding write methods recompiles
300+
# the server write path); no copy in the common Vector case
301+
write(io, data isa Vector{UInt8} ? data : Vector{UInt8}(data))
299302
write(io, "\r\n")
300303
_write_server_stream_bytes!(stream, take!(io))
301304
else
@@ -394,11 +397,15 @@ function _write_response_body_to_stream!(stream::Stream, body)::Nothing
394397
return nothing
395398
end
396399
if body isa AbstractString
397-
write(stream, body::AbstractString)
400+
# concrete String: a `write(::Stream, ::AbstractString)` edge gets
401+
# invalidated whenever any package defines a write method for its
402+
# own string type (LaTeXStrings, ...), recompiling the server
403+
# response path
404+
write(stream, body isa String ? body : String(body))
398405
return nothing
399406
end
400407
if body isa AbstractVector{UInt8}
401-
write(stream, body::AbstractVector{UInt8})
408+
write(stream, body isa Vector{UInt8} ? body : Vector{UInt8}(body))
402409
return nothing
403410
end
404411
if body isa AbstractBody

src/http_stream.jl

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,34 @@ function write(stream::Stream{true}, data::AbstractVector{UInt8})::Int
253253
end
254254
write(stream::Stream{false}, data::AbstractVector{UInt8}) = _server_write(stream, data)
255255

256-
function _client_stream_write(stream::Stream{true}, data::AbstractString)::Int
256+
# Strings are handled via unsafe_write instead of write(::Stream, ::AbstractString)
257+
# methods: Base's generic `write(io::IO, s::Union{String, SubString{String}})`
258+
# funnels through unsafe_write, and extending `write` for string types on a new
259+
# IO type invalidates every abstractly-inferred `write(::IO, ::String)` call
260+
# site in the ecosystem (measured: ~1380 invalidated instances at `using` time).
261+
function Base.unsafe_write(stream::Stream{true}, p::Ptr{UInt8}, n::UInt)::UInt
262+
n == 0 && return UInt(0)
257263
(@atomic :acquire stream.started) && throw(ArgumentError("cannot write request body after response reading has started"))
258264
(@atomic :acquire stream.write_closed) && throw(ArgumentError("request body writes are closed"))
259-
return write(stream.request_buffer, String(data))
265+
return unsafe_write(stream.request_buffer, p, n)
266+
end
267+
268+
function Base.unsafe_write(stream::Stream{false}, p::Ptr{UInt8}, n::UInt)::UInt
269+
n == 0 && return UInt(0)
270+
data = Vector{UInt8}(undef, Int(n))
271+
GC.@preserve data unsafe_copyto!(pointer(data), p, n)
272+
return UInt(_server_write(stream, data))
260273
end
261274

262-
write(stream::Stream{true}, data::AbstractString)::Int = _client_stream_write(stream, data)
263-
write(stream::Stream{true}, data::Union{String,SubString{String}})::Int = _client_stream_write(stream, data)
264-
write(stream::Stream{false}, data::AbstractString) = _server_write(stream, data)
265-
write(stream::Stream{false}, data::Union{String,SubString{String}}) = _server_write(stream, data)
275+
# The byte-I/O primitive: Base's fallbacks for `write(io, ::AbstractString)`
276+
# and `write(io, ::Char)` bottom out in `write(io, ::UInt8)`, so without this
277+
# generic strings throw "does not support byte I/O".
278+
function write(stream::Stream{true}, b::UInt8)::Int
279+
(@atomic :acquire stream.started) && throw(ArgumentError("cannot write request body after response reading has started"))
280+
(@atomic :acquire stream.write_closed) && throw(ArgumentError("request body writes are closed"))
281+
return write(stream.request_buffer, b)
282+
end
283+
write(stream::Stream{false}, b::UInt8) = _server_write(stream, UInt8[b])
266284

267285
function closewrite(stream::Stream{true})
268286
@atomic :release stream.write_closed = true

src/http_transport.jl

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,14 @@ Base.write(io::_RequestDeadlineWriteIO, data::Vector{UInt8})::Int = _request_dea
7878
Base.write(io::_RequestDeadlineWriteIO, data::StridedVector{UInt8})::Int = _request_deadline_write_bytes!(io, data)
7979
Base.write(io::_RequestDeadlineWriteIO, data::AbstractVector{UInt8})::Int = _request_deadline_write_bytes!(io, data)
8080

81-
function Base.write(io::_RequestDeadlineWriteIO, data::Union{String,SubString{String}})::Int
81+
# Hook unsafe_write instead of write(::IO, ::Union{String, SubString{String}}):
82+
# Base's generic string write funnels through unsafe_write, so strings still
83+
# get the deadline applied, while extending `write` for String on a new IO
84+
# type invalidates every abstractly-inferred `write(::IO, ::String)` call site
85+
# in the ecosystem (measured: 1383 invalidated instances at `using` time).
86+
function Base.unsafe_write(io::_RequestDeadlineWriteIO, p::Ptr{UInt8}, n::UInt)::Int
8287
_apply_request_write_deadline!(io)
83-
return write(io.inner, data)
88+
return unsafe_write(io.inner, p, n)
8489
end
8590

8691
@inline function _fill_conn_reader!(reader::_ConnReader)::Int
@@ -430,7 +435,10 @@ function _write_exact_bytes_body_transport!(
430435
else
431436
view(body.data, body.next_index:stop_index)
432437
end
433-
n = write(stream, chunk)
438+
# `stream` is deliberately untyped (multiple transports); without the
439+
# Int assert `n` infers Any and poisons the loop comparisons with
440+
# invalidation-prone `>(::Any, ::Int)` edges
441+
n = Int(write(stream, chunk))
434442
n == chunk_len || throw(ProtocolError("transport short write"))
435443
body.next_index = stop_index + 1
436444
remaining -= n

test/http_server_http1_tests.jl

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,44 @@ end
948948
end
949949
end
950950

951+
@testset "HTTP stream byte I/O supports generic AbstractStrings and Chars" begin
952+
# Strings are written via unsafe_write; generic AbstractStrings and Chars
953+
# fall back to Base's per-byte path, which requires write(io, ::UInt8).
954+
server = HT.listen!("127.0.0.1", 0; listenany = true) do stream
955+
_ = HT.startread(stream)
956+
body = String(read(stream))
957+
HT.setstatus(stream, 200)
958+
HT.startwrite(stream)
959+
if isempty(body)
960+
write(stream, Test.GenericString("generic"))
961+
write(stream, ' ')
962+
write(stream, 0x21)
963+
else
964+
write(stream, body)
965+
end
966+
return nothing
967+
end
968+
address = HT.server_addr(server)
969+
try
970+
resp = HT.get("http://$(address)/")
971+
@test resp.status == 200
972+
@test String(resp.body) == "generic !"
973+
974+
echoed = Ref("")
975+
resp2 = HT.open(:POST, "http://$(address)/") do io
976+
write(io, Test.GenericString("client generic"))
977+
write(io, '!')
978+
_ = HT.startread(io)
979+
echoed[] = String(read(io))
980+
end
981+
@test resp2.status == 200
982+
@test echoed[] == "client generic!"
983+
finally
984+
_run_with_timeout(() -> HT.forceclose(server); label = "server forceclose")
985+
_run_with_timeout(() -> wait(server); label = "server task completion")
986+
end
987+
end
988+
951989
@testset "HTTP server stream handlers reject fixed-length mismatches before writing malformed bodies" begin
952990
server = HT.listen!("127.0.0.1", 0; listenany = true) do stream
953991
request = HT.startread(stream)

0 commit comments

Comments
 (0)