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

Write benchmarks for Ecto as proof of concept #27

Open
tallysmartins opened this issue Jun 11, 2018 · 5 comments
Open

Write benchmarks for Ecto as proof of concept #27

tallysmartins opened this issue Jun 11, 2018 · 5 comments

Comments

@tallysmartins
Copy link
Member

The idea is to continue the work from the branch mm/benches in the ecto repository:

Clarification needed:

  • Should we follow the implementation of previous benchmarks and write similar ones for the other methods of the Repo module? aka: delete, preloads, get, one update....
  • Should we test code without touching db layers and use fixtures or something like that?
  • For which versions of ecto should we write benchmarks for?
@michalmuskala
Copy link
Contributor

For ideas of what benchmarks to actually implement, I referenced the RubyBench suite and the rails benchmarks for active record. We can't cover everything perfectly, but this should be a good starting point around looking for some ideas.

We should write benchmarks for the other repo functions, we don't need to replicate the benchmarks for the "read" functions, though - ultimately they all end up using the all function in the end. What we could check, though, is some complex query - this could highlight some performance issues in the query compiler and planner.

I think always hitting db there is the right approach - we want to test realistic situations and this keeps it as close to reality as it gets.

We should write the benchmarks for latest version of ecto. You might write them against the last stable version for now - I'm not 100% how good the documentation is for some of the current changes in master. Bringing it up to date to master shouldn't be a huge endeavour, though - there weren't many breaking changes.

@PragTob
Copy link
Member

PragTob commented Jun 12, 2018

@michalmuskala sure about always hitting the database? I agree lots of them should so we get the most integrated case but that's one more variable that might change and introduce variance. I obviously don't know the ecto design but I imagined that there's an encode part (create SQL query from what I'm given) and a decode part (created maps/structs from the db result) and I thought that more micro benchmarks for these could also be helpful?

@michalmuskala
Copy link
Contributor

I think you're right - they would be useful. On the other hand, I'm just afraid of reaching too far into the internals of ecto - I'm worried we'd have to update the benchmarks often when we change things.

@PragTob
Copy link
Member

PragTob commented Jun 12, 2018

@michalmuskala in my ideal world that has nothing to do with reality there is a nice high level function for each of these encapsulating it. At least there is a function for "you give me query, I give you SQL" (not sure if that is used internally as well?) and on the other end I don't think/hope the data structure that postgrex or postgres itself returns changes too much but this is me guessing at a project whose code I've barely looked at 🤔

But you're right, if there isn't such a thing and ecto changes as much on master as you imply it might not be the use of our time. 😁

@michalmuskala
Copy link
Contributor

We have Ecto.Adapters.SQL.to_sql/3 so that way would be fine - that's a good point. The reverse could also be partially achieved by using Repo.load/2 that takes "raw" data and puts it back into ecto structs.

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

No branches or pull requests

3 participants