Re: No need for object file removal before compilation

2008-04-12 Thread Ralf Wildenhues 
OK, this issue is much more intricate than it looked like at first.

First, the existing code had some bugs:
- it removed files too often.  After the first compile, we *know* that
  there are no files to stumble over; if the compiler doesn't know -c
  -o, then we have moved away $output_obj ourselves.

- if the compiler doesn't grok '-c -o', it removed files when it may not
  remove them: the lockfile and the $output_obj may only be removed with
  a trap once we've obtained the lock.  Further, if $obj = $output_obj,
  then of course the same holds for $obj.  Yuck.
  OTOH, once we've obtained the lock, we should install that trap ASAP,
  so that we don't leave a lockfile when interrupted.

Then, while I have some doubts about the practical applicability of
Bob's scenario, I left the code so that it would be supported: all
files we cause to be created are unlinked beforehand, except for the
eventual lock file of course.

I found another very tiny race:  At the very end of func_mode_compile,
we remove the eventual lockfile before exiting.  If the user happens to
interrupt libtool after that, but before exiting, our trap will remove,
among others, $output_obj.  But that file may already be used by another
libtool instance, it could've grabbed the lock already.  Fixed by
updating $removlist before removing the lockfile.

 I would have been inclined to ok this one.

OK for the updated one?  It makes for 5% faster
  libtool --mode=compile gcc -c a.c

Thanks,
Ralf

2008-04-12  Ralf Wildenhues  [EMAIL PROTECTED]

* libltdl/config/ltmain.m4sh (func_mode_compile): Avoid
redundant removal of old output files before compilation.
Do not remove $obj until we have successfully grabbed the
lockfile (in case the compiler doesn't grok `-c -o'), because
it might be identical to $output_obj.
At the end of the function, before we remove the lockfile,
update $removelist so that if the trap hits after the lockfile
has been removed, we do not accidentally remove $output_obj that
does not belong to us.
(func_write_libtool_object): Use $MV instead of mv.

Index: libltdl/config/ltmain.m4sh
===
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.106
diff -u -r1.106 ltmain.m4sh
--- libltdl/config/ltmain.m4sh  11 Apr 2008 17:21:55 -  1.106
+++ libltdl/config/ltmain.m4sh  12 Apr 2008 06:55:46 -
@@ -650,7 +650,7 @@
 non_pic_object=$write_oldobj
 
 EOF
-  mv -f ${write_libobj}T ${write_libobj}
+  $MV ${write_libobj}T ${write_libobj}
 }
 }
 
@@ -831,9 +831,6 @@
   removelist=$lobj $libobj ${libobj}T
 fi
 
-$opt_dry_run || $RM $removelist
-trap $opt_dry_run || $RM $removelist; exit $EXIT_FAILURE 1 2 15
-
 # On Cygwin there's no real PIC flag so we must build both object types
 case $host_os in
 cygwin* | mingw* | pw32* | os2*)
@@ -850,8 +847,6 @@
 if test $compiler_c_o = no; then
   output_obj=`$ECHO X$srcfile | $Xsed -e 's%^.*/%%' -e 
's%\.[^.]*$%%'`.${objext}
   lockfile=$output_obj.lock
-  removelist=$removelist $output_obj $lockfile
-  trap $opt_dry_run || $RM $removelist; exit $EXIT_FAILURE 1 2 15
 else
   output_obj=
   need_locks=no
@@ -881,17 +876,20 @@
$opt_dry_run || $RM $removelist
exit $EXIT_FAILURE
   fi
+  removelist=$removelist $output_obj
   $ECHO $srcfile  $lockfile
 fi
 
+$opt_dry_run || $RM $removelist
+removelist=$removelist $lockfile
+trap '$opt_dry_run || $RM $removelist; exit $EXIT_FAILURE' 1 2 15
+
 if test -n $fix_srcfile_path; then
   eval srcfile=\$fix_srcfile_path\
 fi
 func_quote_for_eval $srcfile
 qsrcfile=$func_quote_for_eval_result
 
-$opt_dry_run || $RM $libobj ${libobj}T
-
 # Only build a PIC object if we are building libtool libraries.
 if test $build_libtool_libs = yes; then
   # Without this assignment, base_compile gets emptied.
@@ -911,8 +909,6 @@
command=$command -o $lobj
   fi
 
-  $opt_dry_run || $RM $lobj $output_obj
-
   func_show_eval_locale $command \
   'test -n $output_obj  $RM $removelist; exit $EXIT_FAILURE'
 
@@ -962,7 +958,6 @@
 
   # Suppress compiler output if we already did a PIC compilation.
   command=$command$suppress_output
-  $opt_dry_run || $RM $obj $output_obj
   func_show_eval_locale $command \
 '$opt_dry_run || $RM $removelist; exit $EXIT_FAILURE'
 
@@ -998,6 +993,7 @@
 
   # Unlock the critical section if it was locked
   if test $need_locks != no; then
+   removelist=$lockfile
 $RM $lockfile
   fi
 }

Re: No need for object file removal before compilation

2008-04-12 Thread Ralf Wildenhues 
* Ralf Wildenhues wrote on Sat, Apr 12, 2008 at 08:56:24AM CEST:
 OK for the updated one?  It makes for 5% faster
   libtool --mode=compile gcc -c a.c

I should be more precise: the above holds for an empty file a.c,
measured using a script that calls the above 50 times, pre and post
patch.

I should also note that this patch introduces an inevitable race
(of course all dealing with the no -c -o case):

After we grabbed the lockfile, but before we install the trap to remove
it, if the user interrupts libtool in between, we have a leftover
lockfile.  The next compile will barf out, but the error message will
point the user towards the lockfile.

This is benign, and IMNSHO unavoidable.

Cheers,
Ralf

 2008-04-12  Ralf Wildenhues  [EMAIL PROTECTED]
 
   * libltdl/config/ltmain.m4sh (func_mode_compile): Avoid
   redundant removal of old output files before compilation.
   Do not remove $obj until we have successfully grabbed the
   lockfile (in case the compiler doesn't grok `-c -o'), because
   it might be identical to $output_obj.
   At the end of the function, before we remove the lockfile,
   update $removelist so that if the trap hits after the lockfile
   has been removed, we do not accidentally remove $output_obj that
   does not belong to us.
   (func_write_libtool_object): Use $MV instead of mv.

Re: No need for object file removal before compilation

2008-04-12 Thread Bob Friesenhahn 

On Sat, 12 Apr 2008, Ralf Wildenhues wrote:

Then, while I have some doubts about the practical applicability of
Bob's scenario, I left the code so that it would be supported: all


Bob's scenarios are rarely practical.


I would have been inclined to ok this one.


OK for the updated one?  It makes for 5% faster
 libtool --mode=compile gcc -c a.c


The patch looks ok to me.  We shall see about the actual performance 
improvement.


Bob



Thanks,
Ralf

2008-04-12  Ralf Wildenhues  [EMAIL PROTECTED]

* libltdl/config/ltmain.m4sh (func_mode_compile): Avoid
redundant removal of old output files before compilation.
Do not remove $obj until we have successfully grabbed the
lockfile (in case the compiler doesn't grok `-c -o'), because
it might be identical to $output_obj.
At the end of the function, before we remove the lockfile,
update $removelist so that if the trap hits after the lockfile
has been removed, we do not accidentally remove $output_obj that
does not belong to us.
(func_write_libtool_object): Use $MV instead of mv.

Index: libltdl/config/ltmain.m4sh
===
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.106
diff -u -r1.106 ltmain.m4sh
--- libltdl/config/ltmain.m4sh  11 Apr 2008 17:21:55 -  1.106
+++ libltdl/config/ltmain.m4sh  12 Apr 2008 06:55:46 -
@@ -650,7 +650,7 @@
non_pic_object=$write_oldobj

EOF
-  mv -f ${write_libobj}T ${write_libobj}
+  $MV ${write_libobj}T ${write_libobj}
}
}

@@ -831,9 +831,6 @@
  removelist=$lobj $libobj ${libobj}T
fi

-$opt_dry_run || $RM $removelist
-trap $opt_dry_run || $RM $removelist; exit $EXIT_FAILURE 1 2 15
-
# On Cygwin there's no real PIC flag so we must build both object types
case $host_os in
cygwin* | mingw* | pw32* | os2*)
@@ -850,8 +847,6 @@
if test $compiler_c_o = no; then
  output_obj=`$ECHO X$srcfile | $Xsed -e 's%^.*/%%' -e 
's%\.[^.]*$%%'`.${objext}
  lockfile=$output_obj.lock
-  removelist=$removelist $output_obj $lockfile
-  trap $opt_dry_run || $RM $removelist; exit $EXIT_FAILURE 1 2 15
else
  output_obj=
  need_locks=no
@@ -881,17 +876,20 @@
$opt_dry_run || $RM $removelist
exit $EXIT_FAILURE
  fi
+  removelist=$removelist $output_obj
  $ECHO $srcfile  $lockfile
fi

+$opt_dry_run || $RM $removelist
+removelist=$removelist $lockfile
+trap '$opt_dry_run || $RM $removelist; exit $EXIT_FAILURE' 1 2 15
+
if test -n $fix_srcfile_path; then
  eval srcfile=\$fix_srcfile_path\
fi
func_quote_for_eval $srcfile
qsrcfile=$func_quote_for_eval_result

-$opt_dry_run || $RM $libobj ${libobj}T
-
# Only build a PIC object if we are building libtool libraries.
if test $build_libtool_libs = yes; then
  # Without this assignment, base_compile gets emptied.
@@ -911,8 +909,6 @@
command=$command -o $lobj
  fi

-  $opt_dry_run || $RM $lobj $output_obj
-
  func_show_eval_locale $command\
  'test -n $output_obj  $RM $removelist; exit $EXIT_FAILURE'

@@ -962,7 +958,6 @@

  # Suppress compiler output if we already did a PIC compilation.
  command=$command$suppress_output
-  $opt_dry_run || $RM $obj $output_obj
  func_show_eval_locale $command \
'$opt_dry_run || $RM $removelist; exit $EXIT_FAILURE'

@@ -998,6 +993,7 @@

  # Unlock the critical section if it was locked
  if test $need_locks != no; then
+   removelist=$lockfile
$RM $lockfile
  fi
}



==
Bob Friesenhahn
[EMAIL PROTECTED], http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,http://www.GraphicsMagick.org/

switching to git, please no more CVS commits

2008-04-12 Thread Ralf Wildenhues 
Hello,

please do not commit to CVS for now, until further notification.

Thanks,
Ralf