ridiculousfish / libdivide

Official git repository for libdivide: optimized integer division
http://libdivide.com
Other
1.09k stars 77 forks source link

Add ARM CI builds using Drone CI #62

Closed nightlark closed 4 years ago

nightlark commented 4 years ago

I setup some CI tests running on ARM hardware (32-bit and 64-bit builds) using Drone CI (https://cloud.drone.io/ for setting up -- uses webhooks).

The benchmark results are interesting, it looks like for 32-bit ARM builds branchfree is faster than the system divide for 32-bit datatypes, but 64-bit builds the benchmark results are all pretty similar.

kimwalisch commented 4 years ago

At this point in time I am only interested in either improving our existing Appveyor CI script or a new GitHub actions CI script. I have never heard of Drone CI before and I don't know if it will still be around in 10 years.

nightlark commented 4 years ago

@kimwalisch not being open to other CI providers is limiting for tests/benchmarks that run on actual ARM hardware. What I've found is the current situation for ARM testing on CI services:

Drone has been around in some form or another since 2014. Servers with ARM processors installed seem to have only become more widely available around 2016-2017. If the concern is maintaining it, yea, there's nothing that guarantees Drone or Appveyor (or even GitHub) will be around in 10 years either or that any of them could make changes that break the current CI setup.

The impression I'm getting from the response is that the project is in maintenance mode (README and website update might be worthwhile if this is the case)? Or doesn't care about running ARM CI tests on actual hardware instead of emulated? If it's the latter then a docker container with qemu running a build and emulated ARM smoke tests in GitHub actions would be fairly easy to get setup.

kimwalisch commented 4 years ago

@kimwalisch not being open to other CI providers is limiting for tests/benchmarks that run on actual ARM hardware. What I've found is the current situation for ARM testing on CI services:

I take testing very seriously and I can assure you that libdivide has been tested thoroughly on x86, x64, ARM, ARM64, PowerPC and PPC64. Since I am maintaining many open source projects (for free in my sparse time) one of my goals is to reduce the maintenance burden of each of these projects to a minimum. For this reason I want to support only one CI service in libdivide. I have nothing against Drone CI expect that I don't know it and if at some point in the future the Drone CI tests would fail I would have to learn Drone CI to fix the issue (for free in my sparse time).

The impression I'm getting from the response is that the project is in maintenance mode (README and website update might be worthwhile if this is the case)?

It is not strictly in maintenance mode but both the API and the division algorithms are very stable at this point in time and I expect mostly small code changes going forward.

nightlark commented 4 years ago

@kimwalisch thanks for clarifying the reason for not wanting to add another CI service. As someone who put spare time into opening a PR, the extra explanation helps a lot with making it not feel like the changes are being tossed in the trash (any response is great compared to no life on a PR for months/years). 👍

If I have spare time, I can look into getting some ARM (+ PowerPC) CI builds/tests running on Appveyor or GitHub actions using qemu. Let me know if there's interest in that option (I expect I'd be at least partially on the hook for helping fix the CI builds if they break in the future).

@jwood000 nevermind, see edit history if you want to know what the original question was. tl;dr is that I was feeling kinda hurt earlier, and I'm over it now.

kimwalisch commented 4 years ago

If I have spare time, I can look into getting some ARM (+ PowerPC) CI builds/tests running on Appveyor or GitHub actions using qemu. Let me know if there's interest in that option (I expect I'd be at least partially on the hook for helping fix the CI builds if they break in the future).

The main reason why I think we should not add ARM testing to our Appveyor CI script is that ARM testing is not supported out of the box by Appveyor and it will hence require a large amount of code to make it work and this increases the maintenance burden.

If ARM testing is supported out of the box by GitHub actions (similar to Drone CI) then I would happily accept a pull request to add this feature as I think it would make sense to migrate testing from Appveyor to the offical testing service from GitHub. But then ideally all our tests should be migrated from Appveyor to GitHub actions so that we can get rid of Appveyor.

Testing in libdivide is rather complicated and unfortunately I am missing the required rights to improve the situation. If I was the original owner of the libdivide repo I would even have likely accepted your Drone CI pull request as I would have been able to properly set up testing. But as is, even if I would have accepted your PR I wouldn't have been able to configure Drone CI so that the tests would be run automatically for each new commit or PR. So please don't take it personal that I have not accepted your PR...