-
Notifications
You must be signed in to change notification settings - Fork 134
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
Performance tests #190
Performance tests #190
Conversation
2f44419
to
0ed01a0
Compare
The last build (on Travis) on this branch on Ruby 2.5 looked like this:
Building #173 rebased on this branch yields this result:
This shows that Redis Streams are marginally slower to publish to than Sorted Sets (though this result may not be statistically significant), while they are currently clearly slower to subscribe to, though it doesn't seem to matter if trimming happens or not. I suspect the main issue is the overhead in executing XREAD vs the streaming of results from Redis PubSub. I'll see if I can run the benchmarks against the version which uses Streams + Redis PubSub to evaluate this possibility. |
@SamSaffron What do you think of these benchmarks for their own sake, regardless of the Streams implementation? |
Here are the results for Streams + Redis PubSub:
This appears to confirm that the overhead is in usage of |
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.
These changes look good to me, and I think it's definitely valuable to have performance benchmarks. I don't know that I would go so far as to running them in the default rake task, but I don't have strong feelings either way.
I included them in the default rake task for visibility. If they don’t get shoved in one’s face regularly, they’ll be very easy to forget about. |
Thanks I love having the perf tests but lets not have them as our default rake task. Feel free to merge the tests once you shift it to a dedicated task and add info to the readme. I think calling this out nicely in the readme is a way of showing this off. We can also look at adding this to: https://rubybench.org/ which would be super nice. Regarding redis streams, I kind of feel it does not have the numbers to justify large changes to message bus at the moment, if it was 30% faster or somehow more reliable then I would be far more behind it but it is not faster so I am not sure what this adds at the moment outside of code churn that is very risky. I think it is far better to focus on nicer diagnostics, seeing if we can squeeze more perf out of our current implementations a nice public web page and so on. Stuff like service workers are also interesting cause a bunch of tabs can be multiplexed across a single connection to the db, but I just don't feel the amount of boat rocking is justified for a slower provider. |
also curious if @antirez has any feedback here. regardless I think it is pretty amazing how redis is so close performance wise to a pure memory provider. |
One less place to keep a list of backends. Also moves non-spec checks out of `rake spec` and into the `rake` default task, as well as renaming `spec_BACKEND` to `spec:BACKEND` (eg `rake spec:memory`) and giving tasks descriptions so they show up in `rake -T`.
* Strictly publishing only, without subscribing * Publishing and subscribing with max backlog length large enough to not have to trim backlogs during the benchmark * Publishing and subscribing backlogs with approximately 10% of the capacity of the benchmark
0ed01a0
to
255c50b
Compare
Executing this on my laptop I get this: