Api mode cffi compile

[georgeharker]

Cffi code is much faster when compiled with a c compiler (API mode) rather than using libfffi (ABI mode) - however this requires a compiler to be installed .

This change allows an install to be made using

PANGOCFFI_API_MODE=1 CAIROCFFI_API_MODE=1 XCFFIB_API_MODE=1 pip3 install cairocffi

Various other projects (pangocffi and pangocairocffi) can then benefit from similar changes.

This change requires xcffib and cairocffi changes to be accepted - see here and here

During installation PANGOCFFI_API_MODE=0 or PANGOCFFI_API_MODE not set defaults to the previous ABI mode install which incurs lever overhead on load, runs the ffi_build as before and will dynamically translate arguments to C using the general libffi.

During installation PANGOCFFI_API_MODE=1 compiles a shared library / C extension which dynamically depends on xcd.

At runtime if the shared library / C extension is present it will be used (unless PANGOCFFI_API_MODE =0). The user does not have to arrange for PANGOCFFI_API_MODE =1 to be set. If the extension is not present, the old behavior is used.

To make this work I added the relevant FFI calls / setup.py calls to have the C extension be built.

I don't suggest this be default, but API mode is a potential substantial performance improvement. It also allows full speed threading as the C side unlocks the GIL.

[leifgehrmann]

Hi George,

Given that this change requires changes to both cairocffi and xcffib to add this mode (which may include changes to the final implementation pattern), I'll hold off from doing a full review. Hope that's understandable.

In the meantime, here's some initial feedback:

Tests

The tests are failing because the enum was removed from pangocffi/c_definitions_pango.txt. It appears the constants were moved to the bottom of pangocffi/__init.py__. This causes tests in that use this constant (via pangocffi/attribute.py) to fail. I'd recommend moving the constants to pangocffi/enums.py and update pangocffi/attribute.py to reference them, but I'm unsure why they were removed from the definitions file in the first place. Was this intended to fix a compilation error that is exists only in API mode?

Linting

Linting is failing because of max-line-lengths and whitespace in pangocffi/__init.py__. Thankfully nothing too difficult to resolve.

Version bump

Changes to VERSION will be done by myself after this change has been merged in, so it is not needed in this PR.

Requiring cairocffi as a dependency

Does pangocffi have to have cairocffi as a dependency in setup.cfg/setup.py?

I understand pangocairocffi having cairocffi as a dependency, but in theory pangocffi is able to stand independently from cairocffi. This is because Xft could be used as the renderer, rather than Cairo. I'm unaware of anyone using this package with Xft, but my point is there's a possibility.*

So I wonder if it can be removed. But correct me if I'm wrong as I'm unfamiliar with the setup that API mode requires.

*I have wondered if it would make sense to merge pangocffi with pangocairocffi, as I'm guessing the primary use-case of Pango will probably be with Cairo. That would simplify a lot of things, both for maintenance and for usability. So I'm not against adding cairocffi as a dependency, I'm just explaining why pangocffi and pangocairocffi co-exist as two packages.

[codecov]

Codecov Report

Attention: Patch coverage is 80.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 98.75%. Comparing base (5c6bcac) to head (1751247).

Files with missing lines	Patch %	Lines
pangocffi/__init__.py	74.19%	8 Missing ⚠️
pangocffi/ffi_instance_builder.py	71.42%	2 Missing ⚠️
pangocffi/ffi_build.py	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
- Coverage   99.36%   98.75%   -0.61%     
==========================================
  Files          23       23              
  Lines        1099     1125      +26     
==========================================
+ Hits         1092     1111      +19     
- Misses          7       14       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[georgeharker]

Thanks Lief,

Working on getting cairocffi and xcffib sorted.

The tests are failing because the enum was removed from pangocffi/c_definitions_pango.txt. It appears the constants were moved to the bottom of pangocffi/__init.py__. This causes tests in that use this constant (via pangocffi/attribute.py) to fail. I'd recommend moving the constants to pangocffi/enums.py and update pangocffi/attribute.py to reference them, but I'm unsure why they were removed from the definitions file in the first place. Was this intended to fix a compilation error that is exists only in API mode?

Done!

Linting

Linting is failing because of max-line-lengths and whitespace in pangocffi/__init.py__. Thankfully nothing too difficult to resolve.

Fixed.

Version bump

Changes to VERSION will be done by myself after this change has been merged in, so it is not needed in this PR.

Requiring cairocffi as a dependency

Does pangocffi have to have cairocffi as a dependency in setup.cfg/setup.py?

I don't think it does have to, removed for pangocffi.

I understand pangocairocffi having cairocffi as a dependency, but in theory pangocffi is able to stand independently from cairocffi. This is because Xft could be used as the renderer, rather than Cairo. I'm unaware of anyone using this package with Xft, but my point is there's a possibility.*

So I wonder if it can be removed. But correct me if I'm wrong as I'm unfamiliar with the setup that API mode requires.

*I have wondered if it would make sense to merge pangocffi with pangocairocffi, as I'm guessing the primary use-case of Pango will probably be with Cairo. That would simplify a lot of things, both for maintenance and for usability. So I'm not against adding cairocffi as a dependency, I'm just explaining why pangocffi and pangocairocffi co-exist as two packages.

Thanks for the context. I'll try and tidy up pangocairocffi changes next.