Re: Performance issue of libtool-2.4.4

[Richard Purdie]

On Tue, 2015-02-10 at 10:53 +0000, Gary V. Vaughan wrote:
> On Feb 10, 2015, at 10:35 AM, Richard Purdie <address@hidden> wrote:
> > On Mon, 2015-02-09 at 23:36 +0000, Richard Purdie wrote:
> >> On Mon, 2015-02-09 at 13:05 +0000, Richard Purdie wrote:
> >>> In an effort to get to the bottom of this I made a git bisection, timing
> >>> the performance of building xz with make -j1 using each different
> >>> libtool.
> >>> 
> >>> The issues come down to this commit:
> >>> 
> >>> http://git.savannah.gnu.org/cgit/libtool.git/commit/?id=0a42997c6032b9550a009a271552e811bfbcc430
> >>> 
> >>> libtool: rewritten over funclib.sh instead of general.m4sh.
> >>> 
> >>> Before that, I get a time of about 20s, after it, 39s. If I cherry-pick
> >>> in the fix in master mentioned above, I get 27s.
> >>> 
> >>> So whilst things are better (thanks!), the above change is still causing
> >>> a regression in the performance somewhere else. Any ideas what else in
> >>> that rather large change may be causing this?
> >> 
> >> To further narrow this down, of the changes in the above commit, the
> >> problem appears to be in the changes to the option parsing code. I've
> >> included the diff below which if I apply on top of the above, I get the
> >> speed back. I've left the func_split_short_opt/func_split_long_opt code
> >> in there but that is worth a tiny part of the speed, the issues are
> >> around the addition of the func_options call.
> >> 
> >> As yet I don't know enough about the code in question to know why this
> >> is an issue but traces of libtool show a lot more looping in code to do
> >> with argument parsing and quoting.
> > 
> > To be more specific, if I take my "good" libtool and add:
> > 
> > func_options_prep ${1+"$@"}
> > 
> > it slows the build down by 0.5s on a 21s build. If I look at
> > func_options_prep and comment out the line:
> > 
> >    func_run_hooks func_options_prep ${1+"$@"}
> > 
> > I get the 0.5s back.
> > 
> > In func_run_hooks, if I comment:
> > 
> >    func_quote_for_eval ${1+"$@"}
> >    func_run_hooks_result=$func_quote_for_eval_result
> > 
> > I get the 0.5s back. The issue is all the quoting of the various return
> > values through all this looping. It doesn't appear to be hitting the
> > printf/sed in func_quote_for_eval which would be an obvious slow path,
> > its just the shear number of loops run through with the commandline
> > arguments. The change adds a number of calls to func_run_hooks, not just
> > the single test case I have above and all combined, it slows things down
> > significantly.
> > 
> > So is there a way we can change things so its not calling
> > func_quote_for_eval all the time with all the looping that entails?
> 
> One possibility would be to add a post-processing script that rewrites
> the hook functions used by func_options into a a single top-level blob of
> sequential shell code.  I imagine that adding some carefully chosen comment
> strings would make extracting the right parts in the right order relatively
> straight forward... it might even be that inlining func_options_prep and
> any hook functions it calls would be enough?

Thanks for the background info on this. I understand the need to change
and improve the software so I'm not proposing reverting, I do think
there has to be some way to get some of the speed here back though.

I have a bit of a pressing need to have a things performing as they were
and I'd prefer to stay on libtool 2.4.5 than revert back to 2.4.2 so I
cooked up the patch I've included below. This basically manually unrolls
the key problematic parts, cut and paste from options-parser. With this
applied to master, I have a build time of 22s compared to 20s or 21s
(need to go and retest as I'm forgetting numbers) with 2.4.2.

For now I'll probably merge something like this into the Yocto
Project/OpenEmbedded, the exact way to solve this longer term is TBD.

Cheers,

Richard




diff --git a/build-aux/ltmain.in b/build-aux/ltmain.in
index d5cf07a..0f54303 100644
--- a/build-aux/ltmain.in
+++ b/build-aux/ltmain.in
@@ -342,11 +342,15 @@ _LT_EOF
 # libtool_options_prep [ARG]...
 # -----------------------------
 # Preparation for options parsed by libtool.
-libtool_options_prep ()
-{
+#libtool_options_prep ()
+#{
     $debug_mode
 
     # Option defaults:
+    opt_verbose=false
+    opt_warning_types=
+
+    # Option defaults:
     opt_config=false
     opt_dlopen=
     opt_dry_run=false
@@ -382,19 +386,14 @@ libtool_options_prep ()
       shift; set dummy --mode uninstall ${1+"$@"}; shift
       ;;
     esac
-
-    # Pass back the list of options.
-    func_quote_for_eval ${1+"$@"}
-    libtool_options_prep_result=$func_quote_for_eval_result
-}
-func_add_hook func_options_prep libtool_options_prep
+#}
 
 
 # libtool_parse_options [ARG]...
 # ---------------------------------
 # Provide handling for libtool specific options.
-libtool_parse_options ()
-{
+#libtool_parse_options ()
+#{
     $debug_cmd
 
     # Perform our own loop to consume as many options as possible in
@@ -474,29 +473,90 @@ libtool_parse_options ()
                         func_append preserve_args " $_G_opt"
                         ;;
 
-       # An option not handled by this hook function:
-        *)             set dummy "$_G_opt" ${1+"$@"};  shift; break  ;;
-      esac
-    done
+        --debug|-x)   debug_cmd='set -x'
+                      func_echo "enabling shell trace mode"
+                      $debug_cmd
+                      ;;
 
+        --no-warnings|--no-warning|--no-warn)
+                      set dummy --warnings none ${1+"$@"}
+                      shift
+                     ;;
 
-    # save modified positional parameters for caller
-    func_quote_for_eval ${1+"$@"}
-    libtool_parse_options_result=$func_quote_for_eval_result
-}
-func_add_hook func_parse_options libtool_parse_options
+        --warnings|--warning|-W)
+                      test $# = 0 && func_missing_arg $_G_opt && break
+                      case " $warning_categories $1" in
+                        *" $1 "*)
+                          # trailing space prevents matching last $1 above
+                          func_append_uniq opt_warning_types " $1"
+                          ;;
+                        *all)
+                          opt_warning_types=$warning_categories
+                          ;;
+                        *none)
+                          opt_warning_types=none
+                          warning_func=:
+                          ;;
+                        *error)
+                          opt_warning_types=$warning_categories
+                          warning_func=func_fatal_error
+                          ;;
+                        *)
+                          func_fatal_error \
+                             "unsupported warning category: '$1'"
+                          ;;
+                      esac
+                      shift
+                      ;;
+
+        --verbose|-v) opt_verbose=: ;;
+        --version)    func_version ;;
+        -\?|-h)       func_usage ;;
+        --help)       func_help ;;
+
+       # Separate optargs to long options (plugins may need this):
+       --*=*)        func_split_equals "$_G_opt"
+                     set dummy "$func_split_equals_lhs" \
+                          "$func_split_equals_rhs" ${1+"$@"}
+                      shift
+                      ;;
+
+       # Separate optargs to short options:
+        -W*)
+                      func_split_short_opt "$_G_opt"
+                      set dummy "$func_split_short_opt_name" \
+                          "$func_split_short_opt_arg" ${1+"$@"}
+                      shift
+                      ;;
+
+        # Separate non-argument short options:
+        -\?*|-h*|-v*|-x*)
+                      func_split_short_opt "$_G_opt"
+                      set dummy "$func_split_short_opt_name" \
+                          "-$func_split_short_opt_arg" ${1+"$@"}
+                      shift
+                      ;;
+
+        --)           break ;;
+        -*)           func_fatal_help "unrecognised option: '$_G_opt'" ;;
+        *)            set dummy "$_G_opt" ${1+"$@"}; shift; break ;;
+      esac
+    done
+#}
 
+# Display all warnings if -W was not given.
+test -n "$opt_warning_types" || opt_warning_types=" $warning_categories"
 
 
 # libtool_validate_options [ARG]...
 # ---------------------------------
 # Perform any sanity checks on option settings and/or unconsumed
 # arguments.
-libtool_validate_options ()
-{
+#libtool_validate_options ()
+#{
     # save first non-option argument
     if test 0 -lt $#; then
-      nonopt=$1
+      nonopt=$_G_opt
       shift
     fi
 
@@ -537,20 +597,10 @@ libtool_validate_options ()
       generic_help=$help
       help="Try '$progname --help --mode=$opt_mode' for more information."
     }
+#}
 
-    # Pass back the unparsed argument list
-    func_quote_for_eval ${1+"$@"}
-    libtool_validate_options_result=$func_quote_for_eval_result
-}
-func_add_hook func_validate_options libtool_validate_options
-
-
-# Process options as early as possible so that --help and --version
-# can return quickly.
-func_options ${1+"$@"}
-eval set dummy "$func_options_result"; shift
-
-
+# Bail if the options were screwed!
+$exit_cmd $EXIT_FAILURE
 
 ## ----------- ##
 ##    Main.    ##