Обсуждение: Parallel build with MSVC
On my system, "build DEBUG" takes ~2.75 minutes. When I tell MSBuild to build in parallel by passing it the /m flag, that goes down to 1.5 minutes. The first patch passes the value of the MSBFLAGS environment variable to msbuild's command line. The output of parallel and sequential builds has identical file count and size after installing; all tests pass. Even without /m, MSBuild will already run enough compilers to keep all CPUs busy whenever it can do that with only a single project's files. With /m, it will also process _projects_ in parallel. One additional fix required is to put gendef.pl's "symbols.out" file into the directory it is working on, rather than the source tree root, to avoid collisions that cause the parallel build to fail otherwise. -- Christian
On Tue, Apr 26, 2016 at 10:09 PM, Christian Ullrich <chris@chrullrich.net> wrote: > The first patch passes the value of the MSBFLAGS environment variable to > msbuild's command line. > > The output of parallel and sequential builds has identical file count and > size after installing; all tests pass. If a committer is willing to pick up that, I'd say go for it. Personally, I have modified some of my windows build scripts in this area to pass additional flags. Not sure that there are many people in a similar case than me around though :) > Even without /m, MSBuild will already run enough compilers to keep all CPUs > busy whenever it can do that with only a single project's files. With /m, it > will also process _projects_ in parallel. > > One additional fix required is to put gendef.pl's "symbols.out" file into > the directory it is working on, rather than the source tree root, to avoid > collisions that cause the parallel build to fail otherwise. <screen> -<userinput>build DEBUG</userinput> +<userinput>build -c DEBUG</userinput></screen> To build just a single project, for example psql, run the commands:<screen><userinput>buildpsql</userinput> -<userinput>build DEBUG psql</userinput> +<userinput>build -c DEBUG psql</userinput></screen> Why is that? Your patch just has a look at argv[0] to see if that's a debug or release build. +use File::Spec::Functions qw(splitpath catpath); This is present since at least perl 5.8, so that's not a blocker. vcbuild also supports /m. Wouldn't it make sense to have a environment variable flag for it as well? vcbuild has been replaced by msbuild in VS2010 but I would think that in back-branches this would have value for users still compiling with VS2008 or older, and those are still supported things. -- Michael
* From: Michael Paquier [mailto:michael.paquier@gmail.com] > On Tue, Apr 26, 2016 at 10:09 PM, Christian Ullrich > <chris@chrullrich.net> wrote: > <screen> > -<userinput>build DEBUG</userinput> > +<userinput>build -c DEBUG</userinput> > </screen> > To build just a single project, for example psql, run the commands: > <screen> > <userinput>build psql</userinput> > -<userinput>build DEBUG psql</userinput> > +<userinput>build -c DEBUG psql</userinput> > </screen> > > Why is that? Your patch just has a look at argv[0] to see if that's a > debug or release build. Sorry, forgot to fix that. I originally used Getopt in build.pl, then realized maintaining compatibility was more important. Thanks for noticing; new patches attached; the second one is unmodified. > +use File::Spec::Functions qw(splitpath catpath); > This is present since at least perl 5.8, so that's not a blocker. > > vcbuild also supports /m. Wouldn't it make sense to have a environment > variable flag for it as well? vcbuild has been replaced by msbuild in > VS2010 but I would think that in back-branches this would have value > for users still compiling with VS2008 or older, and those are still > supported things. Good point, but I have no installation of 2008 around, so I cannot test it. Perhaps there is someone around who could dothat (just add the $msbflags as the first argument to vcbuild)? -- Christian
Вложения
On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich <chris@chrullrich.net> wrote: > * From: Michael Paquier [mailto:michael.paquier@gmail.com] >> Why is that? Your patch just has a look at argv[0] to see if that's a >> debug or release build. > > Sorry, forgot to fix that. I originally used Getopt in build.pl, then realized maintaining compatibility was more important. > > Thanks for noticing; new patches attached; the second one is unmodified. Thanks for the updated patches, those look good to me. The environment flag is missing with vcbuild. you'd want to add that at the end. >> +use File::Spec::Functions qw(splitpath catpath); >> This is present since at least perl 5.8, so that's not a blocker. >> >> vcbuild also supports /m. Wouldn't it make sense to have a environment >> variable flag for it as well? vcbuild has been replaced by msbuild in >> VS2010 but I would think that in back-branches this would have value >> for users still compiling with VS2008 or older, and those are still >> supported things. > > Good point, but I have no installation of 2008 around, so I cannot test it. Perhaps there is someone around who could dothat (just add the $msbflags as the first argument to vcbuild)? I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2, and using VS2008 command prompt with vcbuild /m I am not seeing issues. Moving symbols.out would be the main issue, but I am not noticing problems related to that. -- Michael
* From: Michael Paquier [mailto:michael.paquier@gmail.com] > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich <chris@chrullrich.net> > wrote: > > * From: Michael Paquier [mailto:michael.paquier@gmail.com] > >> vcbuild also supports /m. Wouldn't it make sense to have a environment > >> variable flag for it as well? vcbuild has been replaced by msbuild in > >> VS2010 but I would think that in back-branches this would have value > >> for users still compiling with VS2008 or older, and those are still > >> supported things. > > > > Good point, but I have no installation of 2008 around, so I cannot test > > it. Perhaps there is someone around who could do that (just add the > > $msbflags as the first argument to vcbuild)? > > I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2, > and using VS2008 command prompt with vcbuild /m I am not seeing > issues. Moving symbols.out would be the main issue, but I am not > noticing problems related to that. OK then, hopefully last round attached. -- Christian
Вложения
On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich <chris@chrullrich.net> wrote: > OK then, hopefully last round attached. Thanks. Those are fine in my view. It is hard to tell if a committer is going to have a look at that soon, so I think that it would be wiser to add that to the next CF so as those patches don't fall into oblivion. -- Michael
* Michael Paquier wrote: > On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich <chris@chrullrich.net> wrote: >> OK then, hopefully last round attached. > > Thanks. Those are fine in my view. It is hard to tell if a committer > is going to have a look at that soon, so I think that it would be > wiser to add that to the next CF so as those patches don't fall into > oblivion. Done. -- Christian
On Thu, Apr 28, 2016 at 4:16 PM, Christian Ullrich <chris@chrullrich.net> wrote: > * Michael Paquier wrote: >> On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich <chris@chrullrich.net> >> wrote: >>> OK then, hopefully last round attached. >> >> Thanks. Those are fine in my view. It is hard to tell if a committer >> is going to have a look at that soon, so I think that it would be >> wiser to add that to the next CF so as those patches don't fall into >> oblivion. > > Done. As far as I can see, those patches are still fine, so I switched the patch as 'ready for committer'. -- Michael
On Wed, Apr 27, 2016 at 08:15:05AM +0000, Christian Ullrich wrote: > * From: Michael Paquier [mailto:michael.paquier@gmail.com] > > > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich <chris@chrullrich.net> > > wrote: > > > > * From: Michael Paquier [mailto:michael.paquier@gmail.com] > > > >> vcbuild also supports /m. Wouldn't it make sense to have a environment > > >> variable flag for it as well? vcbuild has been replaced by msbuild in > > >> VS2010 but I would think that in back-branches this would have value > > >> for users still compiling with VS2008 or older, and those are still > > >> supported things. > > > > > > Good point, but I have no installation of 2008 around, so I cannot test > > > it. Perhaps there is someone around who could do that (just add the > > > $msbflags as the first argument to vcbuild)? > > > > I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2, > > and using VS2008 command prompt with vcbuild /m I am not seeing > > issues. Moving symbols.out would be the main issue, but I am not > > noticing problems related to that. > > OK then, hopefully last round attached. Every vcbuild and msbuild invocation ought to recognize this variable, so please update the two places involving ecpg_regression.proj. Apart from that, the patch looks good. I was tempted to back-patch this. The risk to back branch users seems negligible, and it would be convenient for me as a person who builds all branches. That reason is not good enough, so I plan not to back-patch. I feel like I might be missing a stronger reason to back-patch.
Noah Misch <noah@leadboat.com> writes:
> I was tempted to back-patch this. The risk to back branch users seems
> negligible, and it would be convenient for me as a person who builds all
> branches. That reason is not good enough, so I plan not to back-patch. I
> feel like I might be missing a stronger reason to back-patch.
Hm, wouldn't it help reduce the cycle time for Windows buildfarm members?
That might still not be adequate reason, but it's an advantage beyond
time-saving for individual developers.
regards, tom lane
On Sun, Sep 04, 2016 at 08:26:12PM -0400, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: > > I was tempted to back-patch this. The risk to back branch users seems > > negligible, and it would be convenient for me as a person who builds all > > branches. That reason is not good enough, so I plan not to back-patch. I > > feel like I might be missing a stronger reason to back-patch. > > Hm, wouldn't it help reduce the cycle time for Windows buildfarm members? > That might still not be adequate reason, but it's an advantage beyond > time-saving for individual developers. Yes; multi-core Windows buildfarm members could configure MSBFLAGS=/m to finish more quickly.
On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch <noah@leadboat.com> wrote: > On Wed, Apr 27, 2016 at 08:15:05AM +0000, Christian Ullrich wrote: >> * From: Michael Paquier [mailto:michael.paquier@gmail.com] >> >> > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich <chris@chrullrich.net> >> > wrote: >> >> > > * From: Michael Paquier [mailto:michael.paquier@gmail.com] >> >> > >> vcbuild also supports /m. Wouldn't it make sense to have a environment >> > >> variable flag for it as well? vcbuild has been replaced by msbuild in >> > >> VS2010 but I would think that in back-branches this would have value >> > >> for users still compiling with VS2008 or older, and those are still >> > >> supported things. >> > > >> > > Good point, but I have no installation of 2008 around, so I cannot test >> > > it. Perhaps there is someone around who could do that (just add the >> > > $msbflags as the first argument to vcbuild)? >> > >> > I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2, >> > and using VS2008 command prompt with vcbuild /m I am not seeing >> > issues. Moving symbols.out would be the main issue, but I am not >> > noticing problems related to that. >> >> OK then, hopefully last round attached. > > Every vcbuild and msbuild invocation ought to recognize this variable, so > please update the two places involving ecpg_regression.proj. Apart from that, > the patch looks good. Good catch. I did not notice those during my lookups of those patches. Not my intention to bump into Christian's feet, but here are updated patches. Actually, is that worth adding for clean.bat? I doubt that many people would care if MSBFLAGS is not supported in it. The cleanup script is not the bottleneck, the build script is. I added it in the patch 0001 attached but I doubt that's worth it to be honest. -- Michael
Вложения
* Michael Paquier wrote: > On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch <noah@leadboat.com> wrote: >> Every vcbuild and msbuild invocation ought to recognize this variable, so >> please update the two places involving ecpg_regression.proj. Apart from that, >> the patch looks good. > > Good catch. I did not notice those during my lookups of those patches. > Not my intention to bump into Christian's feet, but here are updated > patches. My feet feel fine. Thanks for updating. -- Christian
On Mon, Sep 05, 2016 at 02:43:48PM +0900, Michael Paquier wrote: > On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch <noah@leadboat.com> wrote: > > Every vcbuild and msbuild invocation ought to recognize this variable, so > > please update the two places involving ecpg_regression.proj. Apart from that, > > the patch looks good. > > Good catch. I did not notice those during my lookups of those patches. > Not my intention to bump into Christian's feet, but here are updated > patches. Committed. > Actually, is that worth adding for clean.bat? I doubt that many people > would care if MSBFLAGS is not supported in it. The cleanup script is > not the bottleneck, the build script is. I added it in the patch 0001 > attached but I doubt that's worth it to be honest. If parallelism were the only consideration, then I would agree. We don't know, in general, how each operation will respond to arbitrary flags the user selects. I did remove your clean.bat documentation change; documenting MSBFLAGS in the one place suffices.
* Noah Misch wrote: > Committed. Much apologizings for coming in late again, but I just realized it would be better if the user-controlled flags came after all predefined options the user might want to override. Right now that is only /verbosity in both build and clean operations. Patch attached, also reordering the ecpg-clean command line in clean.bat to match the others that have the project file first. -- Christian
Вложения
On Thu, Sep 8, 2016 at 3:54 PM, Christian Ullrich <chris@chrullrich.net> wrote: > Much apologizings for coming in late again, but I just realized it would be > better if the user-controlled flags came after all predefined options the > user might want to override. Right now that is only /verbosity in both build > and clean operations. > > Patch attached, also reordering the ecpg-clean command line in clean.bat to > match the others that have the project file first. - "msbuild $buildwhat.vcxproj $msbflags /verbosity:normal /p:Configuration=$bconf" + "msbuild $buildwhat.vcxproj /verbosity:normal $msbflags /p:Configuration=$bconf" Why not... If people are willing to switch to /verbosity:detailed and double the amount of build time... -- Michael
On Thu, Sep 08, 2016 at 08:54:08AM +0200, Christian Ullrich wrote: > * Noah Misch wrote: > > >Committed. > > Much apologizings for coming in late again, but I just realized it would be > better if the user-controlled flags came after all predefined options the > user might want to override. Right now that is only /verbosity in both build > and clean operations. > > Patch attached, also reordering the ecpg-clean command line in clean.bat to > match the others that have the project file first. Committed.