Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new benchmarks #4

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

SergiiShapoval
Copy link

This PR

@aarondl
Copy link
Member

aarondl commented Jun 18, 2020

This PR is far too big and the changes were not discussed beforehand. It cannot be accepted as submitted.

I don't understand quite a few things, like why replace psql with crdb? We only tangentially support crdb via a driver that's not in the core.

Why add sqlc? It is an apples to oranges comparison. All the libraries in the benchmark generate SQL from a Go-based DSL and then send that to the DB for execution. sqlc simply throws around hardcoded strings which makes for a skewed benchmark (of course in its favor). I'm a big fan of sqlc and like the approach very much, but it doesn't make sense to be here.

@SergiiShapoval
Copy link
Author

why replace psql with crdb? We only tangentially support crdb via a driver that's not in the core.

replace of psql with crdb can be removed, was done as on current project we needed comparison based on CockroachDB

Why add sqlc?

sqlc could be a good alternative to existing golang ORMs, especially considering these performance benchmarks, e.g. we have decided to migrate to sqlc from go-pg and boilbench on our project

This PR is far too big

It can be split, proposed parts:

  • add go modules support and fixes to latest ORM versions,
  • add docker support,
  • add sqlc comparison,
  • add go-pg comparison

are these parts good or it should be divided to other parts?

also there is a benchmark against entc in my fork, but entc is still lacking some features that can be compared

@aarondl
Copy link
Member

aarondl commented Jun 18, 2020

replace of psql with crdb can be removed, was done as on current project we needed comparison based on CockroachDB

Okay good. That should be removed then. Optionally could make it work with both but default to postgresql.

sqlc could be a good alternative to existing golang ORMs, especially considering these performance benchmarks, e.g. we have decided to migrate to sqlc from go-pg and boilbench on our project

sqlc is a great alternative to ORMs. But that's just it. It's an alternative to these things because it behaves completely differently. It's always going to be faster, but it's always also going to be less flexible and slower to develop in.

Take sqlboiler for example, you get a huge swath of usable things right out of the gate, whereas if you start with sqlc you have to begin by writing your "get all users" query and your "get single user" query. It would take you a day or two just to get to the point sqlboiler gets you out of the box. But there's a tradeoff for the abstraction that sqlboiler (and all these other ORMs) give you, and that's performance and simplicity. Another example that comes to mind is that there's an abstraction layer in virtually all of these ORMs to support multiple RDBMS (mysql, postgres, mssql, sqlite3, etc) but sqlc only supports postgresql.

So I stand by my statement that it's disingenuous to benchmark against it because it's so incredibly different from the rest of the libraries here.

  • add go modules support and fixes to latest ORM versions
    • sounds good
  • add docker support
    • sure
  • add sqlc comparison
    • still against this for the reasons stated above, I'd need to be convinced it makes sense to compare
  • add go-pg comparison
    • seems fine to me

also there is a benchmark against entc in my fork, but entc is still lacking some features that can be compared

So long as it's similar enough to the other ORMs that's fine. We can have some incomplete benchmarks so long as it's noted that they don't support X or Y feature somewhere and that's why those numbers are missing.

@SergiiShapoval
Copy link
Author

ok, will divide this PR to smaller one as mentioned above except adding sqlc comparison as it is still under discussion

regarding your comments:

It's always going to be faster, but it's always also going to be less flexible and slower to develop in.
Take sqlboiler for example, you get a huge swath of usable things right out of the gate, whereas if you start with sqlc you have to begin by writing your "get all users" query and your "get single user" query. It would take you a day or two just to get to the point sqlboiler gets you out of the box. But there's a tradeoff for the abstraction that sqlboiler (and all these other ORMs) give you, and that's performance and simplicity.

On our project, we are using a Repository pattern to all DB related requests, to make DB interaction isolated from other code as much as possible.
With this pattern, we will need to describe each method separately, agree that describing it in Go-based DSLs could be faster than with SQL, but we avoid debugging of ORM SQL calls in complex queries,
also sometimes we are using Raw SQL ORM methods, writing which could take even more time with ORM,

so the difference exists, but it shouldn't be huge considering that sqlc code is generated from sql files. The schema could be also generated by automated tools from existing DBs

Another example that comes to mind is that there's an abstraction layer in virtually all of these ORMs to support multiple RDBMS (mysql, postgres, mssql, sqlite3, etc) but sqlc only supports postgresql.

kallax, go-pg are also supporting only postgresql, kallax is already existed in benchmarks, go-pg will be added in related PRs...

also, some ORMs putting additional restrictions on DB, that can be avoided with sqlc e.g. volatiletech/sqlboiler#698

add sqlc comparison
still against this for the reasons stated above, I'd need to be convinced it makes sense to compare

do I need to add some additional reasons why sqlc can be added to comparison?

So long as it's similar enough to the other ORMs that's fine. We can have some incomplete benchmarks so long as it's noted that they don't support X or Y feature somewhere and that's why those numbers are missing.

will prepare entc benchmark in separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants