cmake: speed up zstd detection#12200
Conversation
Before this patch we detected the presence of a specific zstd API to see if we can use the library. zstd introduced that API in its first stable release: v1.0.0. Replace that method by detecting the zstd library version instead and accepting if it's v1.0.0 or newer. Also display this detected version. We use the same version detection method as zstd itself, using its public C header. This deviates from autotools which keeps using the slow method of looking for the API by building a test program. The outcome is the same as long as zstd keeps offering this API. Closes #xxxxx
Right, but that's how configure always detects symbols/functions. It makes sure we can link with them. I don't think that is problematic. On my Linux dev machine, a configure at current master run that detects a lot of third party components including HTTP/3, zstd, brotli and more, configure completes in less than 7 seconds. A bare "cmake" run at the same commit on the same machine takes 0.8 seconds longer while detecting fewer 3rd party libs. |
|
On my machine and on most CI machines, configure takes a lot more than 7 seconds. It's 82 seconds for a Windows cross-build on macOS (*) for example and 52 seconds for pure macOS (without running For curl-for-win specifically, this slow configuration stage needs to run twice. This performance has been traditionally the case with all Windows (and apparently macOS) machines and Windows cross-builds, as opposed to pure Linux where the necessary operations are fast. Surely with a bleeding edge computer this could also be made faster, but those are not even an option in free CI tiers. I'm not attempting to make autotools fast, because I reckon its mojo is to do and redo all the detection work on every run. This also has a valid case on *nix systems that have wide and unpredicable variations on what's available (also true for CMake). But, in many cases, these detections are repeating the exact same work with predictable results. Speaking of Windows, almost all of these results are well-known. Same case with zstd: the result is the function of its version (which in this case means ALL existing stable releases), and detecting the version is much faster than triaging a full build. I understand this might not catch issues where zstd cannot be linked for other reasons (say a broken/incompatible zstd build), or a broken toolchain setup, but these are rather rare. Also, toolchain issues are caught earlier anyway and even a broken zstd is caught at link stage latest. I think this is safe. I can fast-track this manually for "my" builds, but why not make all builds benefit? (*) To put into perspective, with CMake, with current master and unity mode, the complete curl build takes 30 seconds (11 seconds configure + 19 seconds build). Each detection (such as |
|
The zstd feature check was added for Ubuntu 16.04 LTS Xenial, according to #5453 (comment). Though Xenial is still officially supported till 2026, Ubuntu's package search page no longer supports it. According to one source, it came with v0.5.1, according to another, with v1.3.1. Either way, version detection works with v0.5.1: -- Checking for one of the modules 'libzstd'
-- Found Zstd: .../zstd-0.5.1/lib/pkg/usr/local/lib/libzstd.a (found version "0.5.1") (That said it might be a nice touch to display a warning or error if zstd is not fit for curl.) ✅ |
Stop pre-filling values that are no longer used by the next curl release. Ref: curl/curl#12200 `HAVE_ZSTD_CREATEDSTREAM` curl/curl#12202 `THREADS_HAVE_PTHREAD_ARG` `CMAKE_HAVE_LIBC_PTHREAD` `CMAKE_HAVE_PTHREADS_CREATE` `CMAKE_HAVE_PTHREAD_CREATE` curl/curl#12210 `DHAVE_VARIADIC_MACROS_C99` `HAVE_VARIADIC_MACROS_GCC`
- use the same pattern across all Find modules:
- verify if the version header exists before reading it.
- use a single regex per lookup.
- sync regexes between Find modules.
- use generic temporary variable names.
- improve readability.
- make it simpler to transition to new CMake syntax in the future:
```cmake
file(STRINGS "${CARES_INCLUDE_DIR}/ares_version.h" _version_str REGEX "<...>")
unset(_version_str)
set(CARES_VERSION "${CMAKE_MATCH_1}")
```
Ref: https://cmake.org/cmake/help/latest/policy/CMP0159.html#policy:CMP0159
- fix zstd version detection to be CMake 3.7 compatible.
Required 3.9 before this patch, for the `CMAKE_MATCH_<n>` feature.
Follow-up to c5d506e #12200
Follow-up to 4e2f364 #14548
Closes #14572
2 participants
Before this patch we detected the presence of a specific zstd API to
see if we can use the library. zstd published that API in its first
stable release: v1.0.0 (2016-08-31).
Replace that method by detecting the zstd library version instead and
accepting if it's v1.0.0 or newer. Also display this detected version
and display a warning if the zstd found is unfit for curl.
We use the same version detection method as zstd itself, via its public
C header.
This deviates from autotools which keeps using the slow method of
looking for the API by building a test program. The outcome is the same
as long as zstd keeps offering this API.
Ref: facebook/zstd@5a0c8e2 (2016-08-12, committed)
Ref: https://github.com/facebook/zstd/releases/tag/v0.8.1 (2016-08-18, first released)
Ref: https://github.com/facebook/zstd/releases/tag/v1.0.0
Closes #12200