Make the PGO profiling step work with VS2017

Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.

There's also a change to the profiling script to use a copy of
pgosweep.exe from the build directory rather than the one in the
toolchain's directory, this is required in order to be able to use
Swarming for the profiling step (and this is probably cleaner because
it avoid duplicating the logic that find these runtime dependencies).

BUG=719319
Review-Url: https://codereview.chromium.org/2878693002
Cr-Commit-Position: refs/heads/master@{#471299}
Committed: https://chromium.googlesource.com/chromium/src/+/79a9cbda265c5ab725a0a25d725980bd0b376439

[Sébastien Marchand, 2017-05-11 18:39:26]

Description was changed from

==========
Make the PGO profiling step work with VS2017
==========

to

==========
Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.


BUG=719319
==========

[Sébastien Marchand, 2017-05-11 18:39:26]

sebmarchand@chromium.org changed reviewers:
+ brucedawson@chromium.org, scottmg@chromium.org

[Sébastien Marchand, 2017-05-11 18:43:43]

Description was changed from

==========
Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.


BUG=719319
==========

to

==========
Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.

There's also a change to the profiling script to use a copy of
pgosweep.exe from the build directory rather than the one in the
toolchain's directory, this is required in order to be able to use
Swarming for the profiling step (and this is probably cleaner because
it avoid duplicating the logic that find these runtime dependencies).

BUG=719319
==========

[Sébastien Marchand, 2017-05-11 18:43:48]

PTAL

[scottmg, 2017-05-11 19:30:28]

lgtm

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py
File build/vs_toolchain.py (left):

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#o...
build/vs_toolchain.py:258: def CopyVsRuntimeDlls(output_dir, runtime_dirs):
https://cs.chromium.org/search/?q=CopyVsRuntimeDlls+package:%5Echromium$&type=cs
:/ (I think it's fine to delete this though, v8 has its own copy, and I don't
think libyuv's is used except in a standalone build so they'll have to update
something.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:233: #
{toolchain_root}/VC/Tools/MSVC/{x.y.z}/Host{target_cpu}/{target_cpu}, the
This comment is missing a "bin/" I think.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:237: vc_tool_msvc_root =
os.path.join(os.environ.get('GYP_MSVS_OVERRIDE_PATH'),
vc_tools_msvc_root (with an 'S') to match the path name.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:247: pgo_x86_runtime_dir = os.path.join(pgo_runtime_root,
'HostX86', 'x86')
HostX86 should be HostX64, as we only want target x86, right?

... Oh, apparently there's no pgosweep in HostX64\x86. Maybe a comment to that
effect would be good.

[Sébastien Marchand, 2017-05-11 20:15:35]

Thanks, committing.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py
File build/vs_toolchain.py (left):

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#o...
build/vs_toolchain.py:258: def CopyVsRuntimeDlls(output_dir, runtime_dirs):
On 2017/05/11 19:30:28, scottmg wrote:
>
https://cs.chromium.org/search/?q=CopyVsRuntimeDlls+package:%5Echromium$&type=cs
> :/ (I think it's fine to delete this though, v8 has its own copy, and I don't
> think libyuv's is used except in a standalone build so they'll have to update
> something.

Yeah, and the call to this function is behind a "if vs2013_runtime_dll_dirs", so
I really doubt that it's still in use?

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:233: #
{toolchain_root}/VC/Tools/MSVC/{x.y.z}/Host{target_cpu}/{target_cpu}, the
On 2017/05/11 19:30:28, scottmg wrote:
> This comment is missing a "bin/" I think.

yep, fixed.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:237: vc_tool_msvc_root =
os.path.join(os.environ.get('GYP_MSVS_OVERRIDE_PATH'),
On 2017/05/11 19:30:28, scottmg wrote:
> vc_tools_msvc_root (with an 'S') to match the path name.

Done.

https://codereview.chromium.org/2878693002/diff/60001/build/vs_toolchain.py#n...
build/vs_toolchain.py:247: pgo_x86_runtime_dir = os.path.join(pgo_runtime_root,
'HostX86', 'x86')
On 2017/05/11 19:30:28, scottmg wrote:
> HostX86 should be HostX64, as we only want target x86, right?
> 
> ... Oh, apparently there's no pgosweep in HostX64\x86. Maybe a comment to that
> effect would be good.

Done.

[Sébastien Marchand, 2017-05-11 20:15:52]

The CQ bit was checked by sebmarchand@chromium.org

[Sébastien Marchand, 2017-05-11 20:15:52]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2878693002/#ps80001
(title: "Address Scott's comments.")

[commit-bot: I haz the power, 2017-05-11 20:17:13]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 21:19:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 21:19:27]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sébastien Marchand, 2017-05-12 12:39:54]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2017-05-12 12:40:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 15:29:31]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494592794934040,
"parent_rev": "149eb6a66fc6ff7d67627cf8305f833ae50b9b66", "commit_rev":
"79a9cbda265c5ab725a0a25d725980bd0b376439"}

[commit-bot: I haz the power, 2017-05-12 15:29:48]

Description was changed from

==========
Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.

There's also a change to the profiling script to use a copy of
pgosweep.exe from the build directory rather than the one in the
toolchain's directory, this is required in order to be able to use
Swarming for the profiling step (and this is probably cleaner because
it avoid duplicating the logic that find these runtime dependencies).

BUG=719319
==========

to

==========
Make the PGO profiling step work with VS2017

The PGO build is failing for the VS2017 binaries because some of the 
dependencies (pgort140.dll, pgosweep.exe...) have moved to a different 
directory of the toolchain.

There's also a change to the profiling script to use a copy of
pgosweep.exe from the build directory rather than the one in the
toolchain's directory, this is required in order to be able to use
Swarming for the profiling step (and this is probably cleaner because
it avoid duplicating the logic that find these runtime dependencies).

BUG=719319

Review-Url: https://codereview.chromium.org/2878693002
Cr-Commit-Position: refs/heads/master@{#471299}
Committed:
https://chromium.googlesource.com/chromium/src/+/79a9cbda265c5ab725a0a25d7259...
==========

[commit-bot: I haz the power, 2017-05-12 15:29:49]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/79a9cbda265c5ab725a0a25d7259...