-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 benchmarks for adapters #1511
Conversation
@ianks Thanks for this! Very awesome. 👍 Do you have a particular philosophy about writing benchmark code for an app, on where to put it, add to it, and when and how to run it? I have a PR to add a benchmark for performance regressions that I've kind of stalled on... If that interests you, I'd love your input, especially since I'd want it to co-exist with this. |
serializer = PostWithCustomKeysSerializer.new(resource) | ||
|
||
Benchmark.ips do |x| | ||
ActiveModel::Serializer::Adapter.adapter_map.each do |adapter_name, adapter_class| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh fun
Hey @bf4 I checked out your PR. It is really good. I don't really have a 'philosophy' on benchmarking per se; other than it is something one should strive to do when coding. I think the approach you take in your PR is very important, as it documents perf regressions from changes in the code automatically. It is almost like an integration test suite for perf. The goal of this PR, is to simply have a place where people can write benchmarks for small sub-sets of the code they are working on. For example, I wanted to see if I could increase the perf of the JSONAPI adapter, and benchmarking only the adapters makes sense in this case. Almost like a unit-test of perf. TL;DR -- Your approach works better for documenting perf and catching regressions. The approach of this PR is more geared towards creating a place for contributors to write benchmarks for specific pieces of code. I hope that makes sense? Ha 😄 |
@ianks you've inspired me.. it's better now.. I focused on just the benchmark before worrying about regression testing across revisions.. and now there's a structure for this PR.. test/dummy/bm_adapter.rb (or should it be test/benchmark/... not sure 'cause I want the dummy app easily accessible). Other thought is to update the https://github.com/rails-api/active-model-serializers-example and use that... |
Just a quick thought on this: shouldn't there be a benchmark using the |
@groyoh so many things should be... :) |
Benchmarking should use and iterate on the structure introduced by #1393. Closing this, but I'd really like to see the work done in this PR done in a new PR with the new benchmarking structure. |
I was curious how the adapters stacked up against each other performance wise. While doing this, I realized it would be useful to maintain some simple benchmark tests that can be used during development. This one is a simple benchmark which tests the performance of each adapter. If you are interested in the results from my machine: here they are.