make_ext.pl direct pm_to_blib handling causes unnecessary rebuilds #15060
Comments
From @tonycozFor pure-perl modules with no Makefile.PL, make_ext.pl avoids the work in creating Unfortunately it always "touches" the module's pm_to_blib, even if no files were copied. Since perlmain.c depends on ext/ExtUtils-Miniperl/pm_to_blib that file is always rebuilt leading to other files being rebuilt. Under normal usage, when ExtUtils::Install::pm_to_blib() is called from a Makefile, the target depends on the .pm files involved, so the pm_to_blib file is only touched when those the .pm files are updated, so it's only a problem with make_ext.pl. make_ext.pl should check the .pm files are newer than pm_to_blib before touching pm_to_blib. Tony Summary of my perl5 (revision 5 version 23 subversion 6) configuration: Characteristics of this binary (from libperl): |
From @jkeenanOn Sun Nov 22 15:35:15 2015, tonyc wrote:
Can you provide a list of those pure-perl modules (or a formula to derive such a list)? Thank you very much.
-- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sun, Nov 22, 2015 at 03:53:19PM -0800, James E Keenan via RT wrote:
In a built perl tree run: grep make_ext `find . -name pm_to_blib` $ grep make_ext `find . -name pm_to_blib` | wc -l So lots, but I'm not sure than any of the others cause rebuilds. Tony |
From @tonycozOn Sun Nov 22 15:35:15 2015, tonyc wrote:
Something like the attached. Tony |
From @tonycoz0001-perl-126710-only-touch-pm_to_blib-if-files-are-copie.patchFrom 942c9d10af7307004b6be235a7d5967c1d2978c7 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 1 Dec 2015 16:10:55 +1100
Subject: [perl #126710] only touch pm_to_blib if files are copied
Add checks similar to what the Makefile would do: only copy the
files if the source file is newer than pm_to_blib
---
make_ext.pl | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/make_ext.pl b/make_ext.pl
index 0745049..787faba 100644
--- a/make_ext.pl
+++ b/make_ext.pl
@@ -708,20 +708,37 @@ sub just_pm_to_blib {
}
# This is running under miniperl, so no autodie
if ($target eq 'all') {
- local $ENV{PERL_INSTALL_QUIET} = 1;
- require ExtUtils::Install;
- ExtUtils::Install::pm_to_blib(\%pm, '../../lib/auto');
- open my $fh, '>', $pm_to_blib
- or die "Can't open '$pm_to_blib': $!";
- print $fh "$0 has handled pm_to_blib directly\n";
- close $fh
- or die "Can't close '$pm_to_blib': $!";
- if (IS_UNIX) {
- # Fake the fallback cleanup
- my $fallback
- = join '', map {s!^\.\./\.\./!!; "rm -f $_\n"} sort values %pm;
- foreach my $clean_target ('realclean', 'veryclean') {
- fallback_cleanup($return_dir, $clean_target, $fallback);
+ my $need_update = 1;
+ if (-f $pm_to_blib) {
+ # avoid touching pm_to_blib unless there's something that
+ # needs updating, see #126710
+ $need_update = 0;
+ my $test_at = -M _;
+ while (my $from = each(%pm)) {
+ if (-M $from < $test_at) {
+ ++$need_update;
+ last;
+ }
+ }
+ keys %pm; # reset iterator
+ }
+
+ if ($need_update) {
+ local $ENV{PERL_INSTALL_QUIET} = 1;
+ require ExtUtils::Install;
+ ExtUtils::Install::pm_to_blib(\%pm, '../../lib/auto');
+ open my $fh, '>', $pm_to_blib
+ or die "Can't open '$pm_to_blib': $!";
+ print $fh "$0 has handled pm_to_blib directly\n";
+ close $fh
+ or die "Can't close '$pm_to_blib': $!";
+ if (IS_UNIX) {
+ # Fake the fallback cleanup
+ my $fallback
+ = join '', map {s!^\.\./\.\./!!; "rm -f $_\n"} sort values %pm;
+ foreach my $clean_target ('realclean', 'veryclean') {
+ fallback_cleanup($return_dir, $clean_target, $fallback);
+ }
}
}
} else {
--
2.1.4
|
From @jkeenanOn Mon Nov 30 21:11:45 2015, tonyc wrote:
I ran 'make' at commit 406d554 and transcribed the output. I then created a branch, reconfigured, applied your patch, ran 'make' and again transcribed the output. The differences were very small; see attachment. Should I have expected to see more differences? Thank you very much. -- |
From @jkeenanblead.126710.make.diff--- blead.make.typescript 2015-12-05 09:13:12.864841463 -0500
+++ 126710.make.typescript 2015-12-05 09:13:20.052841254 -0500
@@ -1,4 +1,4 @@
-Script started on Sat 05 Dec 2015 09:03:24 AM EST
+Script started on Sat 05 Dec 2015 09:10:00 AM EST
[�[01;31mperl�[00m] �[01;33m1 �[00m$ make
echo @`sh cflags "optimize='-O2'" perlmini.o` -DPERL_IS_MINIPERL -DPERL_EXTERNAL_GLOB perlmini.c
@cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c89 -O2 -Wall -Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-strings -DPERL_IS_MINIPERL -DPERL_EXTERNAL_GLOB perlmini.c
@@ -335,14 +335,14 @@
Generating a Unix-style Makefile
Writing Makefile for Pod
make[1]: Entering directory `/home/jkeenan/gitwork/perl/cpan/podlators'
-"../../miniperl" "-I../../lib" "-I../../lib" "-I../../lib" "-I../../lib" pod2text.PL pod2text
-Extracting pod2text (with variable substitutions)
-cp pod2text blib/script/pod2text
-/home/jkeenan/gitwork/perl/cpan/podlators/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/pod2text
"../../miniperl" "-I../../lib" "-I../../lib" "-I../../lib" "-I../../lib" pod2man.PL pod2man
Extracting pod2man (with variable substitutions)
cp pod2man blib/script/pod2man
/home/jkeenan/gitwork/perl/cpan/podlators/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/pod2man
+"../../miniperl" "-I../../lib" "-I../../lib" "-I../../lib" "-I../../lib" pod2text.PL pod2text
+Extracting pod2text (with variable substitutions)
+cp pod2text blib/script/pod2text
+/home/jkeenan/gitwork/perl/cpan/podlators/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/pod2text
make[1]: Leaving directory `/home/jkeenan/gitwork/perl/cpan/podlators'
./miniperl -Ilib make_ext.pl cpan/Pod-Parser/pm_to_blib MAKE="make" LIBPERL_A=libperl.a
Generating a Unix-style Makefile
@@ -1147,10 +1147,10 @@
chmod 755 ../../lib/auto/Encode/Encode.so
/home/jkeenan/gitwork/perl/cpan/Encode/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::Command::MM -e 'cp_nonempty' -- Encode.bs ../../lib/auto/Encode/Encode.bs 644
-cp bin/enc2xs blib/script/enc2xs
-/home/jkeenan/gitwork/perl/cpan/Encode/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/enc2xs
cp bin/encguess blib/script/encguess
/home/jkeenan/gitwork/perl/cpan/Encode/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/encguess
+cp bin/enc2xs blib/script/enc2xs
+/home/jkeenan/gitwork/perl/cpan/Encode/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/enc2xs
cp bin/piconv blib/script/piconv
/home/jkeenan/gitwork/perl/cpan/Encode/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/piconv
make[1]: Leaving directory `/home/jkeenan/gitwork/perl/cpan/Encode'
@@ -1870,5 +1870,5 @@
Everything is up to date. Type 'make test' to run test suite.
[�[01;31mperl�[00m] �[01;33m2 �[00m$ exit
-Script done on Sat 05 Dec 2015 09:06:41 AM EST
+Script done on Sat 05 Dec 2015 09:12:44 AM EST
|
From @tonycozOn Sat Dec 05 06:24:27 2015, jkeenan wrote:
The differences would be in a second build of the same tree. Without the change perlmain.c and its dependents are rebuilt each time. Tony |
From @cpansproutIf I run a simple ‘make’ in the perl build directory multiple times, it recompiles perlmain.c every time. (This causes problems for me, since I often do ‘make && sudo make install’ *followed* by ‘make test’, which then fails since perlmain.o is owned by root and cannot be overwritten. I do things in this order so as to test CPAN modules against blead as soon as possible, while in the mean time running perl’s tests to make sure they pass in the configuration I am using.) ./perl -Ilib -V output: Summary of my perl5 (revision 5 version 25 subversion 2) configuration: Characteristics of this binary (from libperl): -- Father Chrysostomos |
From @cpansproutOn Thu May 26 22:11:33 2016, sprout wrote:
Bisect: $ ../perl.git/Porting/bisect.pl -- perl -e '$age = -M "perlmain.o"; system "make -j25"; $age == -M "perlmain.o" or die' For simple extensions make_ext.pl can emulate the entire MakeMaker/make dance. -- Father Chrysostomos |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Thu Jun 09 22:45:58 2016, sprout wrote:
The problem is that make_ext.pl:just_pm_to_blib touches pm_to_blib unconditionally even if the files have not changed. perlmain.c depends on ext/ExtUtils-Miniperl/pm_to_blib. -- Father Chrysostomos |
From @tonycozOn Sat, Jun 11, 2016 at 07:32:59PM -0700, Father Chrysostomos via RT wrote:
Does the patch in #126710 fix this for you? Tony |
From @cpansproutOn Sat Jun 11 20:46:17 2016, tonyc wrote:
Yes, it does. (I see this ticket is a duplicate. I’ll merge them.) Another approach would be to allow EUMM to handle ExtUtils::Miniperl. I can’t say which is better. What is our goal here? To keep make_ext.pl as simple as possible, or to make things more ‘correct’ from the point of view of proper dependency management. Nevertheless, I would like to see this fixed in blead soon, because I keep having to chown after sudo make install. -- Father Chrysostomos |
From @tonycozOn Sat Jun 11 22:36:15 2016, sprout wrote:
Correctness trumps performance. I've applied my patch to blead as 31b6f23. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#126710 (status was 'resolved')
Searchable as RT126710$
The text was updated successfully, but these errors were encountered: