Parallel build with MSVC

Started by Christian Ullrichover 9 years ago17 messages

chris@chrullrich.net

over 9 years ago

2 attachment(s)

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

Attachments:

0001-Support-passing-arbitrary-arguments-to-MSBuild.patchtext/plain; charset=UTF-8; name=0001-Support-passing-arbitrary-arguments-to-MSBuild.patchDownload

From 4b455e1ac38f1441f1faf695da4be8c5eb478970 Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Tue, 26 Apr 2016 14:52:27 +0200
Subject: [PATCH 1/2] Support passing arbitrary arguments to MSBuild.

This is particularly useful to pass /m, to perform a parallel build.
---
 doc/src/sgml/install-windows.sgml | 11 +++++++++--
 src/tools/msvc/build.pl           |  5 +++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 74265fc..2b9cf1a 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -137,6 +137,13 @@ $config->{python} = 'c:\python26';
 $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
 </programlisting>
  </para>
+ <para>
+  To pass additional command line arguments to msbuild (Visual Studio 2010
+  and later):
+<programlisting>
+$ENV{MSBFLAGS}="/m";
+</programlisting>
+ </para>
 
  <sect2>
   <title>Requirements</title>
@@ -361,12 +368,12 @@ $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
 </screen>
    To build all of PostgreSQL in debug configuration, run the command:
 <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>
    To change the default build configuration to debug, put the following
    in the <filename>buildenv.pl</filename> file:
diff --git a/src/tools/msvc/build.pl b/src/tools/msvc/build.pl
index c4e4dc7..85109a2 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,7 +54,7 @@ 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)
 {
@@ -61,7 +62,7 @@ elsif ($buildwhat)
 }
 else
 {
-	system("msbuild pgsql.sln /verbosity:normal /p:Configuration=$bconf");
+	system("msbuild pgsql.sln $msbflags /verbosity:normal /p:Configuration=$bconf");
 }
 
 # report status
-- 
2.8.1.windows.1

0002-Support-parallel-build-with-MSBuild.patchtext/plain; charset=UTF-8; name=0002-Support-parallel-build-with-MSBuild.patchDownload

From 51cccd968b5ba98da4d971d9c69b59ec85cfa513 Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Tue, 26 Apr 2016 14:52:53 +0200
Subject: [PATCH 2/2] Support parallel build with MSBuild.

gendef.pl now puts the temporary output file into the target directory instead of the source tree root, to avoid collisions.
---
 src/tools/msvc/gendef.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index 8ccaab3..4bd05fa 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 use 5.8.0;
 use List::Util qw(max);
+use File::Spec::Functions qw(splitpath catpath);
 
 #
 # Script that generates a .DEF file for all objects in a directory
@@ -14,9 +15,11 @@ use List::Util qw(max);
 sub dumpsyms
 {
 	my ($objfile, $symfile) = @_;
-	system("dumpbin /symbols /out:symbols.out $_ >NUL")
+	my ($symvol, $symdirs, $symbase) = splitpath($symfile);
+	my $tmpfile = catpath($symvol, $symdirs, "symbols.out");
+	system("dumpbin /symbols /out:$tmpfile $_ >NUL")
 	  && die "Could not call dumpbin";
-	rename("symbols.out", $symfile);
+	rename($tmpfile, $symfile);
 }
 
 # Given a symbol file path, loops over its contents
-- 
2.8.1.windows.1

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Christian Ullrich (#1)

Re: Parallel build with MSVC

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>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.

+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

Christian Ullrich

chris@chrullrich.net

over 9 years ago

In reply to: Michael Paquier (#2)

2 attachment(s)

Re: Parallel build with MSVC

* 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 do that (just add the $msbflags as the first argument to vcbuild)?

--
Christian

Attachments:

0001-Support-passing-arbitrary-arguments-to-MSBuild.patchapplication/octet-stream; name=0001-Support-passing-arbitrary-arguments-to-MSBuild.patchDownload

From d4e014326ff1f30539cc73f2edbf266356dd1bcb Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Wed, 27 Apr 2016 08:57:08 +0200
Subject: [PATCH 1/2] Support passing arbitrary arguments to MSBuild.

This is particularly useful to pass /m, to perform a parallel build.
---
 doc/src/sgml/install-windows.sgml | 7 +++++++
 src/tools/msvc/build.pl           | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 74265fc..0e6f99e 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -137,6 +137,13 @@ $config->{python} = 'c:\python26';
 $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
 </programlisting>
  </para>
+ <para>
+  To pass additional command line arguments to msbuild (Visual Studio 2010
+  and later):
+<programlisting>
+$ENV{MSBFLAGS}="/m";
+</programlisting>
+ </para>
 
  <sect2>
   <title>Requirements</title>
diff --git a/src/tools/msvc/build.pl b/src/tools/msvc/build.pl
index c4e4dc7..85109a2 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,7 +54,7 @@ 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)
 {
@@ -61,7 +62,7 @@ elsif ($buildwhat)
 }
 else
 {
-	system("msbuild pgsql.sln /verbosity:normal /p:Configuration=$bconf");
+	system("msbuild pgsql.sln $msbflags /verbosity:normal /p:Configuration=$bconf");
 }
 
 # report status
-- 
2.8.1.windows.1

0002-Support-parallel-build-with-MSBuild.patchapplication/octet-stream; name=0002-Support-parallel-build-with-MSBuild.patchDownload

From 878440f12541b4bb337e0133af63f06af5daa549 Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Tue, 26 Apr 2016 14:52:53 +0200
Subject: [PATCH 2/2] Support parallel build with MSBuild.

gendef.pl now puts the temporary output file into the target directory instead of the source tree root, to avoid collisions.
---
 src/tools/msvc/gendef.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index 8ccaab3..4bd05fa 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 use 5.8.0;
 use List::Util qw(max);
+use File::Spec::Functions qw(splitpath catpath);
 
 #
 # Script that generates a .DEF file for all objects in a directory
@@ -14,9 +15,11 @@ use List::Util qw(max);
 sub dumpsyms
 {
 	my ($objfile, $symfile) = @_;
-	system("dumpbin /symbols /out:symbols.out $_ >NUL")
+	my ($symvol, $symdirs, $symbase) = splitpath($symfile);
+	my $tmpfile = catpath($symvol, $symdirs, "symbols.out");
+	system("dumpbin /symbols /out:$tmpfile $_ >NUL")
 	  && die "Could not call dumpbin";
-	rename("symbols.out", $symfile);
+	rename($tmpfile, $symfile);
 }
 
 # Given a symbol file path, loops over its contents
-- 
2.8.1.windows.1

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Christian Ullrich (#3)

Re: Parallel build with MSVC

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 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

Christian Ullrich

chris@chrullrich.net

over 9 years ago

In reply to: Michael Paquier (#4)

2 attachment(s)

Re: Parallel build with MSVC

* 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

Attachments:

0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patchapplication/octet-stream; name=0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patchDownload

From fecea54d91988cb664530d4b6398b87bf9ee018f Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Wed, 27 Apr 2016 08:57:08 +0200
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 | 7 +++++++
 src/tools/msvc/build.pl           | 7 ++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 74265fc..0097463 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -137,6 +137,13 @@ $config->{python} = 'c:\python26';
 $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
 </programlisting>
  </para>
+ <para>
+  To pass additional command line arguments to the Visual Studio build
+  command (msbuild or vcbuild):
+<programlisting>
+$ENV{MSBFLAGS}="/m";
+</programlisting>
+ </para>
 
  <sect2>
   <title>Requirements</title>
diff --git a/src/tools/msvc/build.pl b/src/tools/msvc/build.pl
index c4e4dc7..e25a689 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,15 +54,15 @@ 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
-- 
2.8.1.windows.1

0002-Make-gendef.pl-work-in-MSVC-parallel-build.patchapplication/octet-stream; name=0002-Make-gendef.pl-work-in-MSVC-parallel-build.patchDownload

From 48c5b5a276630baf9169832b2c452b407c24241d Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Tue, 26 Apr 2016 14:52:53 +0200
Subject: [PATCH 2/2] Make gendef.pl work in MSVC parallel build

gendef.pl now puts the temporary output file into the target directory instead of the source tree root, to avoid collisions.
---
 src/tools/msvc/gendef.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index 8ccaab3..4bd05fa 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 use 5.8.0;
 use List::Util qw(max);
+use File::Spec::Functions qw(splitpath catpath);
 
 #
 # Script that generates a .DEF file for all objects in a directory
@@ -14,9 +15,11 @@ use List::Util qw(max);
 sub dumpsyms
 {
 	my ($objfile, $symfile) = @_;
-	system("dumpbin /symbols /out:symbols.out $_ >NUL")
+	my ($symvol, $symdirs, $symbase) = splitpath($symfile);
+	my $tmpfile = catpath($symvol, $symdirs, "symbols.out");
+	system("dumpbin /symbols /out:$tmpfile $_ >NUL")
 	  && die "Could not call dumpbin";
-	rename("symbols.out", $symfile);
+	rename($tmpfile, $symfile);
 }
 
 # Given a symbol file path, loops over its contents
-- 
2.8.1.windows.1

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Christian Ullrich (#5)

Re: Parallel build with MSVC

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Christian Ullrich

chris@chrullrich.net

over 9 years ago

In reply to: Michael Paquier (#6)

Re: Parallel build with MSVC

* 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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Christian Ullrich (#7)

Re: Parallel build with MSVC

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Noah Misch

noah@leadboat.com

over 9 years ago

In reply to: Christian Ullrich (#5)

Re: Parallel build with MSVC

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Tom Lane

tgl@sss.pgh.pa.us

over 9 years ago

In reply to: Noah Misch (#9)

Re: Parallel build with MSVC

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Noah Misch

noah@leadboat.com

over 9 years ago

In reply to: Tom Lane (#10)

Re: Parallel build with MSVC

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Noah Misch (#9)

2 attachment(s)

Re: Parallel build with MSVC

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

Attachments:

0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patchtext/x-diff; charset=US-ASCII; name=0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patchDownload

From aaf028d2fab130d908a4efcd3c6316b4e2c3fdd7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
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';
 </programlisting>
  </para>
 
+ <para>
+  To pass additional command line arguments to the Visual Studio build
+  command (msbuild or vcbuild):
+<programlisting>
+$ENV{MSBFLAGS}="/m";
+</programlisting>
+ </para>
+
  <sect2>
   <title>Requirements</title>
   <para>
@@ -401,7 +409,8 @@ $ENV{CONFIG}="Debug";
    all generated files. You can also run it with the
    <parameter>dist</parameter> parameter, in which case it will behave like
    <userinput>make distclean</userinput> 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 <literal>MSBFLAGS</>.
   </para>
 
   <para>
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();
-- 
2.9.3

0002-Make-gendef.pl-work-in-MSVC-parallel-build.patchtext/x-diff; charset=US-ASCII; name=0002-Make-gendef.pl-work-in-MSVC-parallel-build.patchDownload

From 03674527c26c1baf3a7dcc2032b8495d9f532b22 Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
Date: Tue, 26 Apr 2016 14:52:53 +0200
Subject: [PATCH 2/2] Make gendef.pl work in MSVC parallel build

gendef.pl now puts the temporary output file into the target directory instead of the source tree root, to avoid collisions.
---
 src/tools/msvc/gendef.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index 8ccaab3..4bd05fa 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -4,6 +4,7 @@ use warnings;
 use strict;
 use 5.8.0;
 use List::Util qw(max);
+use File::Spec::Functions qw(splitpath catpath);
 
 #
 # Script that generates a .DEF file for all objects in a directory
@@ -14,9 +15,11 @@ use List::Util qw(max);
 sub dumpsyms
 {
 	my ($objfile, $symfile) = @_;
-	system("dumpbin /symbols /out:symbols.out $_ >NUL")
+	my ($symvol, $symdirs, $symbase) = splitpath($symfile);
+	my $tmpfile = catpath($symvol, $symdirs, "symbols.out");
+	system("dumpbin /symbols /out:$tmpfile $_ >NUL")
 	  && die "Could not call dumpbin";
-	rename("symbols.out", $symfile);
+	rename($tmpfile, $symfile);
 }
 
 # Given a symbol file path, loops over its contents
-- 
2.9.3

#13

Christian Ullrich

chris@chrullrich.net

over 9 years ago

In reply to: Michael Paquier (#12)

Re: Parallel build with MSVC

* 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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14

Noah Misch

noah@leadboat.com

over 9 years ago

In reply to: Michael Paquier (#12)

Re: 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 <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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15

Christian Ullrich

chris@chrullrich.net

over 9 years ago

In reply to: Noah Misch (#14)

1 attachment(s)

Re: 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

Attachments:

0001-Let-MSBFLAGS-be-used-to-override-predefined-options.patchtext/plain; charset=UTF-8; name=0001-Let-MSBFLAGS-be-used-to-override-predefined-options.patchDownload

From 09a5f3945e2b87b56ca3525a56db970e9ecf8ffd Mon Sep 17 00:00:00 2001
From: Christian Ullrich <chris@chrullrich.net>
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

#16

Michael Paquier

michael.paquier@gmail.com

over 9 years ago

In reply to: Christian Ullrich (#15)

Re: Parallel build with MSVC

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17

Noah Misch

noah@leadboat.com

about 9 years ago

In reply to: Christian Ullrich (#15)

Re: 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