Skip to content

Improve performance with cffi #12

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

Merged

Conversation

MaxGyver83
Copy link
Contributor

Out of curiosity, I have analyzed the performance of 8vim_keyboard_layout_calculator and tested how much faster this script can be when the bottleneck functions are rewritten in C and called via python-cffi.

This way the optimizer runs more than double as fast. See my article Speed up Python with CFFI for details.

I don't know if it makes sense merging this. Updating the C code when the Python code changes is additional maintenance work. No problem for me if you decline this pull request. At least, interested users might find this PR and apply the changes locally.

@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 15, 2023

Thank you for the PR!
I haven't been able to try it out and test it yet, but I'll still address a few general points.

I don't know if it makes sense merging this. Updating the C code when the Python code changes is additional maintenance work.

As I'm no longer trying to actively extend this optimizer, I don't mind some additional maintenance work (which won't affect me too much).
I think I'm fine with merging C-code if it's well documented (I've never coded in C before) and I first create a release to have a pre-C timestamp.

  1. Just out of curiosity: Do you know whether the same thing would be possible using Rust? That language would be way more maintainable to me.
  2. Over the course of the day I'll leave some comments on the code on things I'm wondering about when first reading the code. Feel free to push back on any suggestions you disagree with.

@MaxGyver83
Copy link
Contributor Author

1. Just out of curiosity: Do you know whether the same thing would be possible using Rust? That language would be way more maintainable to me.

I think this is possible and it would look almost the same on the Python side. I have never tried but I found this example: Calling Rust from Python/PyPy using CFFI

2. Over the course of the day I'll leave some comments on the code on things I'm wondering about when first reading the code. Feel free to push back on any suggestions you disagree with.

Thanks for your time and your comments!

@Glitchy-Tozier
Copy link
Owner

I think this is possible and it would look almost the same on the Python side. I have never tried but I found this example: Calling Rust from Python/PyPy using CFFI

In case you are able to change, it would be neat if we could use Rust.

If you don't feel experienced enough or figure it would take too long, let's leave it using C-code. Unfortunately, I'm too overwhelmed by university at the moment to delve into this issue myself.

@Glitchy-Tozier
Copy link
Owner

By the way, it would feel even more fun to read your article if you mentioned the time it takes for the script to run using python3. "Very long → 13min" doesn't give (me) the same feeling of accomplishment as "XXmin → 13min".
Screenshot_20230512-233607_Firefox Klar.jpg

@MaxGyver83
Copy link
Contributor Author

By the way, it would feel even more fun to read your article if you mentioned the time it takes for the script to run using python3. "Very long → 13min" doesn't give (me) the same feeling of accomplishment as "XXmin → 13min".

I'll add a duration for python3 when I get one. I tried twice but canceled after some time. I wasn't sure if it was still running at all. Maybe my laptop ran out of memory!? I'll try again.

@MaxGyver83
Copy link
Contributor Author

I think this is possible and it would look almost the same on the Python side. I have never tried but I found this example: Calling Rust from Python/PyPy using CFFI

In case you are able to change, it would be neat if we could use Rust.

If you don't feel experienced enough or figure it would take too long, let's leave it using C-code. Unfortunately, I'm too overwhelmed by university at the moment to delve into this issue myself.

My Rust knowledge is also very basic. I'm interested in this language, but it's not at top of the list of things I want to do when I have some free time. :-)

@Glitchy-Tozier
Copy link
Owner

My Rust knowledge is also very basic. I'm interested in this language, but it's not at top of the list of things I want to do when I have some free time. :-)

In that case, let's stick to the C-code for now.
Should you ever feel motivated enough, replacing the c code with rust code would be greatly appreciated.

- Use mixedCase for all variables in Python (for consistency)
- Prepend all ffi variables with 'ffi'
- Rename the 'Bigram' struct in C to 'BigramC'
@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 17, 2023

By the way, could you do a few further things:

For me (=maintainability):

  • Please add comments to the C-code. It would be great to have the purpose of each file and function documented (even though your great naming helps quite a bit). Additional comments mid-code also would be great, as you see fit.
  • It is especially important to add comments in any intermediary programming steps that differ from the python code you're trying to recreate.
  • Please create a short README-file in the cffi-directory that explains what cffi is, what parts of the code were extracted and why it was done. Feel free to also refer to your article or any other (easily understandable) resources.

For users:

  • In the main README, please mention the additional steps necessary to get the optimizer running. Mention that the prompt only needs to be run once.
  • In the error message in main.py, please mention that there is the option to turn off cffi mode. (If you have objections to that point let me know, I'm not 100% convinced of it myself.)

@MaxGyver83
Copy link
Contributor Author

* [ ]  Please add comments to the C-code. It would be great to have the purpose of each file and function documented (even though your great naming helps quite a bit). Additional comments mid-code also would be great, as you see fit.

* [ ]  It is especially important to add comments in any intermediary programming steps that differ from the python code you're trying to recreate.

C comments added in 5e79c9a.

@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 20, 2023

I think I'm slowly running out of things to complain about ;)
I've now started with testing. Three things:

  1. Quick question: What was the thought process behind using USE_CFFI = False by default? To avoid crashes for preexisting users?
    I think this PR is good enough to leave this Option enabled by default.

  1. I saw that it is also required to do pip install cffi. How about we also add that info to the main README, something like this command:
cd cffi && pip install cffi && pypy3 cffi_extension_build.py ; cd ..

(The last point is to return to the root, allowing for the next command to be run seamlessly. ; to do so even if previous steps fail.)


  1. On that note, I receive this error when trying to do pypy3 cffi_extension_build.py (I'm testing on Linux, if that makes any difference):
$ pypy3 cffi_extension_build.py
generating ./_cffi_extension.c
(already up-to-date)
the current directory is '/home/florian/Downloads/8vim_keyboard_layout_calculator-improve-performance-with-cffi/cffi'
running build_ext
building '_cffi_extension' extension
gcc -pthread -DNDEBUG -O2 -fPIC -I/usr/include/pypy3.8 -c _cffi_extension.c -o ./_cffi_extension.o
_cffi_extension.c:50:14: fatal error: pyconfig.h: No such file or directory
   50 | #    include <pyconfig.h>
      |              ^~~~~~~~~~~~
compilation terminated.
Traceback (most recent call last):
  File "/usr/lib/pypy3.8/distutils/unixccompiler.py", line 133, in _compile
    extra_postargs)
  File "/usr/lib/pypy3.8/distutils/ccompiler.py", line 910, in spawn
    spawn(cmd, dry_run=self.dry_run)
  File "/usr/lib/pypy3.8/distutils/spawn.py", line 36, in spawn
    _spawn_posix(cmd, search_path, dry_run=dry_run)
  File "/usr/lib/pypy3.8/distutils/spawn.py", line 159, in _spawn_posix
    % (cmd, exit_status))
distutils.errors.DistutilsExecError: command 'gcc' failed with exit status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 51, in _build
    dist.run_command('build_ext')
  File "/usr/lib/pypy3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 343, in run
    self.build_extensions()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 452, in build_extensions
    self._build_extensions_serial()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 477, in _build_extensions_serial
    self.build_extension(ext)
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 537, in build_extension
    depends=ext.depends)
  File "/usr/lib/pypy3.8/distutils/ccompiler.py", line 574, in compile
    self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts)
  File "/usr/lib/pypy3.8/distutils/unixccompiler.py", line 135, in _compile
    raise CompileError(msg)
distutils.errors.CompileError: command 'gcc' failed with exit status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "cffi_extension_build.py", line 17, in <module>
    ffibuilder.compile(verbose=True)
  File "/usr/lib/pypy3.8/cffi/api.py", line 727, in compile
    compiler_verbose=verbose, debug=debug, **kwds)
  File "/usr/lib/pypy3.8/cffi/recompiler.py", line 1565, in recompile
    compiler_verbose, debug)
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 22, in compile
    outputfilename = _build(tmpdir, ext, compiler_verbose, debug)
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 58, in _build
    raise VerificationError('%s: %s' % (e.__class__.__name__, e))
cffi.VerificationError: CompileError: command 'gcc' failed with exit status 1

Edit: "This will use a C compiler in the background. If you don't have one installed, install one and try again." Could this be the issue, I think I never explicitly installed a C-compiler. If this step is required, I think we should

  • Add that info to the README
  • Maybe keep the cffi-option off by default, since setup is more complicated than anticipated.

@MaxGyver83
Copy link
Contributor Author

1. Quick question: What was the thought process behind using `USE_CFFI = False` by default? To avoid crashes for preexisting users?
   I think this PR is good enough to leave this Option enabled by default.

My thought was that when USE_CFFI defaults to False, the calculator works out of the box. When it defaults to True, the user has to either compile the C extension or to set USE_CFFI to True.

* Maybe keep the cffi-option off by default, since setup is more complicated than anticipated.

OK.

2. I saw that it is also required to do `pip install cffi`. How about we also add that info to the main README, something like this command:
cd cffi && pip install cffi && pypy3 cffi_extension_build.py ; cd ..

(The last point is to return to the root, allowing for the next command to be run seamlessly. ; to do so even if previous steps fail.)

AFAIU, cffi is included in a PyPy installation. It only has to be installed, when somebody wants to use CFFI with CPython.
OK, I'll add a cd ...

3. On that note, I receive this error when trying to do `pypy3 cffi_extension_build.py` (I'm testing on Linux, if that makes any difference):
$ pypy3 cffi_extension_build.py
generating ./_cffi_extension.c
(already up-to-date)
the current directory is '/home/florian/Downloads/8vim_keyboard_layout_calculator-improve-performance-with-cffi/cffi'
running build_ext
building '_cffi_extension' extension
gcc -pthread -DNDEBUG -O2 -fPIC -I/usr/include/pypy3.8 -c _cffi_extension.c -o ./_cffi_extension.o
_cffi_extension.c:50:14: fatal error: pyconfig.h: No such file or directory
   50 | #    include <pyconfig.h>
      |              ^~~~~~~~~~~~
compilation terminated.
Traceback (most recent call last):
  File "/usr/lib/pypy3.8/distutils/unixccompiler.py", line 133, in _compile
    extra_postargs)
  File "/usr/lib/pypy3.8/distutils/ccompiler.py", line 910, in spawn
    spawn(cmd, dry_run=self.dry_run)
  File "/usr/lib/pypy3.8/distutils/spawn.py", line 36, in spawn
    _spawn_posix(cmd, search_path, dry_run=dry_run)
  File "/usr/lib/pypy3.8/distutils/spawn.py", line 159, in _spawn_posix
    % (cmd, exit_status))
distutils.errors.DistutilsExecError: command 'gcc' failed with exit status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 51, in _build
    dist.run_command('build_ext')
  File "/usr/lib/pypy3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 343, in run
    self.build_extensions()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 452, in build_extensions
    self._build_extensions_serial()
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 477, in _build_extensions_serial
    self.build_extension(ext)
  File "/usr/lib/pypy3.8/distutils/command/build_ext.py", line 537, in build_extension
    depends=ext.depends)
  File "/usr/lib/pypy3.8/distutils/ccompiler.py", line 574, in compile
    self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts)
  File "/usr/lib/pypy3.8/distutils/unixccompiler.py", line 135, in _compile
    raise CompileError(msg)
distutils.errors.CompileError: command 'gcc' failed with exit status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "cffi_extension_build.py", line 17, in <module>
    ffibuilder.compile(verbose=True)
  File "/usr/lib/pypy3.8/cffi/api.py", line 727, in compile
    compiler_verbose=verbose, debug=debug, **kwds)
  File "/usr/lib/pypy3.8/cffi/recompiler.py", line 1565, in recompile
    compiler_verbose, debug)
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 22, in compile
    outputfilename = _build(tmpdir, ext, compiler_verbose, debug)
  File "/usr/lib/pypy3.8/cffi/ffiplatform.py", line 58, in _build
    raise VerificationError('%s: %s' % (e.__class__.__name__, e))
cffi.VerificationError: CompileError: command 'gcc' failed with exit status 1

This error message doesn't look like gcc is missing. In this case, I would expect something like gcc: command not found. But you can check if gcc is installed by running gcc --version.

Which distribution do you use? You could try installing the python-dev package if available.

@Glitchy-Tozier
Copy link
Owner

AFAIU, cffi is included in a PyPy installation. It only has to be installed, when somebody wants to use CFFI with CPython.

Interesting, did't know about that.

This error message doesn't look like gcc is missing. In this case, I would expect something like gcc: command not found. But you can check if gcc is installed by running gcc --version.

You're right, it already is installed:

$ gcc --version
gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Which distribution do you use?

Linux Mint (based on Ubuntu).

You could try installing the python-dev package if available.

Which one do you mean and how would it benefit the process?
image

@MaxGyver83
Copy link
Contributor Author

Maybe it's named python3-dev!?

Your error message looks like pyconfig.h is needed. On my laptop it's located here:

/opt/pypy3/include/pypy3.9/pyconfig.h

and here

/usr/include/python3.11/pyconfig.h

Maybe it's already installed but gcc can't find it. You could try searching for it:

fd pyconfig.h /

If you don't have fd installed, then try:

find / -name pyconfig.h 2>/dev/null

@Glitchy-Tozier
Copy link
Owner

Maybe it's named python3-dev!?

Yep, that one exists and I already had it installed.

Your error message looks like pyconfig.h is needed. On my laptop it's located here:

/opt/pypy3/include/pypy3.9/pyconfig.h

and here

/usr/include/python3.11/pyconfig.h

Maybe it's already installed but gcc can't find it. You could try searching for it:

fd pyconfig.h /

If you don't have fd installed, then try:

find / -name pyconfig.h 2>/dev/null

I got this. What does it tell us?

$ find / -name pyconfig.h 2>/dev/null
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/22.08/.74a00ed8035dcdea3f18a098764adfe0243a7bb5e6532caee6c8c14b2ce010a9-1H2BZ1/files/include/python3.10/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/22.08/.74a00ed8035dcdea3f18a098764adfe0243a7bb5e6532caee6c8c14b2ce010a9-1H2BZ1/files/lib/x86_64-linux-gnu/include/python3.10/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/22.08/2652112cf6f02f590602d3379a36a0e59d801a120f8ba4bb43170e75e3ab73da/files/include/python3.10/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/22.08/2652112cf6f02f590602d3379a36a0e59d801a120f8ba4bb43170e75e3ab73da/files/lib/x86_64-linux-gnu/include/python3.10/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/20.08/52034b76d4c302e58dd0c5b2b46d00b9f33a68cbe56859a264e426880e00979c/files/include/python3.8/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/20.08/52034b76d4c302e58dd0c5b2b46d00b9f33a68cbe56859a264e426880e00979c/files/lib/x86_64-linux-gnu/include/python3.8/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/21.08/.a439c06472701e00aead26fc20f474b187604a7803d942c56908743ef54e9c61-LN80X1/files/include/python3.9/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/21.08/.a439c06472701e00aead26fc20f474b187604a7803d942c56908743ef54e9c61-LN80X1/files/lib/x86_64-linux-gnu/include/python3.9/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/21.08/00ad08145692e66d6d7eff58c56299ac1e53b383877ba2c12497bc21cf16f946/files/include/python3.9/pyconfig.h
/var/lib/flatpak/runtime/org.freedesktop.Sdk/x86_64/21.08/00ad08145692e66d6d7eff58c56299ac1e53b383877ba2c12497bc21cf16f946/files/lib/x86_64-linux-gnu/include/python3.9/pyconfig.h

(...Many copies in timeshift snapshots...)

/usr/include/x86_64-linux-gnu/python3.10/pyconfig.h
/usr/include/python3.10/pyconfig.h

@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 21, 2023

By the way, I just realized: Using python3, it does compile:

$ cd cffi && pip install cffi && python3 cffi_extension_build.py ; cd ..

Defaulting to user installation because normal site-packages is not writeable
Collecting cffi
  Using cached cffi-1.15.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (441 kB)
Requirement already satisfied: pycparser in /home/florian/.local/lib/python3.10/site-packages (from cffi) (2.21)
Installing collected packages: cffi
Successfully installed cffi-1.15.1
generating ./_cffi_extension.c
(already up-to-date)
the current directory is '/home/florian/Downloads/8vim_keyboard_layout_calculator-improve-performance-with-cffi/cffi'
running build_ext
building '_cffi_extension' extension
x86_64-linux-gnu-gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/usr/include/python3.10 -c _cffi_extension.c -o ./_cffi_extension.o
x86_64-linux-gnu-gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/usr/include/python3.10 -c cffi_extension.c -o ./cffi_extension.o
x86_64-linux-gnu-gcc -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -g -fwrapv -O2 -Wl,-Bsymbolic-functions -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 ./_cffi_extension.o ./cffi_extension.o -o ./_cffi_extension.cpython-310-x86_64-linux-gnu.so

It can then be started via python3 main.py.

The previous error only occurs when using pypy3 for the build script.

@MaxGyver83
Copy link
Contributor Author

OK, no matches in a pypy folder. I suppose you'll have to install pypy3-dev to get the header files for PyPy, too.

@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 21, 2023

Hey, that did the trick! :)

In that case, to (probably) conclude this PR, could you...

  1. Move the instructions from the main README into the cffi-README and link to it (while keeping the mention of much faster execution time)
  2. Extend the instructions in the cffi-README to mention those {interpreter}-dev-packages. I've written up a suggestion, a markdown-table that seems to be pretty compact but mentions most of the necessary information (Here's the generator.):
| 1. Decide on Interpreter         | pypy3<br>(recommended)                                 | python3                                                  |
|:---------------------------------|--------------------------------------------------------|----------------------------------------------------------|
| 2. Install package (Linux only?) | pypy3-dev                                              | python3-dev                                              |
| 3. Get `cffi`                    | Included by default                                    | Run `pip install cffi`                                   |
| 4. Build code                    | Run `cd cffi && pypy3 cffi_extension_build.py ; cd ..` | Run `cd cffi && python3 cffi_extension_build.py ; cd ..` |
| 5. Start optimizer               | Run `pypy3 main.py`                                    | Run `python3 main.py`                                    |
1. Decide on Interpreter pypy3
(recommended)
python3
2. Install package (Linux only?) pypy3-dev python3-dev
3. Get cffi Included by default Run pip install cffi
4. Build code Run cd cffi && pypy3 cffi_extension_build.py ; cd .. Run cd cffi && python3 cffi_extension_build.py ; cd ..
5. Start optimizer Run pypy3 main.py Run python3 main.py

@MaxGyver83
Copy link
Contributor Author

Great overview! I'll update the READMEs when I'm back home.

@Glitchy-Tozier
Copy link
Owner

Glitchy-Tozier commented May 21, 2023

Great! Feel free to edit it if you feel like making any changes / have additions.
(When editing, keep that colon (:-------------------) intact to preserve left-alignment.)

@Glitchy-Tozier
Copy link
Owner

Neat! one typo-fix and we're done! :)
Screenshot_20230521-205636_GitHub.jpg

@MaxGyver83 MaxGyver83 force-pushed the improve-performance-with-cffi branch from 93bcdcf to bf6d3f3 Compare May 21, 2023 19:51
@MaxGyver83
Copy link
Contributor Author

Neat! one typo-fix and we're done! :)

Fixed!

@Glitchy-Tozier
Copy link
Owner

Well then let's merge!
Thank you very much for the hard work and the patience when working through this PR. :)

@Glitchy-Tozier Glitchy-Tozier merged commit 9a65ff3 into Glitchy-Tozier:main May 21, 2023
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.

2 participants