From 4bedff4a0247c4282a84f7e42512d343522bda28 Mon Sep 17 00:00:00 2001
From: Wojtek Mach <wojtek@wojtekmach.pl>
Date: Mon, 5 Aug 2024 12:04:43 +0200
Subject: [PATCH] Finch.request/3: Use improper list and avoid Enum.reverse
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I tried benchmarking this and it's inconclusive but I think conceptually
it makes more sense.

    {:ok, _} =
      Bandit.start_link(
        port: 4000,
        plug: fn conn, _ ->
          conn = Plug.Conn.send_chunked(conn, 200)

          Enum.reduce(1..1000, conn, fn _, conn ->
            {:ok, conn} = Plug.Conn.chunk(conn, String.duplicate("0", 1000))
            conn
          end)
        end
      )

    {:ok, _} = Finch.start_link(name: :a)
    {:ok, _} = Finch.start_link(name: :b)

    req = Finch.build(:get, "http://localhost:4000")

    Benchee.run(
      %{
        "a" => fn ->
          fun = fn
            {:status, value}, {_, headers, body, trailers} ->
              {:cont, {value, headers, body, trailers}}

            {:headers, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers ++ value, body, trailers}}

            {:data, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers, [value | body], trailers}}

            {:trailers, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers, body, trailers ++ value}}
          end

          {:ok, {200, _headers, body, _trailers}} =
            Finch.stream_while(req, :a, {nil, [], [], []}, fun)

          body |> Enum.reverse() |> IO.iodata_to_binary() |> byte_size()
        end,
        "b" => fn ->
          fun = fn
            {:status, value}, {_, headers, body, trailers} ->
              {:cont, {value, headers, body, trailers}}

            {:headers, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers ++ value, body, trailers}}

            {:data, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers, [body | value], trailers}}

            {:trailers, value}, {status, headers, body, trailers} ->
              {:cont, {status, headers, body, trailers ++ value}}
          end

          {:ok, {200, _headers, body, _trailers}} =
            Finch.stream_while(req, :b, {nil, [], [], []}, fun)

          body |> IO.iodata_to_binary() |> byte_size()
        end
      },
      time: 10,
      memory_time: 2
    )

Results:

    12:00:04.273 [info] Running #Function<0.20032135 in file:bench.exs> with Bandit 1.5.3 at 0.0.0.0:4000 (http)
    Operating System: macOS
    CPU Information: Apple M2
    Number of Available Cores: 8
    Available memory: 24 GB
    Elixir 1.18.0-dev
    Erlang 27.0.1
    JIT enabled: true

    Benchmark suite executing with the following configuration:
    warmup: 2 s
    time: 10 s
    memory time: 2 s
    reduction time: 0 ns
    parallel: 1
    inputs: none specified
    Estimated total run time: 28 s

    Benchmarking a ...
    Benchmarking b ...
    Calculating statistics...
    Formatting results...

    Name           ips        average  deviation         median         99th %
    a            95.77       10.44 ms     ±2.19%       10.43 ms       11.21 ms
    b            95.43       10.48 ms     ±2.02%       10.46 ms       11.13 ms

    Comparison:
    a            95.77
    b            95.43 - 1.00x slower +0.0377 ms

    Memory usage statistics:

    Name         average  deviation         median         99th %
    a            2.24 MB     ±0.50%        2.24 MB        2.25 MB
    b            2.23 MB     ±0.35%        2.23 MB        2.24 MB

    Comparison:
    a            2.24 MB
    b            2.23 MB - 1.00x memory usage -0.01002 MB
---
 lib/finch.ex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/finch.ex b/lib/finch.ex
index a91880d..7d3f092 100644
--- a/lib/finch.ex
+++ b/lib/finch.ex
@@ -484,7 +484,7 @@ defmodule Finch do
           {:cont, {status, headers ++ value, body, trailers}}
 
         {:data, value}, {status, headers, body, trailers} ->
-          {:cont, {status, headers, [value | body], trailers}}
+          {:cont, {status, headers, [body | value], trailers}}
 
         {:trailers, value}, {status, headers, body, trailers} ->
           {:cont, {status, headers, body, trailers ++ value}}
@@ -495,7 +495,7 @@ defmodule Finch do
          %Response{
            status: status,
            headers: headers,
-           body: body |> Enum.reverse() |> IO.iodata_to_binary(),
+           body: IO.iodata_to_binary(body),
            trailers: trailers
          }}
       end