Vastly improve perf of parsing bulk strings (#247)

Author: whatyouhide

.formatter.exs

@@ -2,7 +2,8 @@
   inputs: [
     "mix.exs",
     "lib/**/*.ex",
-    "test/**/*.exs"
+    "test/**/*.exs",
+    "benchmarks/**/*.exs",
   ],
   import_deps: [:stream_data]
 ]

benchmarks/protocol.exs

@@ -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"]
+)

lib/redix/protocol.ex

@@ -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
 
@@ -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
 
@@ -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

test/redix/protocol_test.exs

@@ -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