Re: [HACKERS] Parallel build with MSVC
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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Thu, Sep 8, 2016 at 3:54 PM, Christian Ullrich 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
* 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
>From 09a5f3945e2b87b56ca3525a56db970e9ecf8ffd Mon Sep 17 00:00:00 2001
From: Christian Ullrich
Date: Thu, 8 Sep 2016 08:34:45 +0200
Subject: [PATCH] Let MSBFLAGS be used to override predefined options.
---
src/tools/msvc/build.pl | 4 ++--
src/tools/msvc/clean.bat | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/tools/msvc/build.pl b/src/tools/msvc/build.pl
index 5273977..a5469cd 100644
--- a/src/tools/msvc/build.pl
+++ b/src/tools/msvc/build.pl
@@ -54,7 +54,7 @@ elsif (uc($ARGV[0]) ne "RELEASE")
if ($buildwhat and $vcver >= 10.00)
{
system(
- "msbuild $buildwhat.vcxproj $msbflags /verbosity:normal
/p:Configuration=$bconf"
+ "msbuild $buildwhat.vcxproj /verbosity:normal $msbflags
/p:Configuration=$bconf"
);
}
elsif ($buildwhat)
@@ -63,7 +63,7 @@ elsif ($buildwhat)
}
else
{
- system("msbuild pgsql.sln $msbflags /verbosity:normal
/p:Configuration=$bconf");
+ system("msbuild pgsql.sln /verbosity:normal $msbflags
/p:Configuration=$bconf");
}
# report status
diff --git a/src/tools/msvc/clean.bat b/src/tools/msvc/clean.bat
index e21e37f..b972ddf 100755
--- a/src/tools/msvc/clean.bat
+++ b/src/tools/msvc/clean.bat
@@ -107,6 +107,6 @@ REM for /r %%f in (*.sql) do if exist %%f.in del %%f
cd %D%
REM Clean up ecpg regression test files
-msbuild %MSBFLAGS% /NoLogo ecpg_regression.proj /t:clean /v:q
+msbuild ecpg_regression.proj /NoLogo /v:q %MSBFLAGS% /t:clean
goto :eof
--
2.10.0.windows.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Mon, Sep 05, 2016 at 02:43:48PM +0900, Michael Paquier wrote: > On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch 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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
* Michael Paquier wrote: On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Mon, Sep 5, 2016 at 9:18 AM, Noah Misch wrote:
> On Wed, Apr 27, 2016 at 08:15:05AM +, Christian Ullrich wrote:
>> * From: Michael Paquier [mailto:michael.paqu...@gmail.com]
>>
>> > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich
>> > wrote:
>>
>> > > * From: Michael Paquier [mailto:michael.paqu...@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
From aaf028d2fab130d908a4efcd3c6316b4e2c3fdd7 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Mon, 5 Sep 2016 14:36:31 +0900
Subject: [PATCH 1/2] Support passing arbitrary arguments to MSBuild/VCBuild.
This is particularly useful to pass /m, to perform a parallel build.
---
doc/src/sgml/install-windows.sgml | 11 ++-
src/tools/msvc/build.pl | 7 ---
src/tools/msvc/clean.bat | 2 +-
src/tools/msvc/vcregress.pl | 3 ++-
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 8cd189c..f656c66 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -145,6 +145,14 @@ $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
+
+ To pass additional command line arguments to the Visual Studio build
+ command (msbuild or vcbuild):
+
+$ENV{MSBFLAGS}="/m";
+
+
+
Requirements
@@ -401,7 +409,8 @@ $ENV{CONFIG}="Debug";
all generated files. You can also run it with the
dist parameter, in which case it will behave like
make distclean and remove the flex/bison output files
- as well.
+ as well. Additional flags can be passed to this script for commands of
+ msbuild and vcbuild using the environment variable MSBFLAGS.
diff --git a/src/tools/msvc/build.pl b/src/tools/msvc/build.pl
index 007e3c7..5273977 100644
--- a/src/tools/msvc/build.pl
+++ b/src/tools/msvc/build.pl
@@ -38,6 +38,7 @@ my $vcver = Mkvcbuild::mkvcbuild($config);
# check what sort of build we are doing
my $bconf = $ENV{CONFIG} || "Release";
+my $msbflags = $ENV{MSBFLAGS} || "";
my $buildwhat = $ARGV[1] || "";
if (uc($ARGV[0]) eq 'DEBUG')
{
@@ -53,16 +54,16 @@ elsif (uc($ARGV[0]) ne "RELEASE")
if ($buildwhat and $vcver >= 10.00)
{
system(
- "msbuild $buildwhat.vcxproj /verbosity:normal /p:Configuration=$bconf"
+ "msbuild $buildwhat.vcxproj $msbflags /verbosity:normal /p:Configuration=$bconf"
);
}
elsif ($buildwhat)
{
- system("vcbuild $buildwhat.vcproj $bconf");
+ system("vcbuild $msbflags $buildwhat.vcproj $bconf");
}
else
{
- system("msbuild pgsql.sln /verbosity:normal /p:Configuration=$bconf");
+ system("msbuild pgsql.sln $msbflags /verbosity:normal /p:Configuration=$bconf");
}
# report status
diff --git a/src/tools/msvc/clean.bat b/src/tools/msvc/clean.bat
index 469b8a2..e21e37f 100755
--- a/src/tools/msvc/clean.bat
+++ b/src/tools/msvc/clean.bat
@@ -107,6 +107,6 @@ REM for /r %%f in (*.sql) do if exist %%f.in del %%f
cd %D%
REM Clean up ecpg regression test files
-msbuild /NoLogo ecpg_regression.proj /t:clean /v:q
+msbuild %MSBFLAGS% /NoLogo ecpg_regression.proj /t:clean /v:q
goto :eof
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index b4f9464..bcf2267 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -138,8 +138,9 @@ sub check
sub ecpgcheck
{
+ my $msbflags = $ENV{MSBFLAGS} || "";
chdir $startdir;
- system("msbuild ecpg_regression.proj /p:config=$Config");
+ system("msbuild ecpg_regression.proj $msbflags /p:config=$Config");
my $status = $? >> 8;
exit $status if $status;
InstallTemp();
--
Re: [HACKERS] Parallel build with MSVC
On Sun, Sep 04, 2016 at 08:26:12PM -0400, Tom Lane wrote: > Noah Misch 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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
Noah Misch 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Wed, Apr 27, 2016 at 08:15:05AM +, Christian Ullrich wrote: > * From: Michael Paquier [mailto:michael.paqu...@gmail.com] > > > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich > > wrote: > > > > * From: Michael Paquier [mailto:michael.paqu...@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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Thu, Apr 28, 2016 at 4:16 PM, Christian Ullrich wrote: > * Michael Paquier wrote: >> On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich >> 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
* Michael Paquier wrote: On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
* From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich > wrote: > > * From: Michael Paquier [mailto:michael.paqu...@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 0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patch Description: 0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patch 0002-Make-gendef.pl-work-in-MSVC-parallel-build.patch Description: 0002-Make-gendef.pl-work-in-MSVC-parallel-build.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich wrote: > * From: Michael Paquier [mailto:michael.paqu...@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 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. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
* From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Tue, Apr 26, 2016 at 10:09 PM, Christian Ullrich > wrote: > > -build DEBUG > +build -c DEBUG > > To build just a single project, for example psql, run the commands: > > build psql > -build DEBUG psql > +build -c DEBUG psql > > > 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 do that (just add the $msbflags as the first argument to vcbuild)? -- Christian 0001-Support-passing-arbitrary-arguments-to-MSBuild.patch Description: 0001-Support-passing-arbitrary-arguments-to-MSBuild.patch 0002-Support-parallel-build-with-MSBuild.patch Description: 0002-Support-parallel-build-with-MSBuild.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Tue, Apr 26, 2016 at 10:09 PM, Christian Ullrich 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. -build DEBUG +build -c DEBUG To build just a single project, for example psql, run the commands: build psql -build DEBUG psql +build -c DEBUG psql 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
