to_bytes() adds significant overhead to keccak()

[gsalgado]

That function seems to add a big overhead to keccak(), and given its performance-critical nature I think we should consider dropping it (making keccak() accept only bytes). Here are some numbers showing the overhead it adds when compared to running the backend implementation directly:

$ python -m timeit -n 100000 -v -s "import os; from eth_hash.utils import auto_choose_backend; backend = auto_choose_backend()" "backend.keccak256(os.urandom(32))"
raw times: 1.81 1.82 1.85
100000 loops, best of 3: 18.1 usec per loop

$ python -m timeit -n 100000 -v -s "import os; from eth_utils import keccak" "keccak(os.urandom(32))"
raw times: 2.51 2.45 2.44
100000 loops, best of 3: 24.4 usec per loop

[pipermerriam]

@gsalgado I believe our intent with eth-hash was to provide a fast low level API that accepts only bytes, and for eth_utils.keccak to be the slightly higher level API. Performance focused code should directly use eth-hash, otherwise eth_utils provides a slightly friendlier API.

However, I can get onboard with dropping support for text/hexstr since I'd bet we have very few call sites which use non-bytes inputs for keccak.

[carver]

I get why trinity would want to use eth-hash's keccak, but I don't understand the desire to remove the convenience one from eth-utils.

[gsalgado]

Mainly because doing it implicitly on every call adds significant overhead to a performance-critical piece of code, but I'd also argue that there's no point in having a keccak func in two different eth- packages

[pipermerriam]

I think I'm now 👎 on removing keccak from eth-utils. They may be a minority use case, but having a keccak available which can take encoded hex and utf8 strings is useful and use cases which require performance can directly use eth-hash.

[carver]

Maybe it would help to add a note about performance in the eth-utils docs, and link to eth-hash's keccak?