SRP updates

[lsmag]

Hi,

I'm working on some updates on your library. These are the things I'm working on right now:

• making the C extension optional (I talked about it on a PR)
• making the code py2/py3 compatible (I'll probably add tox to manage tests in each python version)

Also, why is the ctypes code optional? It is available in both py2 and py3 (at least in my Arch linux), is there some version or platform which doesn't come with this module? If ctypes is in fact optional, there's another option: compile _pysrp.py with Cython, if available, like mistune does, for example.

I'm yet to benchmark the Cython version against all others, but the advantage of Cython is that you can use just regular Python and still have a performance improvement, so maybe you can get rid of _ctsrp.py and maintain just _srp.c and _pysrp.py; less code duplication, less things to maintain. I'll update this thread later with the benchmarks.

[cocagne]

The reason for the ctypes version was for use on systems where compiling
the c-module is not possible but the openssl library exists. The primary
environment for which that is useful today is Pypy. If I recall correctly,
there's only a minimal performance difference between the native C module
and the ctypes version so if I were going to drop one, it'd probably be the
C module. It's worth discussing though. With respect to Cython, I thought
about that a couple years ago but ruled it out as it's not available
everywhere. I haven't looked at it again since though so if things have
changed, it might be worth reconsidering that stance.

Tom

On Fri, Jun 10, 2016 at 7:58 AM, lsmag notifications@github.com wrote:

Hi,

I'm working on some updates on your library. These are the things I'm
working on right now:

• making the C extension optional (I talked about it on a PR)
• making the code py2/py3 compatible (I'll probably add tox to manage
  tests in each python version)

Also, why is the ctypes code optional? It is available in both py2 and py3
(at least in my Arch linux), is there some version or platform which
doesn't come with this module? If ctypes is in fact optional, there's
another option: compile _pysrp.py with Cython, if available, like mistune
does https://github.com/lepture/mistune/blob/master/setup.py#L92, for
example.

I'm yet to benchmark the Cython version against all others, but the
advantage of Cython is that you can use just regular Python and still have
a performance improvement, so maybe you can get rid of _ctsrp.py and
maintain just _srp.c and _pysrp.py; less code duplication, less things to
maintain. I'll update this thread later with the benchmarks.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#18, or mute the thread
https://github.com/notifications/unsubscribe/ABrLfGSylRy17gSz-RPp0NDJgC3vI78Vks5qKXuBgaJpZM4IzCI9
.

[lsmag]

Mistune actually only uses Cython if it's available. The documentation recommends installing it before if performance is desired, like pip install cython mistune. But anyway, forget about it: I benchmarked it against the pure python code and the performance wasn't good enough to justify the addition. Here's the Cython analysis of the pure python code: cython-analysis.zip

About pypy: you're right, I forgot about it. Maybe it's worth checking cffi out and see if it solves the problem? I'll add tox first and include pypy too, and then maybe I'll look into it.

[lsmag]

Just a quick update: cryptography.io supports PyPy and Py3 and has a handy OpenSSL cffi binding:

>>> from cryptography.hazmat.bindings.openssl.binding import Binding
>>> Binding.lib.BN_new()
<cdata 'BIGNUM *' 0x11d78d0>

[cocagne]

Thanks Ismag. After rerunning the performance tests, I'm starting to think you have the right idea. The C-implementation is not providing a significant performance benefit over the ctypes module so it's probably best to just remove it entirely. With respect to cryptography.io, thanks for pointing that out, I had no idea. The transparent Py3 support is awfully attractive and looks like it would dramatically simplify simultaneous Py2/Py3 support. If you get this working, please send me a pull request. I'll gladly accept it. If not, I'll put it on my queue for investigating when I can once again find time to work through the backlog.

[lsmag]

I'll send a PR with the first patches I mentioned in this thread, then I'll look into porting the ctypes code to cryptography.io. I'll let you know when I start working on it, as there will be some structural refactoring and the code isn't mine, of course :)

[lsmag]

So, a heads up of what I've done last night on this branch:

• I added tox to run the test suite on some Python versions: pypy2, pypy3, py27, py24 and py35 (if they're not available in your system, tox will ignore them)
• I moved the tests to another folder and removed the benchmarks from them. My goal is to split the tests in smaller files (I'll discuss about that below) and add a benchmark script separately.

The test case right now is full of skipIf's, so the majority of the tests simply won't run in a Python version that won't support them, like the C implementation on Py3. They're probably going to disappear as I keep working in the source code, though.

There's a single bug I didn't solve yet on _ctsrp.py, which breaks py3's tests. I don't think it's worth it to fix that specific bug since I'm now moving to a new implementation using cryptography.io, which aims to replace the ctypes implementation completely.

Now, what I'm planning to do from now on:

• I'll add a new file, _newsrp.py, which uses cryptography.io. The name is temporary of course, as you decide later to keep or ditch some implementations.
• I fixed most py2/py3 bugs driven by the test results, so I'm yet to see if the changes I made impact the code in some unexpected way, so I'll make a thorough review when porting the code to cryptography.io.
• I'll benchmark that new implementation against all others on Py27

Finally, about multiple versions: in my experience, installing cryptography.io works on almost all platofrms. It's written and tested to work on all major Python implementations, but I've seen the build fail in some CentOS bases, mostly because of an obsolete OpenSSL. Just letting you know that.

So, in the end, what do you think? Should I proceed?