JoshData / fast_diff_match_patch

Python package for Google's diff-match-patch native C++ implementation.
https://pypi.org/project/fast-diff-match-patch/
Apache License 2.0
73 stars 27 forks source link

Added wheels for aarch64 and Apple silicon chips #22

Closed pratyushmittal closed 2 years ago

pratyushmittal commented 2 years ago

The default wheels are not built for aarch64, Apple M1 and M2 silicon: https://cibuildwheel.readthedocs.io/en/stable/options/#archs

The wheels are built using the native machine. The aarch64 and universal builds are not included in default because they cannot be tested by native machine.

We need to set the builds specifically to support multiple architectures.

This is the usual way and is done by other libraries too: https://github.com/pypa/cibuildwheel/discussions/485

Also updated the version of cibuildwheel to support new architectures.

This fixes https://github.com/JoshData/fast_diff_match_patch/issues/19

JoshData commented 2 years ago

The Linux build is failing. I was able to see an error quickly in the Actions output (something about "exec" image format, I think) but didn't notice in time. After reloading the page, I can't get the log again for the build step. Maybe we can drop aarch64 for now so we can at least get the MacOS builds.

pratyushmittal commented 2 years ago

Hey, thank @JoshData for looking into the builds. Please give me some time, I will try to figure Linux thing too.

I use AWS Graviton servers: https://aws.amazon.com/ec2/graviton/. They use aarch64. Hence I would need it to use it in production.

Thanks for your support.

pratyushmittal commented 2 years ago

Hey, I have fixed the aarch64 build issue in https://github.com/JoshData/fast_diff_match_patch/pull/22/commits/5eb31c852bf4379e83c712ad28aabc431516bcd8.

The non-native builds for Linux require architecture emulation. This was clearly mentioned in the docs with examples, yet I missed it:

On Linux, this option can be used to build non-native architectures under emulation. See this guide for more information. -from guide

The wheels are building fine for all the architectures now. However, the total build time on Github Actions will be around 40 minutes now 😅.

pratyushmittal commented 2 years ago

Hey @JoshData , you can cancel this two runs: https://github.com/JoshData/fast_diff_match_patch/actions/runs/2670712954 https://github.com/JoshData/fast_diff_match_patch/actions/runs/2668872644

These will take forever to complete and waste CPU time.

pratyushmittal commented 2 years ago

It is still taking a lot of time. I have been reading around how other libraries have solved this issue.

Scikit-learn reduced their built times from 6 hours to under 50 minutes. They used a highly targeted wheels distribution.

However, the main issue with our current built system is different. We don't ship the source-dist. We ship only the built wheels. Hence, the installation fails if the wheel is not available.

I am trying to create a build for source dist as well. Will drop an improved pull-request soon.

Thanks a lot for bearing with these :).

JoshData commented 2 years ago

The build is working now so I'm happy to merge this PR if you want to be able to use the wheels now and we can solve the source dist in a later PR, if you like.

pratyushmittal commented 2 years ago

Hey @JoshData, I have included the source distribution in https://github.com/JoshData/fast_diff_match_patch/pull/22/commits/6511bf01afb814201b585615ef55df39acffb772. It reduces the build time to 15 minutes by removing the need for emulation.

This should work fine on all the devices now.

JoshData commented 2 years ago

Thanks very much for getting this working. I think I've gotten everything uploaded to PyPi now.

pratyushmittal commented 2 years ago

Thanks @JoshData for a quick merge.

I tested it on my staging server aarch64. Works perfectly. Takes just a few seconds to build the wheel and install.

👍

JoshData commented 2 years ago

Great!