Skip to content

Commit

Permalink
Optimize HTTP/2 request creation (#423)
Browse files Browse the repository at this point in the history
When constructing the authority pseudo header we check if the conn's
port is the default for the conn's scheme using `URI.default_port/1`.
That corresponds to an ETS lookup into the `:elixir_config` table. It's
relatively fast to read that information but the conn's port and scheme
are static for the life of the conn, so we should determine this
information once while initiating the conn (`Mint.HTTP2.initiate/5`).
This change saves a small amount of time per request and becomes more
valuable as the conn is re-used for more requests.

Using a charlist is slightly faster than passing `"CONNECT"` as a
binary.
  • Loading branch information
the-mikedavis authored Feb 5, 2024
1 parent 321c830 commit 1dd3601
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 30 deletions.
41 changes: 28 additions & 13 deletions lib/mint/http2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ defmodule Mint.HTTP2 do
:hostname,
:port,
:scheme,
:authority,

# Connection state (open, closed, and so on).
:state,
Expand Down Expand Up @@ -974,10 +975,18 @@ defmodule Mint.HTTP2 do
) :: {:ok, t()} | {:error, Types.error()}
def initiate(scheme, socket, hostname, port, opts) do
transport = scheme_to_transport(scheme)
scheme_string = Atom.to_string(scheme)
mode = Keyword.get(opts, :mode, :active)
log? = Keyword.get(opts, :log, false)
client_settings_params = Keyword.get(opts, :client_settings, [])
validate_client_settings!(client_settings_params)
# If the port is the default for the scheme, don't add it to the :authority pseudo-header
authority =
if URI.default_port(scheme_string) == port do
hostname
else
"#{hostname}:#{port}"
end

unless mode in [:active, :passive] do
raise ArgumentError,
Expand All @@ -992,10 +1001,11 @@ defmodule Mint.HTTP2 do
conn = %__MODULE__{
hostname: hostname,
port: port,
authority: authority,
transport: scheme_to_transport(scheme),
socket: socket,
mode: mode,
scheme: Atom.to_string(scheme),
scheme: scheme_string,
state: :handshaking,
log: log?
}
Expand Down Expand Up @@ -1355,23 +1365,37 @@ defmodule Mint.HTTP2 do
end

defp add_pseudo_headers(headers, conn, method, path) do
if String.upcase(method) == "CONNECT" do
if is_method?(method, ~c"CONNECT") do
[
{":method", method},
{":authority", authority_pseudo_header(conn.scheme, conn.port, conn.hostname)}
{":authority", conn.authority}
| headers
]
else
[
{":method", method},
{":path", path},
{":scheme", conn.scheme},
{":authority", authority_pseudo_header(conn.scheme, conn.port, conn.hostname)}
{":authority", conn.authority}
| headers
]
end
end

@spec is_method?(proposed :: binary(), method :: charlist()) :: boolean()
defp is_method?(<<>>, []), do: true

defp is_method?(<<char, rest_bin::binary>>, [char | rest_list]) do
is_method?(rest_bin, rest_list)
end

defp is_method?(<<lower_char, rest_bin::binary>>, [char | rest_list])
when lower_char >= ?a and lower_char <= ?z and lower_char - 32 == char do
is_method?(rest_bin, rest_list)
end

defp is_method?(_proposed, _method), do: false

defp sort_pseudo_headers_to_front(headers) do
Enum.sort_by(headers, fn {key, _value} ->
not String.starts_with?(key, ":")
Expand Down Expand Up @@ -1719,15 +1743,6 @@ defmodule Mint.HTTP2 do
end
end

# If the port is the default for the scheme, don't add it to the :authority pseudo-header
defp authority_pseudo_header(scheme, port, hostname) do
if URI.default_port(scheme) == port do
hostname
else
"#{hostname}:#{port}"
end
end

defp join_cookie_headers(headers) do
# If we have 0 or 1 Cookie headers, we just use the old list of headers.
case Enum.split_with(headers, fn {name, _value} -> Util.downcase_ascii(name) == "cookie" end) do
Expand Down
44 changes: 27 additions & 17 deletions test/mint/http2/conn_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ defmodule Mint.HTTP2Test do
@server_pdict_key {__MODULE__, :http2_test_server}

setup :start_server_async
setup :maybe_change_default_scheme_port
setup :start_connection
setup :maybe_set_transport_mock

Expand Down Expand Up @@ -841,29 +842,21 @@ defmodule Mint.HTTP2Test do
assert HTTP2.open?(conn)
end

@tag :with_overridden_default_port
test ":authority pseudo-header does not include port if it is the scheme's default",
%{conn: conn} do
default_https_port = URI.default_port("https")

try do
# Override default https port for this test
URI.default_port("https", conn.port)

{conn, _ref} = open_request(conn)
{conn, _ref} = open_request(conn)

assert_recv_frames [headers(hbf: hbf)]
assert_recv_frames [headers(hbf: hbf)]

assert {":authority", authority} =
hbf
|> server_decode_headers()
|> List.keyfind(":authority", 0)
assert {":authority", authority} =
hbf
|> server_decode_headers()
|> List.keyfind(":authority", 0)

assert authority == conn.hostname
assert authority == conn.hostname

assert HTTP2.open?(conn)
after
URI.default_port("https", default_https_port)
end
assert HTTP2.open?(conn)
end

test "when there's a request body, the content-length header is passed if not present",
Expand Down Expand Up @@ -2374,6 +2367,23 @@ defmodule Mint.HTTP2Test do
%{}
end

defp maybe_change_default_scheme_port(%{
server_port: server_port,
with_overridden_default_port: _
}) do
default_https_port = URI.default_port("https")

on_exit(fn -> URI.default_port("https", default_https_port) end)

:ok = URI.default_port("https", server_port)

%{}
end

defp maybe_change_default_scheme_port(_context) do
%{}
end

defp recv_next_frames(n) do
server = Process.get(@server_pdict_key)
TestServer.recv_next_frames(server, n)
Expand Down

0 comments on commit 1dd3601

Please sign in to comment.