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

autotools: reduce brute-force when detecting recv/send arg list #9591

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Sep 25, 2022

autotools uses brute-force to detect recv/send/select argument lists, by interating through all argument type combinations on each ./configure run. This logic exists since 01fa02d (from 2006) and was a bit later extended with Windows support.

This results in a worst-case number of compile + link cycles as below:

  • recv: 96
  • send: 192
  • select: 60
    Total: 348 (the number of curl C source files is 195, for comparison)

Notice that e.g. curl-for-win autotools builds require two ./configure invocations, doubling these numbers.

recv on Windows was especially unlucky because SOCKET (the correct choice there) was listed last in one of the outer trial loops. This resulted in lengthy waits while autotools was trying all invalid combinations first, wasting cycles, disk writes and slowing down iteration.

This patch reduces the amount of idle work by reordering the tests in a way to succeed first on a well-known platform such as Windows, and also on non-Windows by testing for POSIX prototypes first, on the assumption that these are the most likely candidates these days. (We do not touch select, where the order was already optimal for these platforms.)

For non-Windows, this means to try a return value of ssize_t first, then int, reordering the buffer argument type to try void * first, then byte *, and prefer the const flavor with send. If we are here, also stop testing for SOCKET type in non-Windows builds.

After the patch, detection on Windows is instantaneous. It should also be faster on popular platforms such as Linux and BSD-based ones.

If there are known-good variations for other platforms, they can also be fast-tracked like above, given a way to check for that platform inside the autotools logic.

Closes #9591

autotools uses brute-force to detect recv/send/select argument lists, by
interating through _all_ argument type combinations on each ./configure
run. This logic exists since 01fa02d
(from 2006) and was a bit later extended with Windows support.

This results in a worst-case number of compile + link cycles as below:
- `recv`: 96
- `send`: 192
- `select`: 60
Total: 348 (the number of curl C source files is 195, for comparison)

Notice that e.g. curl-for-win autotools builds require two ./configure
invocations, doubling these numbers.

`recv()` on Windows was especially unlucky because `SOCKET` (the correct
choice there) was listed last in one of the outer trial loops. This
resulted in lengthy waits while autotools was trying all invalid
combinations first, wasting cycles, disk writes and slowing down
iteration.

This patch reduces the amount of idle work, by reordering the tests in
a way to succeed first on a well-known platform such as Windows, and
also on non-Windows by testing for POSIX prototypes first, on the
assumption that these are the most likely candidates these days. (Except
for `select`, where the order was already optimal for these platforms.)

For non-Windows, this means to try a return value of `ssize_t` first,
then `int`, reordering the buffer argument type to try `void *` first,
then `byte *`, and prefer the `const` flavor with `send`.

After the patch, detection on Windows is instantaneous. It should also
be faster on popular platforms such as Linux and BSD-based ones.

Closes #xxxx
bagder
bagder approved these changes Sep 25, 2022
Copy link
Member

@bagder bagder left a comment

Nice work!

I have thought about ripping this out completely from cmake/autoconf and instead replace it with a basic #ifdef sequence but it hasn't become more than thoughts in my head so I think this is a step forward!

@vszakats
Copy link
Member Author

vszakats commented Sep 25, 2022

Thanks! Ripping it out would definitely be the best. Perf is fine now, but there is a significant complexity in the code and build systems around this, most of it probably overkill.

Data is in: The patch shaved off almost 5 minutes from curl-for-win builds. A 10% improvement.

with patch: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/44877232
without: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/44876993

@vszakats vszakats closed this in 6adcff6 Sep 25, 2022
@vszakats vszakats deleted the autotools-reduce-brute-force branch Sep 25, 2022
@bagder
Copy link
Member

bagder commented Sep 26, 2022

I took a first step in #9592. I imagine it might need a while and polish before we can switch.

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

Successfully merging this pull request may close these issues.

None yet

2 participants