Re: branch-1_4 reduce memory footprint

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 10/10/2006 10:34 AM:
> At any rate, here is the patch for the branch, and I will be porting it to 
> head.  Also on head, I may experiment with adding an obstack_regrow macro 
> that 
> I documented in the bug-m4 list.

Well, I sat on porting that one to head longer than I should.  For now, I
just provide obstack_regrow in m4's system.h, but I will try making a case
for it with the glibc maintainers (the last time I tried to add an API to
glibc, verror/verror_at_line, I was flat out rejected).

This also fixes one other hammering of malloc - in the refactoring of
tokens into m4_symbol_values, where the token name was no longer stored in
the symbol value, Gary had added an xstrdup on every text token, so that
tracing would not reference stale memory on the token stack.  This patch
undoes the xstrdup, by copying to the argc stack instead.  It may still be
possible to find a way to share the token stack and argc stack, to avoid
having to even copy token names from one obstack to the other, but that is
analysis to perform at some later day.

2006-10-25  Eric Blake  <address@hidden>

        * m4/system_.h (obstack_regrow): New macro.  Hopefully glibc will
        accept it someday.
        * m4/macro.c (expand_macro): Avoid referencing invalid memory.
        Handle nesting the argc obstack.
        (expand_token): Avoid unnecessary malloc.
        (collect_arguments): Copy name before invalidating it.
        (argc_stack, argv_stack): New variables.
        (m4_macro_expand_input): Initialize argc and argv stacks once per
        file, instead of once per macro.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFP1ye84KuGfSFAYARAl0pAKCCSmgP7CqmdT+qse/xvYTZ8EyorgCgggdk
j8QGPdlOu3LBN5vL16OFJXQ=
=Ooyy
-----END PGP SIGNATURE-----

Index: m4/system_.h
===================================================================
RCS file: /sources/m4/m4/m4/system_.h,v
retrieving revision 1.17
diff -u -p -r1.17 system_.h
--- m4/system_.h        19 Oct 2006 13:38:46 -0000      1.17
+++ m4/system_.h        25 Oct 2006 12:33:29 -0000
@@ -42,6 +42,14 @@
 #include <gnu/xalloc.h>
 #include <gnu/xstrndup.h>
 
+/* glibc's obstack left out the ability to suspend and resume growth
+   of an object on the stack.  Reopen OBJECT (previously returned by
+   obstack_alloc or obstack_finish) with SIZE for additional growth,
+   freeing all objects that occur later in the stack.  */
+#define obstack_regrow(OBS, OBJECT, SIZE)               \
+  (obstack_free (OBS, (char *)(OBJECT) + SIZE),         \
+   (OBS)->object_base = (char *)(OBJECT))
+
 /* In addition to EXIT_SUCCESS and EXIT_FAILURE, m4 can fail with version
    mismatch when trying to load a frozen file produced by a newer m4 than
    the version doing the reload.  */
Index: m4/macro.c
===================================================================
RCS file: /sources/m4/m4/m4/macro.c,v
retrieving revision 1.59
diff -u -p -r1.59 macro.c
--- m4/macro.c  12 Oct 2006 21:14:50 -0000      1.59
+++ m4/macro.c  25 Oct 2006 12:33:30 -0000
@@ -55,6 +55,19 @@ static size_t expansion_level = 0;
 /* The number of the current call of expand_macro ().  */
 static size_t macro_call_id = 0;
 
+/* The shared stack of collected arguments for macro calls; as each
+   argument is collected, it is finished and its location stored in
+   argv_stack.  This stack can be used simultaneously by multiple
+   macro calls, using obstack_regrow to handle partial objects
+   embedded in the stack.  */
+static struct obstack argc_stack;
+
+/* The shared stack of pointers to collected arguments for macro
+   calls.  This object is never finished; we exploit the fact that
+   obstack_blank is documented to take a negative size to reduce the
+   size again.  */
+static struct obstack argv_stack;
+
 /* This function reads all input, and expands each token, one at a time.  */
 void
 m4_macro_expand_input (m4 *context)
@@ -62,8 +75,14 @@ m4_macro_expand_input (m4 *context)
   m4__token_type type;
   m4_symbol_value token;
 
+  obstack_init (&argc_stack);
+  obstack_init (&argv_stack);
+
   while ((type = m4__next_token (context, &token)) != M4_TOKEN_EOF)
     expand_token (context, (m4_obstack *) NULL, type, &token);
+
+  obstack_free (&argc_stack, NULL);
+  obstack_free (&argv_stack, NULL);
 }
 
 
@@ -76,10 +95,8 @@ expand_token (m4 *context, m4_obstack *o
              m4__token_type type, m4_symbol_value *token)
 {
   m4_symbol *symbol;
-  /* Copy name, since expand_macro can consume additional tokens,
-     invalidating the current token.  */
   char *text = (m4_is_symbol_value_text (token)
-               ? xstrdup (m4_get_symbol_value_text (token)) : NULL);
+               ? m4_get_symbol_value_text (token) : NULL);
 
   switch (type)
     {                          /* TOKSW */
@@ -121,8 +138,6 @@ expand_token (m4 *context, m4_obstack *o
       assert (!"INTERNAL ERROR: bad token type in expand_token ()");
       abort ();
     }
-
-  free (text);
 }
 
 
@@ -214,12 +229,17 @@ expand_argument (m4 *context, m4_obstack
    Expand_macro () uses call_macro () to do the call of the macro.
 
    Expand_macro () is potentially recursive, since it calls expand_argument
-   (), which might call expand_token (), which might call expand_macro ().  */
+   (), which might call expand_token (), which might call expand_macro ().
+
+   NAME points to storage on the token stack, so it is only valid
+   until a call to collect_arguments parses more tokens.  SYMBOL is
+   the result of the symbol table lookup on NAME.  */
 static void
 expand_macro (m4 *context, const char *name, m4_symbol *symbol)
 {
-  m4_obstack arguments;
-  m4_obstack argptr;
+  char *argc_base;             /* Base of argc_stack on entry.  */
+  unsigned int argc_size;      /* Size of argc_stack on entry.  */
+  unsigned int argv_size;      /* Size of argv_stack on entry.  */
   m4_symbol_value **argv;
   int argc;
   m4_obstack *expansion;
@@ -261,16 +281,22 @@ recursion limit of %d exceeded, use -L<N
   macro_call_id++;
   my_call_id = macro_call_id;
 
-  obstack_init (&argptr);
-  obstack_init (&arguments);
+  argc_size = obstack_object_size (&argc_stack);
+  argv_size = obstack_object_size (&argv_stack);
+  if (0 < argc_size)
+    argc_base = obstack_finish (&argc_stack);
 
   if (traced && m4_is_debug_bit (context, M4_DEBUG_TRACE_CALL))
     trace_prepre (context, name, my_call_id, value);
 
-  collect_arguments (context, name, symbol, &argptr, &arguments);
+  collect_arguments (context, name, symbol, &argv_stack, &argc_stack);
 
-  argc = obstack_object_size (&argptr) / sizeof (m4_symbol_value *);
-  argv = (m4_symbol_value **) obstack_finish (&argptr);
+  argc = ((obstack_object_size (&argv_stack) - argv_size)
+         / sizeof (m4_symbol_value *));
+  argv = (m4_symbol_value **) (obstack_base (&argv_stack) + argv_size);
+  /* Calling collect_arguments invalidated name, but we copied it as
+     argv[0].  */
+  name = m4_get_symbol_value_text (argv[0]);
 
   loc_close_file = m4_get_current_file (context);
   loc_close_line = m4_get_current_line (context);
@@ -296,8 +322,11 @@ recursion limit of %d exceeded, use -L<N
   if (BIT_TEST (VALUE_FLAGS (value), VALUE_DELETED_BIT))
     m4_symbol_value_delete (value);
 
-  obstack_free (&arguments, NULL);
-  obstack_free (&argptr, NULL);
+  if (0 < argc_size)
+    obstack_regrow (&argc_stack, argc_base, argc_size);
+  else
+    obstack_free (&argc_stack, argv[0]);
+  obstack_blank (&argv_stack, -argc * sizeof (m4_symbol_value *));
 }
 
 /* Collect all the arguments to a call of the macro SYMBOL (called NAME).
@@ -314,10 +343,10 @@ collect_arguments (m4 *context, const ch
 
   groks_macro_args = BIT_TEST (SYMBOL_FLAGS (symbol), VALUE_MACRO_ARGS_BIT);
 
-  m4_set_symbol_value_text (&token, (char *) name);
-  tokenp = (m4_symbol_value *) obstack_copy (arguments, (void *) &token,
-                                     sizeof (token));
-  obstack_grow (argptr, (void *) &tokenp, sizeof (tokenp));
+  tokenp = (m4_symbol_value *) obstack_alloc (arguments, sizeof *tokenp);
+  m4_set_symbol_value_text (tokenp, (char *) obstack_copy0 (arguments, name,
+                                                           strlen (name)));
+  obstack_ptr_grow (argptr, tokenp);
 
   if (m4__next_token_is_open (context))
     {
@@ -330,9 +359,9 @@ collect_arguments (m4 *context, const ch
            {
              m4_set_symbol_value_text (&token, "");
            }
-         tokenp = (m4_symbol_value *)
-           obstack_copy (arguments, (void *) &token, sizeof (token));
-         obstack_grow (argptr, (void *) &tokenp, sizeof (tokenp));
+         tokenp = (m4_symbol_value *) obstack_copy (arguments, &token,
+                                                    sizeof token);
+         obstack_ptr_grow (argptr, tokenp);
        }
       while (more_args);
     }