Update insertion benchmark #144
Conversation
MIX_ENV=bench doesn't exist, and postgrex and myxql are installed for MIX_ENV=dev, so we can simplify this
5.7 won't start on arm64 architecture
|
@gmile you've stumbled on a bug. How did you run the benchmarks. Were you running MySQL and Postgres locally vs running them in docker? Overall thanks for bumping the benchmarks! |
|
@warmwaffles to your question:
I ran MySQL in a container, but PostgreSQL locally (natively, running the the host OS). Your question made me realise its not fair to compare results of SQLite3 and PostgreSQL running on my computer natively with results for MySQL running inside a container, e.g. via virtualization which has a performance penalty. I'm using orbstack which claims to be faster the Docker, but still. I'll install MySQL on my mac natively and will regenerate the benchmark once again. On a separately note, to clarify existence of commit a9902f3. Using docker image for MySQL 5.7 didn't work for me, because there's no arm64 OCI manifest generated for 5.7 version, but there's for 8. |
|
I was able to reproduce the exception mentioned in my message with this self-contained Elixir script: Mix.install([:ecto_sqlite3])
Application.put_env(:foo, Repo, pool_size: 1, database: ":memory:")
defmodule Repo do
use Ecto.Repo,
adapter: Ecto.Adapters.SQLite3,
otp_app: :foo
end
defmodule Migration0 do
use Ecto.Migration
def change do
create table("posts") do
add(:title, :string)
add(:published_at, :time)
end
end
end
defmodule Post do
use Ecto.Schema
schema "posts" do
field(:title, :string)
field(:published_at, :time)
end
end
defmodule Main do
import Ecto.Query, warn: false
def reproduce do
:ok = Repo.__adapter__().storage_up(Repo.config())
{:ok, _pid} = Supervisor.start_link([Repo], strategy: :one_for_one)
Ecto.Migrator.run(Repo, [{0, Migration0}], :up, all: true, log_migrations_sql: :debug)
Repo.load(Post, %{title: "New blog post", published_at: ~T[21:25:04.361140]})
end
end
Main.reproduce()If I make a change to a script - it will no longer raise: diff --git a/time-issue-repro.exs b/time-issue-repro.exs
index 8643d65..52e173e 100644
--- a/time-issue-repro.exs
+++ b/time-issue-repro.exs
@@ -38,7 +38,7 @@ defmodule Main do
Ecto.Migrator.run(Repo, [{0, Migration0}], :up, all: true, log_migrations_sql: :debug)
- Repo.load(Post, %{title: "New blog post", published_at: ~T[21:25:04.361140]})
+ Repo.load(Post, %{title: "New blog post", published_at: "21:25:04.361140")
end
end |
gmile
force-pushed
the
refresh-insert-benchmark
branch
from
43a2ee6 to
117488c
Compare
|
@warmwaffles I added this commit bceff9a just to see if I am able to get pass the issue when running the benchmark It work, so not only "insert" part, but all benchmarks were regenerated successfully. However, I don't believe my fix is correct. For example, running benchmark having ecto_sqlite3/bench/scripts/micro/load_bench.exs Lines 33 to 34 in 86b76bb I am not sure why explicit handling of |
|
Sorry this took so long to merge. It completely slipped through on me. Feel free to ping mention me if you see another PR for these libraries fall through. |
|
@warmwaffles noted, and no worries, I kind of forgot about this one too 🙈 Thanks for merging! I just hope this change is not going to break anything bceff9a 🙏 |
|
It shouldn't. You added a test for it and I think it was just an oversight by me initially. |
This refreshes insertion benchmark stats + does a couple of minor fixes to the bench README.md.
I couldn't update all benchmarks due to a failure that I haven't had time to look into. Here's a full failure output: