crypto: speed up secp256k1/field.py #160

teknico · 2020-04-08T21:33:42Z

This speeds up most of crypto/secp256k1/field.py by ~100x (!) using Cython.

The file crypto/secp256k1/field.pxd contains type definitions that are used by Cython to generate field.c, a 23K lines C extension that in turn gets compiled to a dynamic library in a platform-dependent format (.so on Unix-like, .dll on Windows).

The dynamic library is placed in the same directory and is loaded automatically by the Python interpreter at execution time.

The command to generate the C extension and build the dynamic library in development is:

cythonize -X language_level=3 -a -i ./decred/crypto/secp256k1/field.py

Poetry has undocumented support for building C extensions, so that the poetry build command builds the library and includes it into the created, platform-dependent, installable wheel archive.

Cython has been added to the development dependencies of the decred library.

Only tested on Linux, let me know if it works correctly on Windows and macOs.

Based on #158.

teknico · 2020-04-08T21:36:24Z

decred/decred/crypto/secp256k1/field.py

+# n[0] = 977 * 16 = 15632
+# n[1] = 64 * 16 = 1024
+# That is to say (2^26 * 1024) + 15632 = 68719492368
+primePartBy16 = 68719492368


Having this as a named value is needed by Cython.

teknico · 2020-04-08T21:38:13Z

decred/decred/crypto/secp256k1/field.py

-        b = ByteArray(0, length=32)
+        b = bytearray(32)
        self.putBytes(b)


Not passing a ByteArray allows optimizing the putBytes method.

decred/build.py

JoeGruffins

omg. Tests from 21.40s to 4.33s

Will try this on windows asap.

decred/decred/crypto/secp256k1/field.py

decred/build.py

teknico · 2020-04-10T13:56:03Z

decred/decred/crypto/secp256k1/field.py

-            f = FieldVal.fromHex("0abc").add(1)
+            f = FieldVal.fromHex("abc").add(1)
        so that:
-            f = 0x0abc + 1
+            f = 0xabc + 1


This is to show that a missing hex zero is not a problem.

buck54321

This is an amazing improvement. I'm learning cython on the fly, and it's been a while since I've thought in C, so bear with me while I take a couple passes at this. A couple of questions for now.

Should we be checking in our .c files?
There are some official recommendations for distributing though PyPI. Does Poetry take care of all of this?

decred/build.py

decred/decred/crypto/secp256k1/field.pxd

teknico · 2020-04-10T18:40:46Z

This is an amazing improvement.

Thank you.

Should we be checking in our .c files?

I don't think so. From the docs:

It is strongly recommended that you distribute the generated .c files as well as your Cython sources, so that users can install your module without needing to have Cython available.

So the user is supposed to have a C compiler but somehow not Cython, that is listed in the dev dependencies? It doesn't seem likely.

It is also recommended that Cython compilation not be enabled by default in the version you distribute. Even if the user has Cython installed, he/she probably doesn’t want to use it just to install your module.

Huh? Why should they not want to use it?

Also, the installed version may not be the same one you used, and may not compile your sources correctly.

We have dependency check and resolution now, thanks to Poetry. This line of reasoning is outdated.

Then they go on detailing lots of unneeded complexity. No thanks.

There are some official recommendations for distributing though PyPI. Does Poetry take care of all of this?

Yes, it does. Just run poetry build and you have your platform-dependent wheel archive, ready to be uploaded by poetry publish. That is all.

…view.

buck54321 · 2020-04-20T16:03:04Z

I tried to poetry install on a clean install of Windows 10, and I got

[EnvCommandError]
Command ['C:\\Users\\buck5\\AppData\\Local\\pypoetry\\Cache\\virtualenvs\\decred-81_y65CL-py3.8\\Scripts\\pip.exe', 'install', '-e', 'C:\\Users\\buck5\\programs\\tinydecred-secp256k1-field-speedup\\tinydecred-secp256k1-field-speedup\\decred'] errored with the following return code 1, and output:
Obtaining file:///C:/Users/buck5/programs/tinydecred-secp256k1-field-speedup/tinydecred-secp256k1-field-speedup/decred
Requirement already satisfied: appdirs<2.0.0,>=1.4.3 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from decred==0.0.1) (1.4.3)
Requirement already satisfied: base58<3.0.0,>=2.0.0 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from decred==0.0.1) (2.0.0)
Requirement already satisfied: blake256<0.2.0,>=0.1.1 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from decred==0.0.1) (0.1.1)
Requirement already satisfied: pynacl<2.0.0,>=1.3.0 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from decred==0.0.1) (1.3.0)
Requirement already satisfied: websocket_client<0.58.0,>=0.57.0 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from decred==0.0.1) (0.57.0)
Requirement already satisfied: six in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from pynacl<2.0.0,>=1.3.0->decred==0.0.1) (1.14.0)
Requirement already satisfied: cffi>=1.4.1 in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from pynacl<2.0.0,>=1.3.0->decred==0.0.1) (1.14.0)
Requirement already satisfied: pycparser in c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\lib\site-packages (from cffi>=1.4.1->pynacl<2.0.0,>=1.3.0->decred==0.0.1) (2.20)
Installing collected packages: decred
  Running setup.py develop for decred
    ERROR: Command errored out with exit status 1:
     command: 'c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\scripts\python.exe' -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\buck5\\programs\\tinydecred-secp256k1-field-speedup\\tinydecred-secp256k1-field-speedup\\decred\\setup.py'"'"'; __file__='"'"'C:\\Users\\buck5\\programs\\tinydecred-secp256k1-field-speedup\\tinydecred-secp256k1-field-speedup\\decred\\setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps
         cwd: C:\Users\buck5\programs\tinydecred-secp256k1-field-speedup\tinydecred-secp256k1-field-speedup\decred\
    Complete output (11 lines):
    running develop
    running egg_info
    writing decred.egg-info\PKG-INFO
    writing dependency_links to decred.egg-info\dependency_links.txt
    writing requirements to decred.egg-info\requires.txt
    writing top-level names to decred.egg-info\top_level.txt
    reading manifest file 'decred.egg-info\SOURCES.txt'
    writing manifest file 'decred.egg-info\SOURCES.txt'
    running build_ext
    building 'decred.crypto.secp256k1.field' extension
    error: Microsoft Visual C++ 14.0 is required. Get it with "Microsoft Visual C++ Build Tools": https://visualstudio.microsoft.com/downloads/
    ----------------------------------------
ERROR: Command errored out with exit status 1: 'c:\users\buck5\appdata\local\pypoetry\cache\virtualenvs\decred-81_y65cl-py3.8\scripts\python.exe' -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\buck5\\programs\\tinydecred-secp256k1-field-speedup\\tinydecred-secp256k1-field-speedup\\decred\\setup.py'"'"'; __file__='"'"'C:\\Users\\buck5\\programs\\tinydecred-secp256k1-field-speedup\\tinydecred-secp256k1-field-speedup\\decred\\setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps Check the logs for full command output.
WARNING: You are using pip version 19.2.3, however version 20.0.2 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.

If there is no compiler, it must fail gracefully and fall back to pure-Python.

teknico · 2020-04-20T21:29:45Z

If there is no compiler, it must fail gracefully and fall back to pure-Python.

Agreed. This needs Poetry support though, so I created an issue in their project.

buck54321 · 2020-04-20T22:57:15Z

Here's a build.py that works.

https://github.com/buck54321/tinydecred/blob/teknico/decred/build.py

teknico · 2020-04-20T23:13:18Z

Here's a build.py that works.

That's swell! Thanks, putting it in.

teknico requested review from buck54321 and JoeGruffins April 8, 2020 21:33

teknico commented Apr 8, 2020

View reviewed changes

teknico commented Apr 9, 2020

View reviewed changes

decred/build.py Outdated Show resolved Hide resolved

JoeGruffins reviewed Apr 10, 2020

View reviewed changes

teknico commented Apr 10, 2020

View reviewed changes

decred/build.py Outdated Show resolved Hide resolved

teknico commented Apr 10, 2020

View reviewed changes

teknico requested a review from JoeGruffins April 10, 2020 14:04

buck54321 reviewed Apr 10, 2020

View reviewed changes

decred/build.py Outdated Show resolved Hide resolved

decred/build.py Outdated Show resolved Hide resolved

decred/decred/crypto/secp256k1/field.pxd Outdated Show resolved Hide resolved

decred/decred/crypto/secp256k1/field.pxd Outdated Show resolved Hide resolved

teknico mentioned this pull request Apr 15, 2020

crypto.secp256k1.curve: clean up code and tests #165

Merged

teknico added 9 commits April 17, 2020 10:28

Show print output in database benchmark tests, add the pudb debugger.

584404f

Changes per review.

10e63ca

Complete docstrings and comments in test_field.py

b11cdbe

Typos and missing preconditions.

07e4f69

Speed up crypto/secp256k1/field.py by 100x (!) using Cython.

b2c1ef4

Changes per review.

77a0b38

Oops, missed several review comments.

13eac03

Fill in the build.py docstring, adjust data types in field.pxd per re…

263fbd4

…view.

Update a secp256k1 mention in the decred package's README.md .

f69a1b7

teknico mentioned this pull request Apr 20, 2020

Allow optional compilation of C extensions python-poetry/poetry#2330

Closed

2 tasks

Stop the build from failing when a C compiler cannot be found.

62326dc

teknico requested a review from buck54321 April 20, 2020 23:24

Intercept any error from Cython.

2ae9258

buck54321 approved these changes Apr 22, 2020

View reviewed changes

buck54321 merged commit 59c700a into decred:master Apr 22, 2020

teknico deleted the secp256k1-field-speedup branch April 22, 2020 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto: speed up secp256k1/field.py #160

crypto: speed up secp256k1/field.py #160

teknico commented Apr 8, 2020 •

edited

Loading

teknico Apr 8, 2020 •

edited

Loading

teknico Apr 8, 2020

JoeGruffins left a comment

teknico Apr 10, 2020

buck54321 left a comment

teknico commented Apr 10, 2020

buck54321 commented Apr 20, 2020

teknico commented Apr 20, 2020

buck54321 commented Apr 20, 2020

teknico commented Apr 20, 2020

crypto: speed up secp256k1/field.py #160

crypto: speed up secp256k1/field.py #160

Conversation

teknico commented Apr 8, 2020 • edited Loading

teknico Apr 8, 2020 • edited Loading

Choose a reason for hiding this comment

teknico Apr 8, 2020

Choose a reason for hiding this comment

JoeGruffins left a comment

Choose a reason for hiding this comment

teknico Apr 10, 2020

Choose a reason for hiding this comment

buck54321 left a comment

Choose a reason for hiding this comment

teknico commented Apr 10, 2020

buck54321 commented Apr 20, 2020

teknico commented Apr 20, 2020

buck54321 commented Apr 20, 2020

teknico commented Apr 20, 2020

teknico commented Apr 8, 2020 •

edited

Loading

teknico Apr 8, 2020 •

edited

Loading