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

Improve __hash__ functions #921

Merged
merged 1 commit into from Jun 6, 2017
Merged

Improve __hash__ functions #921

merged 1 commit into from Jun 6, 2017

Conversation

@franciscouzo
Copy link
Contributor

@franciscouzo franciscouzo commented Mar 22, 2017

No description provided.

@coveralls
Copy link

@coveralls coveralls commented Mar 22, 2017

Coverage Status

Coverage decreased (-1.2%) to 73.077% when pulling d69ef77 on franciscouzo:master into 5061ee6 on paramiko:master.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Mar 22, 2017

Can you clue me in to how this is an improvement? Context-less PRs are no fun :) thanks!

@ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Mar 22, 2017

Less code and "interesting" logic (multiply by 37) in these implementations, while still incorporating all the same class member data into the hash. And I think tuple hash performance is about as good as you'll get by any other strategy.

(Different hash result than before, but that's fine because it just needs to be consistent within the process.)

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Mar 23, 2017

Less code is usually good, I just wish I understood why this "multiply by 37" behavior was added in the first place.

@sethmlarson
Copy link

@sethmlarson sethmlarson commented Mar 23, 2017

Good arbitrary prime number for combining hash values?

@sethmlarson
Copy link

@sethmlarson sethmlarson commented Mar 23, 2017

Also smallest prime greater than 32.

@ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Mar 23, 2017

Compare to the very simple "djb2" hash function http://www.cse.yorku.ca/~oz/hash.html

    unsigned long
    hash(unsigned char *str)
    {
        unsigned long hash = 5381;
        int c;

        while (c = *str++)
            hash = ((hash << 5) + hash) + c; /* hash * 33 + c */

        return hash;
    }

So you see, "hash * CONST_INT + next_input" is a common simple hash strategy (hash is expected to overflow).

@bitprophet bitprophet added this to the 2.2 milestone Mar 24, 2017
@bitprophet bitprophet merged commit d69ef77 into paramiko:master Jun 6, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
bitprophet added a commit that referenced this pull request Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants