Optimize HTTP/2 request creation #423
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the-mikedavis
commented
Feb 3, 2024
| | headers | ||
| ] | ||
| end | ||
| end | ||
|
|
||
| @spec is_method?(proposed :: binary(), method :: charlist()) :: boolean() | ||
| defp is_method?(<<>>, []), do: true |
There was a problem hiding this comment.
I did some benchmarking here to try to find the right replacement for the equality check:
Benchee script...
Mix.install([:benchee])
Code.ensure_loaded(String)
defmodule Helper do
@spec is_method?(proposed :: binary(), method :: charlist()) :: boolean()
def is_method?(<<>>, []), do: true
def is_method?(<<char, rest_bin::binary>>, [char | rest_list]),
do: is_method?(rest_bin, rest_list)
def 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
def is_method?(_proposed, _method), do: false
@spec is_method_2?(proposed :: binary(), method :: binary()) :: boolean()
def is_method_2?(method, method), do: true
def is_method_2?(<<char, rest_proposed::binary>>, <<char, rest_method::binary>>),
do: is_method_2?(rest_proposed, rest_method)
def is_method_2?(<<lower_char, rest_proposed::binary>>, <<char, rest_method::binary>>)
when lower_char >= ?a and lower_char <= ?z and lower_char - 32 == char do
is_method_2?(rest_proposed, rest_method)
end
def is_method_2?(_proposed, _method), do: false
end
source = "CONNECT"
Benchee.run(
%{
"String.upcase/1" => fn ->
String.upcase(source) == "CONNECT"
end,
"String.upcase(source, :ascii)" => fn ->
String.upcase(source, :ascii) == "CONNECT"
end,
"Helper.is_method?" => fn ->
Helper.is_method?(source, ~c"CONNECT")
end,
"Helper.is_method_2?" => fn ->
Helper.is_method_2?(source, ~c"CONNECT")
end
},
warmup: 3,
time: 15,
memory_time: 15
)Results...
source = "POST"
---
Name ips average deviation median 99th %
Helper.is_method? 6.99 M 143.04 ns ±40533.83% 100 ns 230 ns
Helper.is_method_2? 4.85 M 206.05 ns ±34158.32% 130 ns 401 ns
String.upcase(source, :ascii) 3.76 M 265.78 ns ±21955.37% 190 ns 501 ns
String.upcase/1 3.57 M 280.20 ns ±18536.86% 221 ns 541 ns
Comparison:
Helper.is_method? 6.99 M
Helper.is_method_2? 4.85 M - 1.44x slower +63.00 ns
String.upcase(source, :ascii) 3.76 M - 1.86x slower +122.73 ns
String.upcase/1 3.57 M - 1.96x slower +137.16 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 128 B - 3.20x memory usage +88 B
String.upcase/1 192 B - 4.80x memory usage +152 B
source = "connect"
---
Name ips average deviation median 99th %
Helper.is_method? 6.18 M 161.80 ns ±34822.53% 130 ns 230 ns
Helper.is_method_2? 4.79 M 208.91 ns ±33755.96% 161 ns 411 ns
String.upcase(source, :ascii) 3.20 M 312.49 ns ±21079.05% 221 ns 521 ns
String.upcase/1 2.69 M 371.45 ns ±17205.79% 241 ns 1733 ns
Comparison:
Helper.is_method? 6.18 M
Helper.is_method_2? 4.79 M - 1.29x slower +47.11 ns
String.upcase(source, :ascii) 3.20 M - 1.93x slower +150.69 ns
String.upcase/1 2.69 M - 2.30x slower +209.66 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 176 B - 4.40x memory usage +136 B
String.upcase/1 288 B - 7.20x memory usage +248 B
source = "CONNECT"
---
Name ips average deviation median 99th %
Helper.is_method? 6.62 M 150.99 ns ±19398.92% 120 ns 241 ns
Helper.is_method_2? 4.95 M 202.14 ns ±25693.65% 160 ns 411 ns
String.upcase(source, :ascii) 3.26 M 307.22 ns ±22254.21% 220 ns 520 ns
String.upcase/1 2.77 M 360.69 ns ±13072.92% 241 ns 1753 ns
Comparison:
Helper.is_method? 6.62 M
Helper.is_method_2? 4.95 M - 1.34x slower +51.14 ns
String.upcase(source, :ascii) 3.26 M - 2.03x slower +156.22 ns
String.upcase/1 2.77 M - 2.39x slower +209.69 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 176 B - 4.40x memory usage +136 B
String.upcase/1 288 B - 7.20x memory usage +248 B
There was a problem hiding this comment.
Can you bench if a regex case-insensitive check performs much worse than this? Basically check for method =~ ~r/\Aconnect\z/i. Usually regexes are pretty fast and it would avoid the 15 additional LoCs 🙃
There was a problem hiding this comment.
Ah it looks like the regex ends up being a fair bit slower than the String.upcase/2s 😬
Benchee script
Mix.install([:benchee])
Code.ensure_loaded(String)
defmodule Helper do
@spec is_method?(proposed :: binary(), method :: charlist()) :: boolean()
def is_method?(<<>>, []), do: true
def is_method?(<<char, rest_bin::binary>>, [char | rest_list]),
do: is_method?(rest_bin, rest_list)
def 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
def is_method?(_proposed, _method), do: false
@spec is_method_2?(proposed :: binary(), method :: binary()) :: boolean()
def is_method_2?(method, method), do: true
def is_method_2?(<<char, rest_proposed::binary>>, <<char, rest_method::binary>>),
do: is_method_2?(rest_proposed, rest_method)
def is_method_2?(<<lower_char, rest_proposed::binary>>, <<char, rest_method::binary>>)
when lower_char >= ?a and lower_char <= ?z and lower_char - 32 == char do
is_method_2?(rest_proposed, rest_method)
end
def is_method_2?(_proposed, _method), do: false
end
source = "POST"
Benchee.run(
%{
"String.upcase/1" => fn ->
String.upcase(source) == "CONNECT"
end,
"String.upcase(source, :ascii)" => fn ->
String.upcase(source, :ascii) == "CONNECT"
end,
"Helper.is_method?" => fn ->
Helper.is_method?(source, ~c"CONNECT")
end,
"Helper.is_method_2?" => fn ->
Helper.is_method_2?(source, ~c"CONNECT")
end,
"Regex case-insensitive match" => fn ->
source =~ ~r/\Aconnect\z/i
end
},
warmup: 3,
time: 15,
memory_time: 15
)Results
source = "POST"
---
Name ips average deviation median 99th %
Helper.is_method? 7.00 M 142.93 ns ±39330.15% 91 ns 231 ns
Helper.is_method_2? 4.98 M 200.84 ns ±34584.97% 110 ns 401 ns
String.upcase(source, :ascii) 3.90 M 256.67 ns ±21923.01% 181 ns 491 ns
String.upcase/1 3.56 M 280.76 ns ±18300.86% 221 ns 501 ns
Regex case-insensitive match 1.17 M 852.35 ns ±7533.60% 761 ns 962 ns
Comparison:
Helper.is_method? 7.00 M
Helper.is_method_2? 4.98 M - 1.41x slower +57.91 ns
String.upcase(source, :ascii) 3.90 M - 1.80x slower +113.74 ns
String.upcase/1 3.56 M - 1.96x slower +137.83 ns
Regex case-insensitive match 1.17 M - 5.96x slower +709.42 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 128 B - 3.20x memory usage +88 B
String.upcase/1 192 B - 4.80x memory usage +152 B
Regex case-insensitive match 56 B - 1.40x memory usage +16 B
source = "connect"
---
Name ips average deviation median 99th %
Helper.is_method? 6.06 M 165.14 ns ±34145.79% 130 ns 251 ns
Helper.is_method_2? 4.96 M 201.66 ns ±33980.79% 110 ns 401 ns
String.upcase(source, :ascii) 3.23 M 309.30 ns ±20909.48% 221 ns 511 ns
String.upcase/1 2.69 M 372.38 ns ±17259.43% 241 ns 1723 ns
Regex case-insensitive match 1.12 M 888.92 ns ±7217.10% 792 ns 1002 ns
Comparison:
Helper.is_method? 6.06 M
Helper.is_method_2? 4.96 M - 1.22x slower +36.52 ns
String.upcase(source, :ascii) 3.23 M - 1.87x slower +144.16 ns
String.upcase/1 2.69 M - 2.25x slower +207.24 ns
Regex case-insensitive match 1.12 M - 5.38x slower +723.78 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 176 B - 4.40x memory usage +136 B
String.upcase/1 288 B - 7.20x memory usage +248 B
Regex case-insensitive match 56 B - 1.40x memory usage +16 B
source = "CONNECT"
---
Name ips average deviation median 99th %
Helper.is_method? 6.37 M 157.06 ns ±35458.04% 120 ns 250 ns
Helper.is_method_2? 4.97 M 201.05 ns ±34113.80% 110 ns 401 ns
String.upcase(source, :ascii) 3.27 M 306.06 ns ±21259.05% 221 ns 511 ns
String.upcase/1 2.73 M 366.85 ns ±17525.86% 240 ns 1733 ns
Regex case-insensitive match 1.13 M 887.71 ns ±6217.62% 802 ns 1022 ns
Comparison:
Helper.is_method? 6.37 M
Helper.is_method_2? 4.97 M - 1.28x slower +43.99 ns
String.upcase(source, :ascii) 3.27 M - 1.95x slower +149.01 ns
String.upcase/1 2.73 M - 2.34x slower +209.79 ns
Regex case-insensitive match 1.13 M - 5.65x slower +730.66 ns
Memory usage statistics:
Name Memory usage
Helper.is_method? 40 B
Helper.is_method_2? 64 B - 1.60x memory usage +24 B
String.upcase(source, :ascii) 176 B - 4.40x memory usage +136 B
String.upcase/1 288 B - 7.20x memory usage +248 B
Regex case-insensitive match 56 B - 1.40x memory usage +16 B
the-mikedavis
force-pushed
the
optimize-http/2-request-creation
branch
from
7c78455
to
5f66021
Compare
the-mikedavis
force-pushed
the
optimize-http/2-request-creation
branch
2 times, most recently
from
06f620f
to
7bb2e5b
Compare
Pull Request Test Coverage Report for Build 7bb2e5bf376de14cf298d262e723658c7c60747f-PR-423
💛 - Coveralls |
whatyouhide
reviewed
Feb 3, 2024
| | headers | ||
| ] | ||
| end | ||
| end | ||
|
|
||
| @spec is_method?(proposed :: binary(), method :: charlist()) :: boolean() | ||
| defp is_method?(<<>>, []), do: true |
There was a problem hiding this comment.
Can you bench if a regex case-insensitive check performs much worse than this? Basically check for method =~ ~r/\Aconnect\z/i. Usually regexes are pretty fast and it would avoid the 15 additional LoCs 🙃
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.
`String.upcase/1` by default iterates over the graphemes in the input string which can be relatively slow. Since we're only interested in case-insensitive equality, we can write a small custom equality checking function that can avoid creating new terms and can return early. Using a charlist is slightly faster than passing `"CONNECT"` as a binary.
the-mikedavis
force-pushed
the
optimize-http/2-request-creation
branch
from
7bb2e5b
to
f8ece36
Compare
|
Ah this is fantastic, thanks for the benchmarks @the-mikedavis 💟 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is some low-hanging fruit to improve the time it takes to open a new request with HTTP/2 by avoiding
URI.default_port/1andString.upcase/1calls.With some very un-scientific
:timer.tc/1benchmarking I see these changes reduce the time it takes to complete aMint.HTTP2.request/5call (passing:streamas the data argument) from 12.8ms to 2.5ms.