Front page | perl.perl5.porters |
Postings from February 2014
Re: [perl #121119] [PATCH] speed up miniperl @INC searching, buildcustomize
Thread Previous
|
Thread Next
From:
Brian Fraser
Date:
February 3, 2014 05:00
Subject:
Re: [perl #121119] [PATCH] speed up miniperl @INC searching, buildcustomize
Message ID:
CA+nL+nbCNxsc=6bPjWsc3TnyP0b9LwM06t0z2kHsaFH8NY2oWg@mail.gmail.com
On Mon, Feb 3, 2014 at 5:54 AM, Brian Fraser <fraserbn@gmail.com> wrote:
> On Thu, Jan 30, 2014 at 12:49 PM, Nicholas Clark <nick@ccl4.org> wrote:
>
>> 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*,
>
>
> Well, on a technicality, it does:
>
>
> http://perl5.git.perl.org/perl.git/shortlog/refs/heads/hugmeir/cross-compile-win32
>
> Although parallel make on that branch isn't quite functional yet either,
> because of some interaction with create_perllibst_h.pl that I haven't
> investigated yet, which causes make to abort if run in parallel before
> perllib.o is compiled.
>
> I'll be the first to admit that the branch is of dubious value, but if
> it's possible not to break it further, that'd be swell.
>
>
Actually, on further thought, making the change only take effect for $^O eq
'MSWin32' would work -- at that point during cross-compilation, miniperl is
not yet pretending to be the target system, so it wouldn't kick in.
So nevermind part of my comment, the change would only affect native builds.
Thread Previous
|
Thread Next