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

Drop SimpleHash (Was: SimpleHash and other hashes) #2331

Closed
asterite opened this issue Mar 20, 2016 · 4 comments · Fixed by #3562
Closed

Drop SimpleHash (Was: SimpleHash and other hashes) #2331

asterite opened this issue Mar 20, 2016 · 4 comments · Fixed by #3562

Comments

@asterite
Copy link
Member

In the compiler we used to use SimpleHash (a hash built on top of an array of pairs) for some paths where the hash would end up with a few elements. However, we don't use that anymore because the performance difference was negligible.

The problem is, it's kind of public API now, and others want to use it, like here, but I'm not sure I want to stick with that type in the standard library. Or, if we stick with it, it's better if we change its name and clearly state its use case.

The questions are:

  • Should we keep it in the std? If so, what name do we use? Since it's a Hash backed by an array, maybe ArrayHash would be an appropriate name, although it sounds confusing.
  • Should we also add a class similar to Hash but without maintaining order? Right now Hash has a doubly linked list that allows maintaining insertion sorter, but this needs more allocated memory to work, so not having this functionality will speed up things in some cases. If so, what name should we use? Or maybe we can swap things, and maybe Hash dumber and add a LinkedHash.
@ysbaddaden
Copy link
Contributor

Sorry, hit close button on smartphone...

@ysbaddaden ysbaddaden reopened this Mar 20, 2016
@ysbaddaden
Copy link
Contributor

At first we should drop SimpleHash, especially if it's inefficient and we don't want to maintain it.

I'm all in favor for a fast Hash table by default (there are a bunch of papers on the subject), and maybe an OrderedHash that could be slower/use more memory but come handy at times.

@ysbaddaden
Copy link
Contributor

BTW on the topic of hashes: http://preshing.com/20160314/leapfrog-probing/

@ozra
Copy link
Contributor

ozra commented Apr 12, 2016

Wouldn't it be best to have Map and OrderedMap as recommended types? Specific implementations could be chosen amongst via an optional size-ballpark-figure passed to new, or it defaults to a generically fast one. Ofcourse the implementations could be used directly if wanted: HashMap, LeapFrogMap, whatever...

Of course this would cause every call to methods on an instance created via Map.new to be branched I guess (HashMap|LinearMap|LeapFrogMap|MagicMushroomMap|Whatever)

@jhass jhass changed the title SimpleHash and other hashes Drop SimpleHash (Was: SimpleHash and other hashes) May 27, 2016
asterite pushed a commit that referenced this issue Nov 20, 2016
firejox pushed a commit to firejox/crystal that referenced this issue Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants