Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Api mode cffi compile #230

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Api mode cffi compile #230

wants to merge 24 commits into from

Conversation

georgeharker
Copy link

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

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 changes to be accepted - see here

During installation CAIROCFFI_API_MODE=0 or CAIROCFFI_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 CAIROCFFI_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 CAIROCFFI_API_MODE =0). The user does not have to arrange for CAIROCFFI_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.

@georgeharker
Copy link
Author

The changes for xcffib are now in, I'd love to work with you on getting this to a similar state.

There are some reports that the new xcffib isn't playing nice with cairocffi which is perhaps to be expected as including the api mode in a abi mode cffi isn't supported. See tych0/xcffib#179

@georgeharker
Copy link
Author

@SimonSapin would you be able to take a look?

@SimonSapin
Copy link
Member

Sorry, I haven’t been involved for years. Try @liZe

@liZe
Copy link
Member

liZe commented Jan 12, 2025

Hi.

Thanks for your pull request.

The problem with supporting API mode is that it’s an endless source of possible bugs. We already had to handle many problems with the much simpler ABI mode, it’s been quieter for a while now that the library and its packaging are stable, we’re sorry but we don’t want to handle all the problems and answer all the issues that will happen after these changes.

Dropping a simple packaging system in favor of setuptools + setup.py, dropping wheels, adding code launched at install time that may break when Cairo is upgraded, possibly breaking compatibility with other libraries (as it just happened with xcffib)… It seems to be a lot if the goal is only to improve speed, isn’t it?

@georgeharker
Copy link
Author

Yes, it's for speed. But the difference can be very significant.

I understand the reticence to introduce any instability, however in the long run - in theory - api mode should be more not less stable than abi mode.

@georgeharker
Copy link
Author

FWIW I understand pangocffi was willing to look at this if Cairocffi can be updated.

@georgeharker
Copy link
Author

The relevant part of the cffi docs here https://cffi.readthedocs.io/en/stable/overview.html#abi-versus-api lists reasons (in addition to performance) to prefer API mode.

@georgeharker
Copy link
Author

Would you be willing to support a change that had this as be optional during install. Ie. revert functionally to current behavior but allow installing with api mode?

The motivation here is that in embedded contexts the overhead is far far less when using api mode - it should also be much more stable as it does what any c program would do linking to the Cairo libs. If there are concerns about default behavior changes I think those can be rectified. However right now it precludes fast operation of pangocffi or cairocffi. The issue with xcffib is due to inclusion of xcffib as a base starting point for the xcffib inclusions - I can check again but the proposed PR works with that change to make xcffib inclusions compatible. I can do the legwork to ensure that those changes work even when xcffib is installed in api mode.

@liZe
Copy link
Member

liZe commented Jan 15, 2025

The relevant part of the cffi docs here https://cffi.readthedocs.io/en/stable/overview.html#abi-versus-api lists reasons

I can’t remember how many times I’ve already read this chapter 😄. Yes, API mode is technically better than ABI.

Would you be willing to support a change that had this as be optional during install. Ie. revert functionally to current behavior but allow installing with api mode?

To be honest, it doesn’t change a lot for the points I mentioned above:

  • we’d have to drop a simple packaging system in favor of setuptools + setup.py,
  • we’d have to drop our universal wheels,
  • we’d add code launched at install time that may break when Cairo is upgraded,
  • we’d possibly break compatibility with other libraries, because mixing ABI and API is just a terrible way to get awful errors in the future

If you find a way to solve these problems, if you let users enable the API mode with the USE_CAIROCFFI_API_MODE_AND_NEVER_EVER_REPORT_A_BUG environment variable set at runtime, if you accept that we may revert these changes the first or second time the ABI mode is broken for an angry user because of that, then we may merge this PR.

I can check again but the proposed PR works with that change to make xcffib inclusions compatible. I can do the legwork to ensure that those changes work even when xcffib is installed in api mode.

I don’t doubt that you can work hard on this, but I really doubt that it’s possible to be sure that these changes are harmless. Even if you tried your best to propose a clean PR to xcffib, a user opened a bug just a few hours after the package has been uploaded. And xcffib is installed "only" ~1,000 times a day from PyPI, compared to ~45,000 for cairocffi.

However right now it precludes fast operation of pangocffi or cairocffi.

I launched CairoSVG’s tests with this PR, and I can’t see any significant performance differences between API and ABI modes. Do you have an idea (maybe benchmarks) about how important the speed improvements could be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants