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

Try a MapSet implementation #3242

Closed
josevalim opened this issue Apr 5, 2015 · 10 comments · Fixed by #3258
Closed

Try a MapSet implementation #3242

josevalim opened this issue Apr 5, 2015 · 10 comments · Fixed by #3258

Comments

@josevalim
Copy link
Member

With Erlang 18 coming out, I am starting to believe that the fastest way to implement a Set would actually be by using a map behind the scenes. After all, a Set is a map where having a given key means the entry exists in the set.

So here is what I have in mind. We create a new MapSet struct, that implements the Set behaviour. MapSet will be defined as follows:

defstruct set: %{}

And all functions would work on top of this set but return a %MapSet{...} struct. Once implemented, we could benchmark and see how it fares against the current HashSet. Note Map is faster than a HashDict on all operations on Erlang 18 (here is some sample benchmarking) although we may be slower in sets due to the %MapSet{} struct indirection. We need the indirection however if we want it to work as a struct (and have protocols and what not).

@josevalim
Copy link
Member Author

@lexmag interested in experimenting with this one?

@lexmag
Copy link
Member

lexmag commented Apr 5, 2015

@josevalim sure, would love to try.

@ericmj
Copy link
Member

ericmj commented Apr 6, 2015

We could consider cheating and have defstruct [] with values directly on the struct if there is a performance benefit.

@lexmag
Copy link
Member

lexmag commented Apr 6, 2015

@ericmj in that case we have to deal with :__struct__ entry as with a special case.

@ericmj
Copy link
Member

ericmj commented Apr 6, 2015

Yeah, may or may not be worth it.

@josevalim
Copy link
Member Author

Yeah, i didn't figure out a way to handle :__struct__ that's why i didn't propose it. :(

@lexmag
Copy link
Member

lexmag commented Apr 7, 2015

I've made initial implementation of MapSet lexmag@a26cd96 and run benchmarks for several functions https://gist.github.com/lexmag/32977ce8fd7cb44ddefa so far.

@josevalim
Copy link
Member Author

If I read them correctly, the results are really, really good. Please do send a pull request. :)

@josevalim
Copy link
Member Author

And before I forget, thank you for doing this! ❤️ 💚 💙 💛 💜

@lexmag
Copy link
Member

lexmag commented Apr 8, 2015

My pleasure. Will send WIP PR soonish; the results are really good, but as you might see I've done the tests not for all the functions (only BIF-based), most interesting ones are left. 😄

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

Successfully merging a pull request may close this issue.

3 participants