Skip to content

Commit

Permalink
Cleanup header initialization to avoid excessive copies (#991)
Browse files Browse the repository at this point in the history
* Cleanup header initialization to avoid excessive copies

Set keepalive to true by default for tcp layer.
Fix bug where both StatusCodes & Messages were exporting `statustext`.

* Update src/ConnectionPool.jl
  • Loading branch information
quinnj committed Jan 11, 2023
1 parent 0ae437a commit ea025e1
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 11 deletions.
5 changes: 4 additions & 1 deletion src/ConnectionPool.jl
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,10 @@ end
function getconnection(::Type{TCPSocket},
host::AbstractString,
port::AbstractString;
keepalive::Bool=false,
# set keepalive to true by default since it's cheap and helps keep long-running requests/responses
# alive in the face of heavy workloads where Julia's task scheduler might take a while to
# keep up with midflight requests
keepalive::Bool=true,
connect_timeout::Int=0,
readtimeout::Int=0,
kw...)::TCPSocket
Expand Down
11 changes: 7 additions & 4 deletions src/HTTP.jl
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ println(String(r.body))
`[string(k) => string(v) for (k,v) in headers]` yields `Vector{Pair}`.
e.g. a `Dict()`, a `Vector{Tuple}`, a `Vector{Pair}` or an iterator.
By convention, if a header _value_ is an empty string, it will not be written
when sending a request (following the curl convention).
when sending a request (following the curl convention). By default, a copy of
provided headers is made (since required headers are typically set during the request);
to avoid this copy and have HTTP.jl mutate the provided headers array, pass `copyheaders=false`
as an additional keyword argument to the request.
`body` can be a variety of objects:
Expand Down Expand Up @@ -278,7 +281,7 @@ HTTP.open("POST", "http://music.com/play") do io
end
```
"""
function request(method, url, h=Header[], b=nobody;
function request(method, url, h=nothing, b=nobody;
headers=h, body=b, query=nothing, kw...)::Response
return request(HTTP.stack(), method, url, headers, body, query; kw...)
end
Expand Down Expand Up @@ -418,9 +421,9 @@ function stack(
return messagelayer(debuglayer(layers4))
end

function request(stack::Base.Callable, method, url, h=Header[], b=nobody, q=nothing;
function request(stack::Base.Callable, method, url, h=nothing, b=nobody, q=nothing;
headers=h, body=b, query=q, kw...)::Response
return stack(string(method), request_uri(url, query), mkheaders(headers), body; kw...)
return stack(string(method), request_uri(url, query), headers, body; kw...)
end

macro remove_linenums!(expr)
Expand Down
9 changes: 5 additions & 4 deletions src/Messages.jl
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export Message, Request, Response,
readchunksize,
writeheaders, writestartline,
bodylength, unknown_length,
payload, decode, statustext, sprintcompact
payload, decode, sprintcompact

using URIs, CodecZlib
using ..Pairs, ..IOExtras, ..Parsers, ..Strings, ..Forms, ..Conditions
Expand Down Expand Up @@ -204,13 +204,14 @@ resource(uri::URI) = string( isempty(uri.path) ? "/" : uri.path,
!isempty(uri.fragment) ? "#" : "", uri.fragment)

mkheaders(h::Headers) = h
function mkheaders(h)::Headers
function mkheaders(h, headers=Vector{Header}(undef, length(h)))::Headers
# validation
foreach(h) do head
for (i, head) in enumerate(h)
head isa String && throw(ArgumentError("header must be passed as key => value pair: `$head`"))
length(head) != 2 && throw(ArgumentError("invalid header key-value pair: $head"))
headers[i] = SubString(string(head[1])) => SubString(string(head[2]))
end
return Header[string(k) => string(v) for (k, v) in h]
return headers
end

method(r::Request) = getfield(r, :method)
Expand Down
10 changes: 8 additions & 2 deletions src/clientlayers/MessageRequest.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,21 @@ using ..Strings: HTTPVersion

export messagelayer

# like Messages.mkheaders, but we want to make a copy of user-provided headers
# and avoid double copy when no headers provided (very common)
mkreqheaders(::Nothing, ch) = Header[]
mkreqheaders(headers::Headers, ch) = ch ? copy(headers) : headers
mkreqheaders(h, ch) = mkheaders(h)

"""
messagelayer(handler) -> handler
Construct a [`Request`](@ref) object from method, url, headers, and body.
Hard-coded as the first layer in the request pipeline.
"""
function messagelayer(handler)
return function(method::String, url::URI, headers::Headers, body; response_stream=nothing, http_version=HTTPVersion(1, 1), kw...)
req = Request(method, resource(url), headers, body; url=url, version=http_version, responsebody=response_stream)
return function(method::String, url::URI, headers, body; copyheaders::Bool=true, response_stream=nothing, http_version=HTTPVersion(1, 1), kw...)
req = Request(method, resource(url), mkreqheaders(headers, copyheaders), body; url=url, version=http_version, responsebody=response_stream)
local resp
try
resp = handler(req; response_stream=response_stream, kw...)
Expand Down

0 comments on commit ea025e1

Please sign in to comment.