bug-bash
[Top][All Lists]
Advanced

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

Bash 2.05 performance speedup for unwind_protect_var


From: Paul Eggert
Subject: Bash 2.05 performance speedup for unwind_protect_var
Date: Thu, 3 May 2001 12:31:01 -0700 (PDT)

Configuration Information [Automatically generated, do not change]:
Machine: sparc
OS: solaris2.7
Compiler: cc -xarch=v9 -xO2
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='sparc' 
-DCONF_OSTYPE='solaris2.7' -DCONF_MACHTYPE='sparc-sun-solaris2.7' 
-DCONF_VENDOR='sun' -DSHELL  -DHAVE_CONFIG_H   -I.  -I.. -I../include -I../lib 
-I/usr/local/include -g
uname output: SunOS sic.twinsun.com 5.7 Generic_106541-15 sun4u sparc 
SUNW,UltraSPARC-IIi-Engine
Machine Type: sparc-sun-solaris2.7

Bash Version: 2.05
Patch Level: 0
Release Status: release

Description:
        While thinking about the unwind-protect bug I sent in a patch
        for yesterday, I thought of a better way to do unwind protection.
        This method avoids portability problems inherent to Bash's current
        way of doing things, and it also speeds up Bash by about 11%
        on my host, when running on the following little benchmark:

        j=0
        for ((i=0; i < 10000; i++))
        do eval "((j=$j+$i*$i))"
        done
        echo $j

Repeat-By:
        Find a host where sizeof (int) < sizeof (pid_t); then
        Bash will handle process-IDs when unwind-protecting them.
        POSIX allows such hosts, though I don't know of one offhand.

Fix:

2001-05-03  Paul Eggert  <eggert@twinsun.com>

        Unwind_protect_var cleanup and performance enhancement.  This
        sped things up by 11% on my 64-bit Solaris sparc host on a
        little arithmetic benchmark.

        * unwind_prot.h (begin_unwind_frame, discard_unwind_frame,
        run_unwind_frame, remove_unwind_protect, run_unwind_protects,
        clear_unwind_protect_list): Declare with prototypes.
        (unwind_protect_int, unwind_protect_short, unwind_protect_string,
        unwind_protect_pointer, unwind_protect_jmp_buf): Remove; now subsumed
        by unwind_protect_var.  All callers changed to use unwind_protect_var.
        (unwind_protect_mem): Renamed from unwind_protect_var.  Do not accept
        a value as a separate arg; just take it from the var.
        (unwind_protect_var): New macro.

        * unwind_prot.c: Include <stddef.h> if STDC_HEADERS.
        (offsetof): New macro, if <stddef.h> does not define it.
        (SAVED_VAR): Use struct hack to store desired setting; this avoids
        an extra malloc/free.
        (UNWIND_ELT): Use union hack to store saved var; this avoids another
        extra malloc/free.
        (without_interrupts, unwind_frame_discard_internal,
        unwind_frame_run_internal, add_unwind_protect_internal,
        remove_unwind_protect_internal, run_unwind_protects_internal,
        clear_unwind_protects_internal, restore_variable,
        unwind_protect_mem_internal): Declare with prototypes.
        Use struct and/or union hack to avoid extra mallocs.
        (run_unwind_protects_internal):
        Implement using unwind_frame_run_internal.
        (restore_variable): No need to free, or for a special case for int.
        (unwind_frame_run_internal): Restore variable ourselves if the cleanup
        function is restore_variable.
        (discard_saved_var): Remove; no longer used.
        (unwind_protect_mem_internal): New function.
        (unwind_protect_mem): Renamed from unwind_protect_var.  Do not accept
        a value as a separate arg; just take it from the var.
        Use unwind_protect_mem_internal to do the real work.

        * builtins/bind.def (bind_builtin): Use unwind_protect_var on FILE*;
        it works for arbitrary object types.

        * jobs.c (run_sigchld_trap): Remove special case for pid_t,
        now that unwind_protect_var works on arbitrary object types.

===================================================================
RCS file: builtins/bind.def,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/bind.def   2001/02/14 22:07:33     2.5
+++ builtins/bind.def   2001/05/03 19:02:50     2.5.0.1
@@ -102,7 +102,6 @@ bind_builtin (list)
      WORD_LIST *list;
 {
   int return_code;
-  FILE *old_rl_outstream;
   Keymap kmap, saved_keymap;
   int flags, opt;
   char *initfile, *map_name, *fun_name, *unbind_name, *remove_seq, *cmd_seq;
@@ -118,10 +117,8 @@ bind_builtin (list)
   if (!bash_readline_initialized)
     initialize_readline ();
 
-  /* Cannot use unwind_protect_pointer () on "FILE *", it is only
-     guaranteed to work for strings. */
-  /* XXX -- see if we can use unwind_protect here */
-  old_rl_outstream = rl_outstream;
+  begin_unwind_frame ("bind builtin");
+  unwind_protect_var (rl_outstream);
   rl_outstream = stdout;
 
   reset_internal_getopt ();  
@@ -263,7 +260,8 @@ bind_builtin (list)
   if (saved_keymap)
     rl_set_keymap (saved_keymap);
 
-  rl_outstream = old_rl_outstream;
+  run_unwind_frame ("bind builtin");
+
   return (return_code);
 }
 
===================================================================
RCS file: builtins/evalfile.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/evalfile.c 2001/02/14 22:05:16     2.5
+++ builtins/evalfile.c 2001/05/03 19:02:50     2.5.0.1
@@ -154,11 +154,11 @@ file_error_and_exit:
     {
       begin_unwind_frame ("_evalfile");
 
-      unwind_protect_int (return_catch_flag);
-      unwind_protect_jmp_buf (return_catch);
+      unwind_protect_var (return_catch_flag);
+      unwind_protect_var (return_catch);
       if (flags & FEVAL_NONINT)
-       unwind_protect_int (interactive);
-      unwind_protect_int (sourcelevel);
+       unwind_protect_var (interactive);
+      unwind_protect_var (sourcelevel);
     }
   else
     {
===================================================================
RCS file: builtins/evalstring.c,v
retrieving revision 2.5.0.1
retrieving revision 2.5.0.2
diff -pu -r2.5.0.1 -r2.5.0.2
--- builtins/evalstring.c       2001/04/13 08:30:59     2.5.0.1
+++ builtins/evalstring.c       2001/05/03 19:06:02     2.5.0.2
@@ -103,19 +103,19 @@ parse_and_execute (string, from_file, fl
   orig_string = string;
   /* Unwind protect this invocation of parse_and_execute (). */
   begin_unwind_frame ("parse_and_execute_top");
-  unwind_protect_int (parse_and_execute_level);
-  unwind_protect_jmp_buf (top_level);
-  unwind_protect_int (indirection_level);
-  unwind_protect_int (line_number);
+  unwind_protect_var (parse_and_execute_level);
+  unwind_protect_var (top_level);
+  unwind_protect_var (indirection_level);
+  unwind_protect_var (line_number);
   if (flags & (SEVAL_NONINT|SEVAL_INTERACT))
-    unwind_protect_int (interactive);
+    unwind_protect_var (interactive);
 
 #if defined (HISTORY)
-  unwind_protect_int (remember_on_history);    /* can be used in scripts */
+  unwind_protect_var (remember_on_history);    /* can be used in scripts */
 #  if defined (BANG_HISTORY)
   if (interactive_shell)
     {
-      unwind_protect_int (history_expansion_inhibited);
+      unwind_protect_var (history_expansion_inhibited);
     }
 #  endif /* BANG_HISTORY */
 #endif /* HISTORY */
===================================================================
RCS file: builtins/fc.def,v
retrieving revision 2.5.0.1
retrieving revision 2.5.0.2
diff -pu -r2.5.0.1 -r2.5.0.2
--- builtins/fc.def     2001/05/01 01:00:09     2.5.0.1
+++ builtins/fc.def     2001/05/03 19:06:02     2.5.0.2
@@ -381,7 +381,7 @@ fc_builtin (list)
   begin_unwind_frame ("fc builtin");
   add_unwind_protect ((Function *)xfree, fn);
   add_unwind_protect (unlink, fn);
-  unwind_protect_int (echo_input_at_read);
+  unwind_protect_var (echo_input_at_read);
   echo_input_at_read = 1;
     
   retval = fc_execute_file (fn);
===================================================================
RCS file: builtins/read.def,v
retrieving revision 2.5.0.2
retrieving revision 2.5.0.3
diff -pu -r2.5.0.2 -r2.5.0.3
--- builtins/read.def   2001/05/01 01:13:01     2.5.0.2
+++ builtins/read.def   2001/05/03 19:06:02     2.5.0.3
@@ -282,7 +282,7 @@ read_builtin (list)
        {
          if (nchars > 0)
            {
-             unwind_protect_int (rl_num_chars_to_read);
+             unwind_protect_var (rl_num_chars_to_read);
              rl_num_chars_to_read = nchars;
            }
          if (delim != '\n')
===================================================================
RCS file: execute_cmd.c,v
retrieving revision 2.5.0.6
retrieving revision 2.5.0.7
diff -pu -r2.5.0.6 -r2.5.0.7
--- execute_cmd.c       2001/05/03 07:03:48     2.5.0.6
+++ execute_cmd.c       2001/05/03 19:06:02     2.5.0.7
@@ -1886,8 +1886,8 @@ execute_select_command (select_command)
 
   retval = EXECUTION_SUCCESS;
 
-  unwind_protect_int (return_catch_flag);
-  unwind_protect_jmp_buf (return_catch);
+  unwind_protect_var (return_catch_flag);
+  unwind_protect_var (return_catch);
   return_catch_flag++;
 
   while (1)
@@ -2711,7 +2711,7 @@ execute_builtin (builtin, words, flags, 
   if (subshell == 0 && builtin == eval_builtin && (flags & CMD_IGNORE_RETURN))
     {
       begin_unwind_frame ("eval_builtin");
-      unwind_protect_int (exit_immediately_on_error);
+      unwind_protect_var (exit_immediately_on_error);
       exit_immediately_on_error = 0;
       eval_unwind = 1;
     }
@@ -2794,12 +2794,12 @@ execute_function (var, words, flags, fds
       begin_unwind_frame ("function_calling");
       push_context ();
       add_unwind_protect (pop_context, (char *)NULL);
-      unwind_protect_int (line_number);
-      unwind_protect_int (return_catch_flag);
-      unwind_protect_jmp_buf (return_catch);
+      unwind_protect_var (line_number);
+      unwind_protect_var (return_catch_flag);
+      unwind_protect_var (return_catch);
       add_unwind_protect (dispose_command, (char *)tc);
-      unwind_protect_pointer (this_shell_function);
-      unwind_protect_int (loop_level);
+      unwind_protect_var (this_shell_function);
+      unwind_protect_var (loop_level);
     }
 
   this_shell_function = var;
===================================================================
RCS file: jobs.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- jobs.c      2001/03/26 18:08:24     2.5
+++ jobs.c      2001/05/03 19:06:02     2.5.0.1
@@ -2587,15 +2587,12 @@ run_sigchld_trap (nchild)
   trap_command = savestring (trap_list[SIGCHLD]);
 
   begin_unwind_frame ("SIGCHLD trap");
-  unwind_protect_int (last_command_exit_value);
-  if (sizeof (pid_t) == sizeof (short))
-    unwind_protect_short (last_made_pid);
-  else
-    unwind_protect_int (last_made_pid);
-  unwind_protect_int (interrupt_immediately);
-  unwind_protect_int (jobs_list_frozen);
-  unwind_protect_pointer (the_pipeline);
-  unwind_protect_pointer (subst_assign_varlist);
+  unwind_protect_var (last_command_exit_value);
+  unwind_protect_var (last_made_pid);
+  unwind_protect_var (interrupt_immediately);
+  unwind_protect_var (jobs_list_frozen);
+  unwind_protect_var (the_pipeline);
+  unwind_protect_var (subst_assign_varlist);
 
   /* We have to add the commands this way because they will be run
      in reverse order of adding.  We don't want maybe_set_sigchld_trap ()
===================================================================
RCS file: unwind_prot.c,v
retrieving revision 2.5.0.1
retrieving revision 2.5.0.2
diff -pu -r2.5.0.1 -r2.5.0.2
--- unwind_prot.c       2001/05/03 01:55:33     2.5.0.1
+++ unwind_prot.c       2001/05/03 19:06:02     2.5.0.2
@@ -33,37 +33,53 @@ Foundation, 59 Temple Place, Suite 330, 
 #  include <unistd.h>
 #endif
 
+#if STDC_HEADERS
+# include <stddef.h>
+#endif
+#ifndef offsetof
+# define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+#endif
+
 #include "command.h"
 #include "general.h"
 #include "unwind_prot.h"
 #include "quit.h"
 #include "sig.h"
 
-/* If CLEANUP is null, then ARG contains a tag to throw back to. */
-typedef struct _uwp {
-  struct _uwp *next;
-  Function *cleanup;
-  char *arg;
-} UNWIND_ELT;
-
-/* Structure describing a saved variable and the value to restore it to.
-   If a cleanup function is set to restore_variable, the `arg' pointer
-   points to this. */
+/* Structure describing a saved variable and the value to restore it to.  */
 typedef struct {
-  int *variable;
-  char *desired_setting;
+  char *variable;
   int size;
+  char desired_setting[1]; /* actual size is `size' */
 } SAVED_VAR;
 
-static void without_interrupts ();
-static void unwind_frame_discard_internal ();
-static void unwind_frame_run_internal ();
-static void add_unwind_protect_internal ();
-static void remove_unwind_protect_internal ();
-static void run_unwind_protects_internal ();
-static void clear_unwind_protects_internal ();
-static void restore_variable ();
-static void discard_saved_var ();
+/* If HEAD.CLEANUP is null, then ARG.V contains a tag to throw back to.
+   If HEAD.CLEANUP is restore_variable, then SV.V contains the saved
+   variable.  Otherwise, call HEAD.CLEANUP (ARG.V) to clean up.  */
+typedef union uwp {
+  struct uwp_head {
+    union uwp *next;
+    Function *cleanup;
+  } head;
+  struct {
+    struct uwp_head uwp_head;
+    char *v;
+  } arg;
+  struct {
+    struct uwp_head uwp_head;
+    SAVED_VAR v;
+  } sv;
+} UNWIND_ELT;
+
+static void without_interrupts __P((VFunction *, char *, char *));
+static void unwind_frame_discard_internal __P((char *, char *));
+static void unwind_frame_run_internal __P((char *, char *));
+static void add_unwind_protect_internal __P((Function *, char *));
+static void remove_unwind_protect_internal __P((char *, char *));
+static void run_unwind_protects_internal __P((char *, char *));
+static void clear_unwind_protects_internal __P((char *, char *));
+static inline void restore_variable __P((SAVED_VAR *));
+static void unwind_protect_mem_internal __P((char *, char *));
 
 static UNWIND_ELT *unwind_protect_list = (UNWIND_ELT *)NULL;
 
@@ -164,9 +180,9 @@ add_unwind_protect_internal (cleanup, ar
   UNWIND_ELT *elt;
 
   elt = (UNWIND_ELT *)xmalloc (sizeof (UNWIND_ELT));
-  elt->cleanup = cleanup;
-  elt->arg = arg;
-  elt->next = unwind_protect_list;
+  elt->head.next = unwind_protect_list;
+  elt->head.cleanup = cleanup;
+  elt->arg.v = arg;
   unwind_protect_list = elt;
 }
 
@@ -179,9 +195,7 @@ remove_unwind_protect_internal (ignore1,
   elt = unwind_protect_list;
   if (elt)
     {
-      unwind_protect_list = unwind_protect_list->next;
-      if (elt->cleanup && elt->cleanup == (Function *)restore_variable)
-       discard_saved_var ((SAVED_VAR *)elt->arg);
+      unwind_protect_list = unwind_protect_list->head.next;
       free (elt);
     }
 }
@@ -190,22 +204,7 @@ static void
 run_unwind_protects_internal (ignore1, ignore2)
      char *ignore1, *ignore2;
 {
-  UNWIND_ELT *t, *elt = unwind_protect_list;
-
-  while (elt)
-   {
-      /* This function can be run at strange times, like when unwinding
-        the entire world of unwind protects.  Thus, we may come across
-        an element which is simply a label for a catch frame.  Don't call
-        the non-existant function. */
-      if (elt->cleanup)
-       (*(elt->cleanup)) (elt->arg);
-
-      t = elt;
-      elt = elt->next;
-      free (t);
-    }
-  unwind_protect_list = elt;
+  unwind_frame_run_internal ((char *) NULL, (char *) NULL);
 }
 
 static void
@@ -231,22 +230,27 @@ unwind_frame_discard_internal (tag, igno
 
   while (elt = unwind_protect_list)
     {
-      unwind_protect_list = unwind_protect_list->next;
-      if (elt->cleanup == 0 && (STREQ (elt->arg, tag)))
+      unwind_protect_list = unwind_protect_list->head.next;
+      if (elt->head.cleanup == 0 && (STREQ (elt->arg.v, tag)))
        {
          free (elt);
          break;
        }
-      else if (elt->cleanup && elt->cleanup == (Function *)restore_variable)
-       {
-         discard_saved_var ((SAVED_VAR *)elt->arg);
-         free (elt);
-       }
       else
        free (elt);
     }
 }
 
+/* Restore the value of a variable, based on the contents of SV.
+   sv->desired_setting is a block of memory SIZE bytes long holding the
+   value itself.  This block of memory is copied back into the variable. */
+static inline void
+restore_variable (sv)
+     SAVED_VAR *sv;
+{
+  FASTCOPY (sv->desired_setting, sv->variable, sv->size);
+}
+
 static void
 unwind_frame_run_internal (tag, ignore)
      char *tag, *ignore;
@@ -255,80 +259,52 @@ unwind_frame_run_internal (tag, ignore)
 
   while (elt = unwind_protect_list)
     {
-      unwind_protect_list = elt->next;
+      unwind_protect_list = elt->head.next;
 
       /* If tag, then compare. */
-      if (!elt->cleanup)
+      if (!elt->head.cleanup)
        {
-         if (STREQ (elt->arg, tag))
+         if (tag && STREQ (elt->arg.v, tag))
            {
              free (elt);
              break;
            }
-         free (elt);
-         continue;
        }
       else
        {
-         (*(elt->cleanup)) (elt->arg);
-         free (elt);
+         if (elt->head.cleanup == (Function *) restore_variable)
+           restore_variable (&elt->sv.v);
+         else
+           (*(elt->head.cleanup)) (elt->arg.v);
        }
-    }
-}
 
-static void
-discard_saved_var (sv)
-     SAVED_VAR *sv;
-{
-  if (sv->size != sizeof (int))
-    free (sv->desired_setting);
-  free (sv);
+      free (elt);
+    }
 }
 
-/* Restore the value of a variable, based on the contents of SV.  If
-   sv->size is greater than sizeof (int), sv->desired_setting points to
-   a block of memory SIZE bytes long holding the value, rather than the
-   value itself.  This block of memory is copied back into the variable. */
 static void
-restore_variable (sv)
-     SAVED_VAR *sv;
-{
-  if (sv->size != sizeof (int))
-    {
-      FASTCOPY ((char *)sv->desired_setting, (char *)sv->variable, sv->size);
-      free (sv->desired_setting);
-    }
-  else
-    {
-      UWP u;
-      u.s = sv->desired_setting;
-      *(sv->variable) = u.i;
-    }
-
-  free (sv);
+unwind_protect_mem_internal (var, psize)
+     char *var;
+     char *psize;
+{
+  int size = *(int *) psize;
+  int allocated = size + offsetof (UNWIND_ELT, sv.v.desired_setting[0]);
+  UNWIND_ELT *elt = (UNWIND_ELT *) xmalloc (allocated);
+  elt->head.next = unwind_protect_list;
+  elt->head.cleanup = (Function *) restore_variable;
+  elt->sv.v.variable = var;
+  elt->sv.v.size = size;
+  FASTCOPY (var, elt->sv.v.desired_setting, size);
+  unwind_protect_list = elt;
 }
 
 /* Save the value of a variable so it will be restored when unwind-protects
-   are run.  VAR is a pointer to the variable.  VALUE is the value to be
-   saved.  SIZE is the size in bytes of VALUE.  If SIZE is bigger than what
-   can be saved in an int, memory will be allocated and the value saved
-   into that using bcopy (). */
+   are run.  VAR is a pointer to the variable.  SIZE is the size in
+   bytes of VAR.  */
 void
-unwind_protect_var (var, value, size)
-     int *var;
-     char *value;
+unwind_protect_mem (var, size)
+     char *var;
      int size;
 {
-  SAVED_VAR *s = (SAVED_VAR *)xmalloc (sizeof (SAVED_VAR));
-
-  s->variable = var;
-  if (size != sizeof (int))
-    {
-      s->desired_setting = (char *)xmalloc (size);
-      FASTCOPY (value, (char *)s->desired_setting, size);
-    }
-  else
-    s->desired_setting = value;
-  s->size = size;
-  add_unwind_protect ((Function *)restore_variable, (char *)s);
+  without_interrupts (unwind_protect_mem_internal, var, (char *) &size);
 }
===================================================================
RCS file: unwind_prot.h,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- unwind_prot.h       2001/02/01 18:51:00     2.5
+++ unwind_prot.h       2001/05/03 19:06:02     2.5.0.1
@@ -22,49 +22,19 @@
 #define _UNWIND_PROT_H
 
 /* Run a function without interrupts. */
-extern void begin_unwind_frame ();
-extern void discard_unwind_frame ();
-extern void run_unwind_frame ();
-extern void add_unwind_protect ();
-extern void remove_unwind_protect ();
-extern void run_unwind_protects ();
-extern void unwind_protect_var ();
-extern void clear_unwind_protect_list ();
-
-/* Try to force correct alignment on machines where pointers and ints
-   differ in size. */
-typedef union {
-  char *s;
-  int i;
-} UWP;
+extern void begin_unwind_frame __P((char *));
+extern void discard_unwind_frame __P((char *));
+extern void run_unwind_frame __P((char *));
+extern void add_unwind_protect (); /* Not portable to arbitrary C99 hosts.  */
+extern void remove_unwind_protect __P((void));
+extern void run_unwind_protects __P((void));
+extern void clear_unwind_protect_list __P((int));
 
 /* Define for people who like their code to look a certain way. */
 #define end_unwind_frame()
 
-/* How to protect an integer. */
-#define unwind_protect_int(X) \
-       do \
-         { \
-           UWP u; \
-           u.i = (X); \
-           unwind_protect_var (&(X), u.s, sizeof (int)); \
-         } \
-       while (0)
-
-#define unwind_protect_short(X) \
-  unwind_protect_var ((int *)&(X), (char *)&(X), sizeof (short))
-
-/* How to protect a pointer to a string. */
-#define unwind_protect_string(X) \
-  unwind_protect_var ((int *)&(X), \
-                     ((sizeof (char *) == sizeof (int)) ? (char *) (X) : (char 
*) &(X)), \
-                      sizeof (char *))
-
-/* How to protect any old pointer. */
-#define unwind_protect_pointer(X) unwind_protect_string (X)
-
-/* How to protect the contents of a jmp_buf. */
-#define unwind_protect_jmp_buf(X) \
-  unwind_protect_var ((int *)(X), (char *)(X), sizeof (procenv_t))
+/* How to protect a variable.  */
+#define unwind_protect_var(X) unwind_protect_mem ((char *)&(X), sizeof (X))
+extern void unwind_protect_mem __P((char *, int));
 
 #endif /* _UNWIND_PROT_H */



reply via email to

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