qpid-proton icon indicating copy to clipboard operation
qpid-proton copied to clipboard

PROTON-2095 Move away from SWIG to CFFI (rebased from #318)

Open jiridanek opened this issue 3 years ago • 2 comments
trafficstars

jiridanek avatar Jul 16 '22 16:07 jiridanek

@astitcher I've updated #318 to use current proton main and to run all python-tests. I want to keep that previous PR for future reference, so I am closing that one and opening a new PR.

So far, I am thinking that

  1. CFFI is certainly possible and (maybe with the exception of the tracer test) it can maintain the api that swig previously exposed to the python code
  2. Ownership of the cffi cdata values might be tricky in places. So far I never had problems with it, but maybe I was just lucky. The thing is that CFFI discourages relying on CPython specifics (ref counting) so values must be kept alive by something on the python side always referencing them. Trying out Pypy should flush out problems, I hope
  3. python-tests don't cover many data types for pn_atom_t; the type conversion in the PR is woefully incomplete but they still pass. some other ctest targets maybe test that
    • see https://cffi.readthedocs.io/en/latest/ref.html?highlight=memory#ffi-new-allocator about just calling malloc/free and about getting unzeroed memory for buffers
  4. The CFFI wrapper functions (in _cpython directory) can be probably mostly generated from the c type signatures, but I don't think that there is a cffi tool for that; it would need to be done by us; currently they are hand-written as needed (to pass tests)
  5. [x] I noticed asan job on GHA for this PR is not running, that is something to fix
  6. There is a script to autogenerate the c signatures for cffi from .h files, somewhere, I need to find it. Currently, I made edits by hand.

jiridanek avatar Jul 16 '22 16:07 jiridanek

At this point, assuming that the overall structure of the PR is acceptable, the next steps are I think

  • [ ] try it in pypy
  • [ ] do a perf test

So that we know if the goals were fulfilled. And next, autogenerate the wrapper code as already mentioned above. Both the header extraction, and the API wrapping, such as the buffer-size pattern in

def pn_ssl_get_cipher_name(ssl, size):
    buffer = ffi.new('char[]', size)
    rc = lib.pn_ssl_get_cipher_name(ssl, buffer, size)
    name = ffi.string(buffer, size)
    return rc, name

jiridanek avatar Jul 18 '22 08:07 jiridanek

Obsoleted by #386

astitcher avatar Jan 27 '23 18:01 astitcher