autoconf-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

m4_defn overhead


From: Eric Blake
Subject: m4_defn overhead
Date: Wed, 10 Oct 2007 23:04:51 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

My last patch[1] got me thinking: we intentionally add overhead to m4_
{defn,popdef,undefine} in order to check that the variable is defined.  But in 
some of the heaviest code paths, this is needless overhead - we can prove that 
the variable is defined, and that no arbitrary code has been expanded in the 
meantime that could undefine it.

[1] http://git.sv.gnu.org/gitweb/?p=autoconf.git;a=commitdiff;h=45b46a

Furthermore, there were some subtle bugs:

M4 1.4.5 was the first version to correctly implement the POSIX rule of popping 
multiple variables with popdef(`var1', `var2'), and we now require 1.4.5.  But 
if you try m4_popdef([var1], [var2]), you will either get no warning about var2 
being undefined, or else var2 is popped contrary to the documentation.  It 
would be nice to make m4_{defn,popdef,undefine} accept multiple arguments as 
per the underlying M4 (m4_defn would be trickiest, since the underlying builtin 
isn't fixed until M4 1.4.11), but doing this would imply even more overhead in 
these three functions (ie. the addition of a new m4_if and some recursion if 
there is more than one argument); so for now I am sticking with the documented 
behavior of only one argument.

Next, the sequence
 m4_define([a],[oops])m4_define([b],[a])m4_append_u([b],[-],[a])
was not robust to unwanted expansion of [a] (fortunately, all of autoconf's 
uses of m4_append* had a non-macro separator).

Also, m4_for was buggy when start and end are the same:
  m4_define([_m4_step],[oops])m4_for(i,1,1,,l)oops!
resulted in [l!] instead of [loops!]

Finally, AS_BOX was inconsistent; when operating on literals, it forgot to 
account for expansion and quadrigraphs, but the sed script when operating on 
non-literals only sees the expanded results.  Compare:
  m4_define([a,AA])
  AS_BOX([-a@&address@hidden)
=> ## ------ ##
=> ## -AA@&t@ ##
=> ## ------ ##
  AS_BOX([`a@&address@hidden)
=> ## --- ##
=> ## `AA ##
=> ## --- ##

The result is this commit, which again knocks some time off of my coreutils 
test case.  Note that I only switched to m4_builtin if the code path proves 
that no intermediate expansions could mess with the definition; for example, I 
did not optimize the m4_popdef out of m4_foreach, because arbitrary code (in 
the form of the user's expression) occurs after the original m4_pushdef.

2007-10-10  Eric Blake  <address@hidden>

        Avoid some overhead from m4_defn and m4_popdef.
        * lib/m4sugar/m4sugar.m4 (m4_defn, m4_popdef, m4_undefine): Only
        pass on first argument, since we are documented that way.
        (m4_for, m4_append_uniq, m4_text_wrap): Optimize out defined-ness
        check where it is safe to do so.
        (m4_append): Likewise, and quote the separator.
        (m4_text_box): Likewise, and avoid regex, also be robust to
        expansion and quadrigraphs.

---

From: Eric Blake <address@hidden>
Date: Wed, 10 Oct 2007 12:17:59 -0600
Subject: [PATCH] Avoid some overhead from m4_defn and m4_popdef.

* lib/m4sugar/m4sugar.m4 (m4_defn, m4_popdef, m4_undefine): Only
pass on first argument, since we are documented that way.
(m4_for, m4_append_uniq, m4_text_wrap): Optimize out defined-ness
check where it is safe to do so.
(m4_append): Likewise, and quote the separator.
(m4_text_box): Likewise, and avoid regex, also be robust to
expansion and quadrigraphs.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |    9 ++++++
 lib/m4sugar/m4sugar.m4 |   71 ++++++++++++++++++++++++++++-------------------
 2 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ea4f4bd..1e1735a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2007-10-10  Eric Blake  <address@hidden>
 
+       Avoid some overhead from m4_defn and m4_popdef.
+       * lib/m4sugar/m4sugar.m4 (m4_defn, m4_popdef, m4_undefine): Only
+       pass on first argument, since we are documented that way.
+       (m4_for, m4_append_uniq, m4_text_wrap): Optimize out defined-ness
+       check where it is safe to do so.
+       (m4_append): Likewise, and quote the separator.
+       (m4_text_box): Likewise, and avoid regex, also be robust to
+       expansion and quadrigraphs.
+
        Another AC_DEFINE speedup.
        * lib/autoconf/general.m4 (AC_DEFINE_TRACE): Move parameter
        elision...
diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index 952794a..85f2c72 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -530,7 +530,7 @@ m4_define([m4_default],
 m4_define([m4_defn],
 [m4_ifdef([$1], [],
          [m4_fatal([$0: undefined macro: $1])])]dnl
-[m4_builtin([defn], $@)])
+[m4_builtin([defn], [$1])])
 
 
 # _m4_dumpdefs_up(NAME)
@@ -571,7 +571,7 @@ _m4_dumpdefs_down([$1])])
 m4_define([m4_popdef],
 [m4_ifdef([$1], [],
          [m4_fatal([$0: undefined macro: $1])])]dnl
-[m4_builtin([popdef], $@)])
+[m4_builtin([popdef], [$1])])
 
 
 # m4_quote(ARGS)
@@ -631,7 +631,7 @@ m4_define([m4_shift3], [m4_shift(m4_shift(m4_shift($@)))])
 m4_define([m4_undefine],
 [m4_ifdef([$1], [],
          [m4_fatal([$0: undefined macro: $1])])]dnl
-[m4_builtin([undefine], $@)])
+[m4_builtin([undefine], [$1])])
 
 
 ## -------------------------- ##
@@ -644,22 +644,23 @@ m4_define([m4_undefine],
 # Expand EXPRESSION defining VARIABLE to FROM, FROM + 1, ..., TO with
 # increments of STEP.
 # Both limits are included, and bounds are checked for consistency.
-# The algorithm is robust to indirect VARIABLE names.
+# The algorithm is robust to indirect VARIABLE names, and uses m4_builtin
+# to avoid some of the m4_defn overhead.
 m4_define([m4_for],
 [m4_pushdef([$1], m4_eval([$2]))dnl
-m4_if(m4_eval(([$3]) > m4_defn([$1])), 1,
+m4_cond([m4_eval(([$3]) > m4_builtin([defn], [$1]))], 1,
 [m4_pushdef([_m4_step], m4_eval(m4_default([$4], 1)))dnl
 m4_assert(_m4_step > 0)dnl
-_m4_for([$1], m4_eval((([$3]) - m4_defn([$1]))
-                     / _m4_step * _m4_step + m4_defn([$1])),
+_m4_for([$1], m4_eval((([$3]) - m4_builtin([defn], [$1]))
+                     / _m4_step * _m4_step + m4_builtin([defn], [$1])),
        _m4_step, [$5])],
-      m4_eval(([$3]) < m4_defn([$1])), 1,
+       [m4_eval(([$3]) < m4_builtin([defn], [$1]))], 1,
 [m4_pushdef([_m4_step], m4_eval(m4_default([$4], -1)))dnl
 m4_assert(_m4_step < 0)dnl
-_m4_for([$1], m4_eval((m4_defn([$1]) - ([$3]))
-                     / -(_m4_step) * _m4_step + m4_defn([$1])),
+_m4_for([$1], m4_eval((m4_builtin([defn], [$1]) - ([$3]))
+                     / -(_m4_step) * _m4_step + m4_builtin([defn], [$1])),
        _m4_step, [$5])],
-      [m4_pushdef(_m4_step,[])dnl
+       [m4_pushdef([_m4_step])dnl
 $5])[]dnl
 m4_popdef([_m4_step])dnl
 m4_popdef([$1])])
@@ -1680,9 +1681,11 @@ m4_defun([m4_join],
 #    => act1
 #    =>
 #    => active
+#
+# Use m4_builtin to avoid overhead of m4_defn.
 m4_define([m4_append],
 [m4_define([$1],
-          m4_ifdef([$1], [m4_defn([$1])$3])[$2])])
+          m4_ifdef([$1], [m4_builtin([defn], [$1])[$3]])[$2])])
 
 
 # m4_append_uniq(MACRO-NAME, STRING, [SEPARATOR])
@@ -1690,7 +1693,7 @@ m4_define([m4_append],
 # Like `m4_append', but append only if not yet present.
 m4_define([m4_append_uniq],
 [m4_ifdef([$1],
-         [m4_if(m4_index([$3]m4_defn([$1])[$3], [$3$2$3]), [-1],
+         [m4_if(m4_index([$3]m4_builtin([defn], [$1])[$3], [$3$2$3]), [-1],
                 [m4_append($@)])],
          [m4_append($@)])])
 
@@ -1732,38 +1735,44 @@ m4_define([m4_append_uniq],
 # which complicates it a bit.  The algorithm is otherwise stupid and simple:
 # all the words are preceded by m4_Separator which is defined to empty for
 # the first word, and then ` ' (single space) for all the others.
+#
+# The algorithm overquotes m4_Prefix1 to avoid m4_defn overhead, and bypasses
+# m4_popdef overhead with m4_builtin since no user macro expansion occurs in
+# the meantime.
 m4_define([m4_text_wrap],
 [m4_pushdef([m4_Prefix], [$2])dnl
-m4_pushdef([m4_Prefix1], m4_default([$3], [m4_Prefix]))dnl
+m4_pushdef([m4_Prefix1], m4_dquote(m4_default([$3], [m4_Prefix])))dnl
 m4_pushdef([m4_Width], m4_default([$4], 79))dnl
-m4_pushdef([m4_Cursor], m4_qlen(m4_defn([m4_Prefix1])))dnl
+m4_pushdef([m4_Cursor], m4_qlen(m4_Prefix1))dnl
 m4_pushdef([m4_Separator], [])dnl
-m4_defn([m4_Prefix1])[]dnl
-m4_cond([m4_eval(m4_qlen(m4_defn([m4_Prefix1])) > m4_len(m4_Prefix))],
+m4_Prefix1[]dnl
+m4_cond([m4_eval(m4_qlen(m4_Prefix1) > m4_len(m4_Prefix))],
        [1], [m4_define([m4_Cursor], m4_len(m4_Prefix))
 m4_Prefix],
-       [m4_eval(m4_qlen(m4_defn([m4_Prefix1])) < m4_len(m4_Prefix))],
+       [m4_eval(m4_qlen(m4_Prefix1) < m4_len(m4_Prefix))],
        [0], [],
        [m4_define([m4_Cursor], m4_len(m4_Prefix))[]dnl
 m4_format([%*s],
-         m4_max(0,m4_eval(m4_len(m4_Prefix) - m4_qlen(m4_defn([m4_Prefix1])))),
+         m4_max([0], m4_eval(m4_len(m4_Prefix) - m4_qlen(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
+[m4_define([m4_Cursor],
+          m4_eval(m4_Cursor + m4_qlen(m4_builtin([defn], [m4_Word])) + 1))dnl
 dnl New line if too long, else insert a space unless it is the first
 dnl of the words.
 m4_if(m4_eval(m4_Cursor > m4_Width),
       1, [m4_define([m4_Cursor],
-                   m4_eval(m4_len(m4_Prefix) + m4_qlen(m4_defn([m4_Word])) + 
1))]
+                   m4_eval(m4_len(m4_Prefix)
+                           + m4_qlen(m4_builtin([defn], [m4_Word])) + 1))]
 m4_Prefix,
        [m4_Separator])[]dnl
-m4_defn([m4_Word])[]dnl
+m4_builtin([defn], [m4_Word])[]dnl
 m4_define([m4_Separator], [ ])])dnl
-m4_popdef([m4_Separator])dnl
-m4_popdef([m4_Cursor])dnl
-m4_popdef([m4_Width])dnl
-m4_popdef([m4_Prefix1])dnl
-m4_popdef([m4_Prefix])dnl
+m4_builtin([popdef], [m4_Separator])dnl
+m4_builtin([popdef], [m4_Cursor])dnl
+m4_builtin([popdef], [m4_Width])dnl
+m4_builtin([popdef], [m4_Prefix1])dnl
+m4_builtin([popdef], [m4_Prefix])dnl
 ])
 
 
@@ -1775,9 +1784,13 @@ m4_popdef([m4_Prefix])dnl
 #  ## ------- ##
 # using FRAME-CHARACTER in the border.
 m4_define([m4_text_box],
address@hidden:@@%:@ m4_bpatsubst([$1], [.], m4_if([$2], [], [[-]], [[$2]])) 
@%:@@%:@
+[m4_pushdef([m4_Border],
+           m4_translit(m4_format([%*s], m4_qlen(m4_quote($1)), []),
+                       [ ], m4_if([$2], [], [[-]], [[$2]])))dnl
address@hidden:@@%:@ m4_Border @%:@@%:@
 @%:@@%:@ $1 @%:@@%:@
address@hidden:@@%:@ m4_bpatsubst([$1], [.], m4_if([$2], [], [[-]], [[$2]])) 
@%:@@%:@[]dnl
address@hidden:@@%:@ m4_Border @%:@@%:@dnl
+m4_builtin([popdef], [m4_Border])dnl
 ])
 
 
-- 
1.5.3.2







reply via email to

[Prev in Thread] Current Thread [Next in Thread]