Re: [PATCH v2 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

[Eric Blake]

On 10/31/2016 12:08 PM, Paolo Bonzini wrote:
> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
> AS_TR_CPP.  Prefer it if there is nothing in the second and third
> argument of AC_CHECK_FUNCS and the first argument is a literal.
> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
> 
> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
> (_AC_CHECK_FUNCS): Move basic implementation here.
> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  lib/autoconf/functions.m4 | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
> index 66abe29..ccfb053 100644
> --- a/lib/autoconf/functions.m4
> +++ b/lib/autoconf/functions.m4
> @@ -85,20 +85,24 @@ m4_define([_AH_CHECK_FUNC],
>  # `break' to stop the search.
>  AC_DEFUN([AC_CHECK_FUNCS],
>  [m4_map_args_w([$1], [_AH_CHECK_FUNC(], [)])]dnl
> -[AS_FOR([AC_func], [ac_func], [$1],
> -[AC_CHECK_FUNC(AC_func,
> -            [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_func)) $2],
> -            [$3])dnl])
> +[m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes],
> +       [m4_map_args_w([$1], [_$0(], [)])],
> +       [AS_FOR([AC_func], [ac_func], [$1], [_$0(AC_func, [$2], [$3])])])

Dropping the dnl adds a lot of blank lines to configure scripts, I will
massage that slightly.

>  ])# AC_CHECK_FUNCS
>  
> +m4_define([_AC_CHECK_FUNCS],
> +[AC_CHECK_FUNC([$1],
> +            [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]$1)) $2],
> +            [$3])
> +])
> +

I tested this with coreutils.  Timing wise, on my system, I was able to
consistently see a reduction of './config.status --recheck' execution
time from around 34.3s pre-patch, to around 33.9s post-patch; a little
better than 1%.  That's not huge, but big enough that I can say it is a
real improvement and not just measurement noise.

Size-wise, this results in about 17k of growth in the size of coreutils'
configure script. There is no longer an ac_func_list (which shaved some
lines up front), but unrolling the loop means you now have a lot of code
additions that look like:

+ac_fn_c_check_func "$LINENO" "btowc" "ac_cv_func_btowc"
+if test "x$ac_cv_func_btowc" = xyes
+then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_BTOWC 1
+_ACEOF
+
+fi
+

(there's that blank line I mentioned earlier)

It makes me wonder if we want a followup patch with some sort of new:

# ac_fn_c_check_func_simple line func cache witness
ac_fn_c_check_func_simple () {
  ac_fn_c_check_func "$1" "$2" "$3"
  if eval test "x\$$3" = xyes; then
    printf "#define $4\n" >>confdefs.h
  fi
}

so that we can compress call current 9-line call sites to a 1-line:

ac_fn_c_check_func_simple "$LINENO" "btowc" "ac_cv_func_btowc" "HAVE_BTWOC"

or maybe it will be easier to maintain via a 2-line approach:

ac_fn_c_check_func "$LINENO" "btowc" "ac_cv_func_btowc"
ac_fn_simple_define "ac_cv_func_btowc" "HAVE_BTOWC"

At any rate, that's food for a later patch (which I'm playing with), so
it shouldn't stop me from committing your patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature