-
Notifications
You must be signed in to change notification settings - Fork 41
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
Compare membership #13
base: master
Are you sure you want to change the base?
Conversation
Benchee 1.0 is compatible with elixir ~> 1.6
The logger is not being used directly in this project.
To keep the comparisons more focused. For reference, when the element is not in the list, MapSet.member?/2 is the fastest for all n. For small n, the difference is almost insignificant. But for large n, MapSet.member?/2 can be up to 1559.06x faster than the next fastest (Enum.member?/2) since the other methods are exhaustive.
def benchmark do | ||
Benchee.run( | ||
%{ | ||
"New MapSet + MapSet.member?" => fn input -> bench(Membership.NewMapSetMember, input) end, |
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.
Wasn't sure whether to keep this scenario. It ends up being the slowest for all inputs. Including may be useful to show that you lose all performance benefits when creating a new mapset that's infrequently used.
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.
I'm very in favor of keeping this. Sometimes when folks look at stuff like this they think "oh, MapSet is faster, so I'll use that!" but in the end the added runtime of the conversion ends up making their code slower overall.
That's the unfortunate consequence of microbenchmarks. They can be helpful, but you also need to look at the macro picture.
@@ -0,0 +1,64 @@ | |||
defmodule Membership.NewMapSetMember do | |||
def check_membership(range, list, _mapset) do | |||
find = Enum.random(range) |
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.
I'm fairly certain that the runtime of Enum.random/1
should be constant, but for the sake of making these comparisons even stronger, how would you feel about moving this call to a before_each
hook so the runtime of Enum.find/1
isn't included in the comparison?
def benchmark do | ||
Benchee.run( | ||
%{ | ||
"New MapSet + MapSet.member?" => fn input -> bench(Membership.NewMapSetMember, input) end, |
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.
I'm very in favor of keeping this. Sometimes when folks look at stuff like this they think "oh, MapSet is faster, so I'll use that!" but in the end the added runtime of the conversion ends up making their code slower overall.
That's the unfortunate consequence of microbenchmarks. They can be helpful, but you also need to look at the macro picture.
end | ||
|
||
defmodule Membership.EnumAny do | ||
def check_membership(range, list, _mapset) do |
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.
I'm not sure this is fair to include in this comparison since Enum.any?/2
is a much more flexible function than the others that we're comparing. It will almost always be slower since in the majority of use cases it's testing for more than just strict equality (i.e. comparing with Kernel.===/2
like all the other functions do).
Branched off of
update-elixir-erlang
, will rebase once that's been merged.Compares various methods for checking whether an element is a member of an enumerable.
I'll add the README section once the benchmark code gets a thumbs up.
I removed benchmarks that tested misses (i.e. element not a member) to keep the results more focused, but can revert 917386a if we think it's useful to include. Basically,
MapSet.member?/2
is way more performant for large n.Click to see full benchmark results