Skip to content

Commit

Permalink
Vastly improve perf of parsing bulk strings (#247)
Browse files Browse the repository at this point in the history
  • Loading branch information
whatyouhide committed Apr 17, 2023
1 parent b890b0b commit 56a0851
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 14 deletions.
3 changes: 2 additions & 1 deletion .formatter.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
inputs: [
"mix.exs",
"lib/**/*.ex",
"test/**/*.exs"
"test/**/*.exs",
"benchmarks/**/*.exs",
],
import_deps: [:stream_data]
]
38 changes: 38 additions & 0 deletions benchmarks/protocol.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
Mix.install([
{:redix, path: "."},
{:benchee, "~> 1.1"},
{:benchee_html, "~> 1.0"},
{:benchee_markdown, "~> 0.3"},
{:eredis, "~> 1.7"}
])

defmodule Helpers do
def parse_with_continuations([data | datas], cont \\ &Redix.Protocol.parse/1) do
case cont.(data) do
{:continuation, new_cont} -> parse_with_continuations(datas, new_cont)
{:ok, value, rest} -> {:ok, value, rest}
end
end
end

Benchee.run(
%{
"Parse a bulk string split into 1kb chunks" => fn %{chunks: datas} ->
{:ok, _value, _rest} = Helpers.parse_with_continuations(datas)
end
},
# Inputs are expressed in number of 1kb chunks
inputs: %{
"1 Kb" => 1,
"1 Mb" => 1024,
"70 Mb" => 70 * 1024
},
before_scenario: fn chunks_of_1kb ->
chunks = for _ <- 1..chunks_of_1kb, do: :crypto.strong_rand_bytes(1024)
total_size = chunks_of_1kb * 1024
chunks = ["$#{total_size}\r\n" | chunks] ++ ["\r\n"]

%{chunks: chunks}
end,
save: [path: "redix-main.benchee", tag: "main"]
)
29 changes: 16 additions & 13 deletions lib/redix/protocol.ex
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,7 @@ defmodule Redix.Protocol do

defp parse_integer_without_sign(<<digit, _::binary>> = bin) when digit in ?0..?9 do
resolve_cont(parse_integer_digits(bin, 0), fn i, rest ->
resolve_cont(until_crlf(rest), fn
"", rest ->
{:ok, i, rest}

<<char, _::binary>>, _rest ->
raise ParseError, message: "expected CRLF, found: #{inspect(<<char>>)}"
end)
resolve_cont(crlf(rest), fn :no_value, rest -> {:ok, i, rest} end)
end)
end

Expand All @@ -167,17 +161,19 @@ defmodule Redix.Protocol do
{:ok, nil, rest}

size, rest ->
parse_string_of_known_size(rest, size)
parse_string_of_known_size(rest, _acc = [], _size_left = size)
end)
end

defp parse_string_of_known_size(data, size) do
defp parse_string_of_known_size(data, acc, size_left) do
case data do
<<str::bytes-size(size), @crlf, rest::binary>> ->
{:ok, str, rest}
str when byte_size(str) < size_left ->
{:continuation, &parse_string_of_known_size(&1, [acc | str], size_left - byte_size(str))}

_ ->
{:continuation, &parse_string_of_known_size(data <> &1, size)}
<<str::bytes-size(size_left), rest::binary>> ->
resolve_cont(crlf(rest), fn :no_value, rest ->
{:ok, IO.iodata_to_binary([acc | str]), rest}
end)
end
end

Expand All @@ -198,6 +194,13 @@ defmodule Redix.Protocol do
defp until_crlf(<<?\r>>, acc), do: {:continuation, &until_crlf(<<?\r, &1::binary>>, acc)}
defp until_crlf(<<byte, rest::binary>>, acc), do: until_crlf(rest, <<acc::binary, byte>>)

defp crlf(<<@crlf, rest::binary>>), do: {:ok, :no_value, rest}
defp crlf(<<?\r>>), do: {:continuation, &crlf(<<?\r, &1::binary>>)}
defp crlf(<<>>), do: {:continuation, &crlf/1}

defp crlf(<<byte, _::binary>>),
do: raise(ParseError, message: "expected CRLF, found: #{inspect(<<byte>>)}")

defp take_elems(data, 0, acc) do
{:ok, Enum.reverse(acc), data}
end
Expand Down
5 changes: 5 additions & 0 deletions test/redix/protocol_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ defmodule Redix.ProtocolTest do
assert parse_with_continuations(split_command) == {:ok, nil, ""}
assert parse_with_continuations(split_command_with_rest) == {:ok, nil, "rest"}
end

# No CRLF after bulk string contents
assert_raise ParseError, "expected CRLF, found: \"n\"", fn ->
parse("$3\r\nfoonocrlf")
end
end

property "arrays" do
Expand Down

0 comments on commit 56a0851

Please sign in to comment.