close
The Wayback Machine - https://web.archive.org/web/20221223125620/https://github.com/python/cpython/pull/5023
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

bpo-32433: Optimized HMAC digest #5023

Merged
merged 1 commit into from Jan 27, 2018
Merged

bpo-32433: Optimized HMAC digest #5023

merged 1 commit into from Jan 27, 2018

Conversation

tiran
Copy link
Member

@tiran tiran commented Dec 27, 2017

The hmac module now has hmac.digest(), which provides an optimized HMAC
digest for short messages. hmac.digest() is up to three times faster
than hmac.HMAC().digest().

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue32433

@tiran tiran added the performance Performance or resource usage label Dec 27, 2017
@tiran tiran requested a review from gpshead Dec 27, 2017
Return digest of *msg* for given secret *key* and *digestmod*. The
function is equivalent to ``HMAC(key, msg, digestmod).digest()``, but
uses an optimized C or inline implementation, which is faster for messages
that fit into memory. The parameters *key*, *msg*, and *digestmod* have
Copy link
Member

@gpshead gpshead Dec 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize digestmod matches the parameter name in hmac.new, but it is an antiquated name from the days when the only two hash functions were top level modules. how about just calling it digest on the new API?

Are there any hash function modules with a new method left anywhere that anyone actually uses? Why continue to support that old idiom in new APIs?

Copy link
Member Author

@tiran tiran Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though about it but then decided to keep the new function compatible to the old PEP.

Since you also dislike the old name, let's use digest in the new function.

hmac.digest(key, data, digestmod='md5'),
binascii.unhexlify(digest)
)
orig = hmac._openssl_md_meths
Copy link
Member

@gpshead gpshead Dec 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from unittest import mock
...
with mock.patch('hmac._openssl_md_meths', {}):
  ...

@tiran tiran force-pushed the fast_hmac branch 2 times, most recently from 0100b1c to 7fd218c Compare Jan 14, 2018
@tiran
Copy link
Member Author

tiran commented Jan 26, 2018

@gpshead Are you ok with the new code? I'd like to get the feature into 3.7.

Copy link
Member

@gpshead gpshead left a comment

a couple comments, you can address those in this PR or in follow up minor commits as you see fit.



def digest(key, msg, digest):
"""Fast inline implementation of HMAC
Copy link
Member

@gpshead gpshead Jan 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe the parameters so that interactive users doing help(hmac.digest) see something useful.

@@ -0,0 +1,2 @@
The hmac module now has hmac.digest(), which provides an optimized HMAC
digest for short messages.
Copy link
Member

@gpshead gpshead Jan 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the "for short messages" part true? does anything care how long the message is? I'd just trim those words from the sentence. :)

Copy link
Member Author

@tiran tiran Jan 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"short" for some arbitrary measurement. After all the message has to fit into memory. Anything up to 64kB should be fine. :p

The hmac module now has hmac.digest(), which provides an optimized HMAC
digest for short messages. hmac.digest() is up to three times faster
than hmac.HMAC().digest().

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran merged commit 2f050c7 into python:master Jan 27, 2018
@bedevere-bot
Copy link

bedevere-bot commented Jan 27, 2018

@tiran: Please replace # with GH- in the commit message next time. Thanks!

@tiran tiran deleted the fast_hmac branch Jan 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants