remove redundant check in unicode_hash(PyObject *self)

[animalize]

BPO	35636
Nosy	@scoder, @serhiy-storchaka, @animalize
PRs	bpo-35636: remove redundant check in unicode_hash(PyObject *self) #11402

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-01-05.19:14:26.587>
created_at = <Date 2019-01-02.07:20:57.297>
labels = ['interpreter-core', '3.8', 'performance']
title = 'remove redundant check in unicode_hash(PyObject *self)'
updated_at = <Date 2019-01-05.19:14:26.586>
user = 'https://github.com/animalize'

bugs.python.org fields:

activity = <Date 2019-01-05.19:14:26.586>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2019-01-05.19:14:26.587>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2019-01-02.07:20:57.297>
creator = 'malin'
dependencies = []
files = []
hgrepos = []
issue_num = 35636
keywords = ['patch', 'patch', 'patch']
message_count = 11.0
messages = ['332857', '332858', '332861', '332864', '332866', '332908', '332909', '332910', '332911', '332916', '333068']
nosy_count = 3.0
nosy_names = ['scoder', 'serhiy.storchaka', 'malin']
pr_nums = ['11402']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue35636'
versions = ['Python 3.8']

[animalize]

Please see the PR

[animalize]

Every non-empty str will be checked twice at present.

[animalize]

This redundant exists since Python 3.4 or earlier.

[scoder]

Unlikely to get changed in Py3.4/5 anymore, since this is not even a bug fix. I wouldn't even fight for backporting, although 3.7 seems ok for it.

I agree that this code duplication is worth removing. I don't consider hashing the empty string important enough for leaving it in, especially because the net performance effect can at most be zero, if not negative, for the normal case of non-empty strings.

[serhiy-storchaka]

New changeset a1d1425 by Serhiy Storchaka (animalize) in branch 'master':
bpo-35636: Remove redundant check in unicode_hash(). (GH-11402)
a1d1425

[animalize]

Thanks for review.

Don't know why bytes and str generates the same hash value for ASCII sequence.
>>> hash('abc') == hash(b'abc')
True

This may brings some hash collisions, does it affect performance slightly?

[scoder]

why bytes and str generates the same hash value for ASCII sequence

Probably mostly for historical Py2 reasons. These days, both are somewhat unlikely to appear in the same dict. But still, I'd advise against changing the hash function without a very good reason. You never know how much code relies on it in one way or another.