Re: [RFA] autotest speedup

[Eric Blake]

Paolo Bonzini <paolo.bonzini <at> lu.unisi.ch> writes:

> 
> This speeds up autotest generation by ~6% (15.9 to 15.1 seconds) by 
> outlining detection of tracing into a shell function, and removing the 
> at_trace_this variable

This would be the first use of shell functions in autotest.  I think we are 
finally ready for this move - we've had the shell function sniffer out in the 
wild for quite some time now, with only a few reports of failure, and even 
then, it wasn't due to lack of functions, but to bugs that we can be careful of 
in our use of functions.

However, I was ALSO planning on touching this area of code, to speed up the 
m4_bpatsubsts.

> 
> +#######################
> +### Shell functions ###
> +#######################

Let's use m4_text_box here.  And are we doing this in the optimal diversion?

> +
> +at_check_newline ()
> +{
> +  case "$[1]" in
> + *'
> +'*) echo 'Not enabling shell tracing (command contains an embedded 
newline)' ; return 1 ;;
> + *) return 0 ;;
> +  esac
> +}

Do we need any m4sh support for writing portable shell functions?  Are there 
lessons to be learned from libtool?

Here's where I got, when I saw your email.

I debated about making:
m4_cond(TEST, [not,] VAL1, IF-[NOT-]VAL1, ...)
with semantics that if $2 is the literal string [not], then VAL1 is $3 instead 
of $2, and IF-VAL1 is executed only when TEST is not equal to VAL1, otherwise 
proceed on to the next TEST.  I may still try something like that (it would 
reduce the number of m4_eval's in the clients of m4_cond), but would make 
m4_cond itself more complex, so I don't know if the tradeoff would save any 
time.



From: Eric Blake <address@hidden>
Date: Thu, 4 Oct 2007 12:29:45 -0600
Subject: [PATCH] Provide better short-circuiting operation.

* lib/m4sugar/m4sugar.m4 (m4_cond, m4_newline): New macros.
(m4_text_wrap): Use it.  Also avoid useless m4_for.
* lib/m4sugar/m4sh.m4 (_AS_QUOTE_IFELSE, AS_LITERAL_IF): Use
new macro.
(_AS_IDENTIFIER_IF): Likewise, and fix bug when $1 is [,].
* lib/autotest/general.m4 (_AT_DECIDE_TRACEABLE): Use new macros
to avoid regexps.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |    9 +++++++
 lib/autotest/general.m4 |   32 +++++++++++++-------------
 lib/m4sugar/m4sh.m4     |   35 ++++++++++++++-------------
 lib/m4sugar/m4sugar.m4  |   58 ++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5a37f4c..25824d7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2007-10-04  Eric Blake  <address@hidden>
 
+       Provide better short-circuiting operation.
+       * lib/m4sugar/m4sugar.m4 (m4_cond, m4_newline): New macros.
+       (m4_text_wrap): Use it.  Also avoid useless m4_for.
+       * lib/m4sugar/m4sh.m4 (_AS_QUOTE_IFELSE, AS_LITERAL_IF): Use
+       new macro.
+       (_AS_IDENTIFIER_IF): Likewise, and fix bug when $1 is [,].
+       * lib/autotest/general.m4 (_AT_DECIDE_TRACEABLE): Use new macros
+       to avoid regexps.
+
        One more round of m4_foreach_w speedups.
        * lib/m4sugar/m4sugar.m4 (m4_flatten): Only use regex if newline
        is present.
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index dfd098c..d311b6a 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1412,32 +1412,32 @@ m4_define([AT_CHECK_NOESCAPE],
 # to safely expand arbitrary COMMANDS in an argument list, so the below tests
 # examine COMMANDS unexpanded.
 m4_define([_AT_DECIDE_TRACEABLE],
-[dnl Utility macros.
-m4_pushdef([at_lf], [
-])[]dnl
+dnl Utility macro.
 dnl
 dnl Examine COMMANDS for a reason to never trace COMMANDS.
-m4_pushdef([at_reason],
-          m4_bmatch([$1],
-                    [`.*`], [[a `...` command substitution]],
-                    [\$(],  [[a $(...) command substitution]],
-                    [\${],  [[a ${...} parameter expansion]],
-                    at_lf,  [[an embedded newline]],
-                    [[]]dnl No reason.
+[m4_pushdef([at_reason],
+m4_cond([m4_eval(m4_index([$1], [`]) >= 0)], [1],
+               [[a `...` command substitution]],
+       [m4_eval(m4_index([$1], [$(]) >= 0)], [1],
+               [[a $(...) command substitution]],
+       [m4_eval(m4_index([$1], [${]) >= 0)], [1],
+               [[a ${...} parameter expansion]],
+       [m4_eval(m4_index([$1], m4_newline) >= 0)], [1],
+               [[an embedded newline]],
+       []dnl No reason.
 ))dnl
 dnl
 m4_ifval(m4_defn([at_reason]),
 [echo 'Not enabling shell tracing (command contains ]m4_defn([at_reason])[)'],
-[m4_bmatch([$1], [\$],
-dnl COMMANDS may contain parameter expansions; expand them at runtime.
+[m4_if(m4_index([$1], [$]), [-1],
+       dnl We know at build time that tracing COMMANDS is always safe.
+[at_trace_this=yes],
+       dnl COMMANDS may contain parameter expansions; expand them at runtime.
 [case "AS_ESCAPE([$1], [`\"])" in
  *'
 '*) echo 'Not enabling shell tracing (command contains an embedded newline)' ;;
  *) at_trace_this=yes ;;
-    esac],
-dnl We know at build time that tracing COMMANDS is always safe.
-[at_trace_this=yes])])[]dnl
-m4_popdef([at_lf])[]dnl
+    esac])])[]dnl
 m4_popdef([at_reason])])
 
 
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 025934d..8d441ab 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -588,12 +588,12 @@ m4_define([_AS_ESCAPE],
 # The current implementation caters to the common case of no backslashes,
 # to minimize m4_index expansions (hence the nested if).
 m4_define([_AS_QUOTE_IFELSE],
-[m4_if(m4_index([$1], [\]), [-1], [$2],
-       [m4_if(m4_eval(m4_index([$1], [\\]) >= 0), [1], [$2],
-             m4_eval(m4_index([$1], [\$]) >= 0), [1], [$2],
-             m4_eval(m4_index([$1], [\`]) >= 0), [1], [$3],
-             m4_eval(m4_index([$1], [\"]) >= 0), [1], [$3],
-             [$2])])])
+[m4_cond([m4_index([$1], [\])], [-1], [$2],
+        [m4_eval(m4_index([$1], [\\]) >= 0)], [1], [$2],
+        [m4_eval(m4_index([$1], [\$]) >= 0)], [1], [$2],
+        [m4_eval(m4_index([$1], [\`]) >= 0)], [1], [$3],
+        [m4_eval(m4_index([$1], [\"]) >= 0)], [1], [$3],
+        [$2])])
 
 
 # _AS_QUOTE(STRING, [CHARS = `"])
@@ -1248,11 +1248,12 @@ m4_define([AS_IDENTIFIER_IF],
        [_$0($@)],
        [_$0(m4_bpatsubst([[$1]], [@&address@hidden), [$2], [$3])])])
 m4_define([_AS_IDENTIFIER_IF],
-[m4_if([$1], [], [$3],
-       m4_translit([$1], ]m4_dquote(m4_defn([m4_cr_symbols2]))[), [],
-       [m4_if(m4_len(m4_translit(m4_format([[%.1s]], [$1]), ]]dnl
-m4_dquote(m4_dquote(m4_defn([m4_cr_symbols1])))[[)), [0], [$2], [$3])],
-       [$3])])
+[m4_cond([[$1]], [], [$3],
+        [m4_eval(m4_len(m4_translit([[$1]], ]]dnl
+m4_dquote(m4_dquote(m4_defn([m4_cr_symbols2])))[[)) > 0)], [1], [$3],
+        [m4_len(m4_translit(m4_format([[%.1s]], [$1]), ]]dnl
+m4_dquote(m4_dquote(m4_defn([m4_cr_symbols1])))[[))], [0], [$2], [$3])])
+
 
 # AS_LITERAL_IF(EXPRESSION, IF-LITERAL, IF-NOT-LITERAL)
 # -----------------------------------------------------
@@ -1280,12 +1281,12 @@ m4_dquote(m4_dquote(m4_defn([m4_cr_symbols1])))[[)), 
[0], [$2], [$3])],
 # Rather than expand m4_defn every time AS_LITERAL_IF is expanded, we
 # inline its expansion up front.
 m4_define([AS_LITERAL_IF],
-[m4_if(m4_eval(m4_index(m4_quote($1), address@hidden|@]) == -1), [0], [$3],
-       m4_index(m4_translit(m4_quote($1),
-                           [[]`,#]]m4_dquote(m4_defn([m4_cr_symbols2]))[,
-                           [$$$]),
-               [$]), [-1], [$2],
-       [$3])])
+[m4_cond([m4_eval(m4_index(m4_quote($1), address@hidden|@]) == -1)], [0], [$3],
+        [m4_index(m4_translit(m4_quote($1),
+                              [[]`,#]]m4_dquote(m4_defn([m4_cr_symbols2]))[,
+                              [$$$]),
+                  [$])], [-1], [$2],
+        [$3])])
 
 
 # AS_TMPDIR(PREFIX, [DIRECTORY = $TMPDIR [= /tmp]])
diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index d9cf142..c7319c2 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -399,6 +399,42 @@ m4_define([m4_cdr],
        [m4_dquote(m4_shift($@))])])
 
 
+# m4_cond(TEST1, VAL1, IF-VAL1, TEST2, VAL2, IF-VAL2, ..., [DEFAULT])
+# -------------------------------------------------------------------
+# Similar to m4_if, except that each TEST is expanded when encountered.
+# If the expansion of TESTn matches the string VALn, the result is IF-VALn.
+# The result is DEFAULT if no tests passed.  This macro allows
+# short-circuiting of expensive tests, where it pays to arrange quick
+# filter tests to run first.
+#
+# For an example, consider a previous implementation of _AS_QUOTE_IFELSE:
+#
+#    m4_if(m4_index([$1], [\]), [-1], [$2],
+#          m4_eval(m4_index([$1], [\\]) >= 0), [1], [$2],
+#          m4_eval(m4_index([$1], [\$]) >= 0), [1], [$2],
+#          m4_eval(m4_index([$1], [\`]) >= 0), [1], [$3],
+#          m4_eval(m4_index([$1], [\"]) >= 0), [1], [$3],
+#          [$2])
+#
+# Here, m4_index is computed 5 times, and m4_eval 4, even if $1 contains
+# no backslash.  It is more efficient to do:
+#
+#    m4_cond([m4_index([$1], [\])], [-1], [$2],
+#            [m4_eval(m4_index([$1], [\\]) >= 0)], [1], [$2],
+#            [m4_eval(m4_index([$1], [\$]) >= 0)], [1], [$2],
+#            [m4_eval(m4_index([$1], [\`]) >= 0)], [1], [$3],
+#            [m4_eval(m4_index([$1], [\"]) >= 0)], [1], [$3],
+#            [$2])
+#
+# In the common case of $1 with no backslash, only one m4_index expansion
+# occurs, and m4_eval is avoided altogether.
+m4_define([m4_cond],
+[m4_if([$#], [0], [m4_fatal([$0: cannot be called without arguments])],
+       [$#], [1], [$1],
+       [$#], [2], [m4_fatal([$0: missing an argument])],
+       [m4_if($1, [$2], [$3], [$0(m4_shift3($@))])])])
+
+
 # m4_map(MACRO, LIST)
 # -------------------
 # Invoke MACRO($1), MACRO($2) etc. where $1, $2... are the elements
@@ -1448,6 +1484,13 @@ _m4_define_cr_not([symbols1])
 _m4_define_cr_not([symbols2])
 
 
+# m4_newline
+# ----------
+# Expands to a newline.  Exists for formatting reasons.
+m4_define([m4_newline], [
+])
+
+
 # m4_re_escape(STRING)
 # --------------------
 # Escape RE active characters in STRING.
@@ -1696,15 +1739,14 @@ m4_pushdef([m4_Width], m4_default([$4], 79))dnl
 m4_pushdef([m4_Cursor], m4_qlen(m4_defn([m4_Prefix1])))dnl
 m4_pushdef([m4_Separator], [])dnl
 m4_defn([m4_Prefix1])[]dnl
-m4_if(m4_eval(m4_qlen(m4_defn([m4_Prefix1])) > m4_len(m4_Prefix)),
-      1, [m4_define([m4_Cursor], m4_len(m4_Prefix))
+m4_cond([m4_eval(m4_qlen(m4_defn([m4_Prefix1])) > m4_len(m4_Prefix))],
+       [1], [m4_define([m4_Cursor], m4_len(m4_Prefix))
 m4_Prefix],
-      m4_if(m4_eval(m4_qlen(m4_defn([m4_Prefix1])) < m4_len(m4_Prefix)),
-           [0], [],
-           [m4_define([m4_Cursor], m4_len(m4_Prefix))[]dnl
-m4_for(m4_Space, m4_qlen(m4_defn([m4_Prefix1])), m4_eval(m4_len(m4_Prefix) - 
1),
-                   [], [ ])])[]dnl
-)[]dnl
+       [m4_eval(m4_qlen(m4_defn([m4_Prefix1])) < m4_len(m4_Prefix))],
+       [0], [],
+       [m4_define([m4_Cursor], m4_len(m4_Prefix))[]dnl
+m4_format(%m4_eval(m4_len(m4_Prefix) - 1 - m4_qlen(m4_defn([m4_Prefix1]))),
+         [])])[]dnl
 m4_foreach_w([m4_Word], [$1],
 [m4_define([m4_Cursor], m4_eval(m4_Cursor + m4_qlen(m4_defn([m4_Word])) + 1))
dnl
 dnl New line if too long, else insert a space unless it is the first
-- 
1.5.3.2