body recursive fetch_all without Enum.reverse

[ruslandoga]

closes #204

[ruslandoga]

rows should've been reversed here before concatenating, I think

[warmwaffles]

You are right. They should have been reversed, before appending.

[ruslandoga]

And since the reason for changing fetch_all was performance, here's a naive benchmark showing that the performance is unchanged.

bench.exs

Mix.install([:benchee, :exqlite])
alias Exqlite.Sqlite3

defmodule Setup do
  def create_users(conn, count) do
    :ok = Sqlite3.execute(conn, "create table users (id integer primary key, name text)")
    {:ok, stmt} = Sqlite3.prepare(conn, "insert into users values (?, ?)")
    Sqlite3.execute(conn, "begin")

    Enum.each(1..count, fn i ->
      :ok = Sqlite3.bind(conn, stmt, [i, "User-" <> to_string(i)])
      :done = Sqlite3.step(conn, stmt)
    end)

    Sqlite3.execute(conn, "commit")
  end
end

defmodule Tail do
  def fetch_all(conn, statement) do
    fetch_all(conn, statement, 50, [])
  end

  defp fetch_all(conn, statement, chunk_size, accum) do
    case Sqlite3.multi_step(conn, statement, chunk_size) do
      {:done, rows} ->
        case accum do
          [] -> {:ok, rows}
          accum -> {:ok, Enum.reverse(rows ++ accum)}
        end

      {:rows, rows} ->
        fetch_all(conn, statement, chunk_size, Enum.reverse(rows) ++ accum)

      {:error, reason} ->
        {:error, reason}

      :busy ->
        {:error, "Database busy"}
    end
  end
end

defmodule Body do
  def fetch_all(conn, statement) do
    {:ok, try_fetch_all(conn, statement, 50)}
  catch
    :throw, {:error, _reason} = error -> error
  end

  defp try_fetch_all(conn, statement, chunk_size) do
    case Sqlite3.multi_step(conn, statement, chunk_size) do
      {:done, rows} -> rows
      {:rows, rows} -> rows ++ try_fetch_all(conn, statement, chunk_size)
      {:error, _reason} = error -> throw(error)
      :busy -> throw({:error, "Database busy"})
    end
  end
end

{:ok, conn} = Sqlite3.open(":memory:")
:ok = Setup.create_users(conn, 10000)
{:ok, stmt} = Sqlite3.prepare(conn, "select * from users")

Benchee.run(
  %{
    "body-recursive" => fn -> Body.fetch_all(conn, stmt) end,
    "tail-recursive" => fn -> Tail.fetch_all(conn, stmt) end
  },
  memory_time: 2
)

> elixir bench.exs

Operating System: macOS
CPU Information: Apple M1
Number of Available Cores: 8
Available memory: 8 GB
Elixir 1.13.4
Erlang 25.0

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

Benchmarking body-recursive ...
Benchmarking tail-recursive ...

Name                     ips        average  deviation         median         99th %
body-recursive        134.14        7.45 ms    ±14.22%        7.45 ms        9.39 ms
tail-recursive        131.50        7.60 ms    ±16.37%        7.63 ms        9.66 ms

Comparison:
body-recursive        134.14
tail-recursive        131.50 - 1.02x slower +0.150 ms

Memory usage statistics:

Name              Memory usage
body-recursive         1.49 MB
tail-recursive         1.70 MB - 1.14x memory usage +0.21 MB

**All measurements for memory usage were the same**

[warmwaffles]

Fix released as v0.11.2 thanks @ruslandoga

[SebastianSzturo]

Thanks for fixing this properly! Looks like the initial bottle neck was not the body recursion 😅