Front page | perl.perl5.porters |
Postings from January 2014
Re: [perl #121119] [PATCH] speed up miniperl @INC searching,buildcustomize
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
January 30, 2014 11:49
Subject:
Re: [perl #121119] [PATCH] speed up miniperl @INC searching,buildcustomize
Message ID:
20140130114951.GC22619@plum.flirble.org
On Thu, Jan 30, 2014 at 11:25:28AM +0000, Dave Mitchell wrote:
> On Thu, Jan 30, 2014 at 12:13:16AM -0800, bulk88 via RT wrote:
> > > See attached patch. Each dir being searched that fails on Win32
> > > results
> > > in 10 I/O sys calls which I counted at 17 ms wall time, times 16 build
> > > customize dirs that didn't match is an extra 272 ms of wall time
> > > dealing
> > > with the not found dirs during an @INC search. Also as the build
> > > progresses, more and more things will be found on the 1st try in /lib,
> > > and I presume buildcustomize dirs become totally unused once XS module
> > > start being built since everything needed for XS building (which is a
> > > PP
> > > process) will be in /lib and found on 1st try.
>
> [snip; this cuts the 'make' time by: ]
>
> > > (1656 - 1137) / 60 = 8.65 mins
>
> [snip]
>
> > > I left the original comment in place for historical reasons. The
> > > comment
> > > might be from something that happened in a Module::Build module back
> > > when M::B was in core.
>
> For the convenience of others, here is the original patch:
>
> diff --git a/write_buildcustomize.pl b/write_buildcustomize.pl
> index 64bf4ce..5309988 100644
> --- a/write_buildcustomize.pl
> +++ b/write_buildcustomize.pl
> @@ -48,13 +48,18 @@ push @toolchain, 'ext/VMS-Filespec/lib' if $^O eq 'VMS';
> unshift @INC, @toolchain;
> require File::Spec::Functions;
>
> +# former comment
> +#
> # lib must be last, as the toolchain modules write themselves into it
> # as they build, and it's important that @INC order ensures that the partially
> # written files are always masked by the complete versions.
>
> my $inc = join ",\n ",
> map { "q\0$_\0" }
> - (map {File::Spec::Functions::rel2abs($_)} @toolchain, 'lib');
> +# putting lib first shaves a couple minutes off the build time since the most
> +# common modules like warnings and strict are in lib, and as extensions are
> +# built the chances of the module being found in lib increases
> + (map {File::Spec::Functions::rel2abs($_)} 'lib', @toolchain);
>
>
> Before such a patch went in, I think we'd have to be sure that the reasons
> stated in the code for needing to put lib last don't still apply.
> Although I'm not very familiar with lib/buildcustomize.pl, it seems to me
> that the reason still stands. Especially on a parallel make, I could
> see one instance of miniperl running while another instance is copying
> files from ext/Foo to lib, thus allowing it to see empty or truncated
> files; or even if the installation of files into lib/ is atomic, a mixture
> of files from the same distro, some in lib, some still under ext/ or
> whatever might be seem, and might be harmful.
>
> So this doesn't seem safe to me.
Thanks for ensuring that the patch gets to the list.
No, specifically it's completely unsafe for a parallel make.
You end up with race conditions where the build fails because one process
process loads from lib/ a partially written module and aborts, because it
happens just as another process is copying that file there.
(And the rest of the time you don't have a problem because the file is either
not in lib, so loaded from the original dist/... etc, or it's fully copied to
lib, so loaded from there)
Which is exactly what the comment in the file tries to explain.
However, as Win32 doesn't have parallel makes *yet*, and this seems to be a
big speed hit on Win32, I think that it would be reasonable to do the
proposed re-ordering on Win32, with a comment that it needs to be rethought
once anyone starts getting parallel makes working on Win32.
Nicholas Clark
Thread Previous
|
Thread Next