New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 633693 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Make Dr. Memory builders work w/ GN builds

Project Member Reported by dpranke@chromium.org, Aug 2 2016

Issue description

We need to make the Dr. Memory builders work w/ GN builds:

- chromium.memory.full
  - Chromium Windows Builder (DrMemory)
  - Chromium Windows Builder (DrMemory x64)
- chromium.fyi
  - Win LKGR (DrM)
  - Win LKGR (DrM 64)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6

commit eeffc455db326d3267ef0d10f2fce6a2dcdeedb6
Author: dpranke <dpranke@chromium.org>
Date: Tue Aug 02 21:55:21 2016

Make sure the `target` parameter has been propagated in GclientFactory.

The DrMemory builders are still not on recipes, so they're using
the old factory, which isn't passing the target=Release_x64 value
along properly, breaking the `generate_build_files` step.

R=iannucci@chromium.org
BUG= 633693 

Review-Url: https://codereview.chromium.org/2206763002

[modify] https://crrev.com/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6/scripts/master/factory/commands.py
[modify] https://crrev.com/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6/scripts/master/factory/gclient_factory.py

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6

commit eeffc455db326d3267ef0d10f2fce6a2dcdeedb6
Author: dpranke <dpranke@chromium.org>
Date: Tue Aug 02 21:55:21 2016

Make sure the `target` parameter has been propagated in GclientFactory.

The DrMemory builders are still not on recipes, so they're using
the old factory, which isn't passing the target=Release_x64 value
along properly, breaking the `generate_build_files` step.

R=iannucci@chromium.org
BUG= 633693 

Review-Url: https://codereview.chromium.org/2206763002

[modify] https://crrev.com/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6/scripts/master/factory/commands.py
[modify] https://crrev.com/eeffc455db326d3267ef0d10f2fce6a2dcdeedb6/scripts/master/factory/gclient_factory.py

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 2 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/master-manager.git/+/b7aba352a10370356415a35e64587e50f9bd3ac9

commit b7aba352a10370356415a35e64587e50f9bd3ac9
Author: dpranke <dpranke@google.com>
Date: Tue Aug 02 22:21:18 2016

Cc: infe...@chromium.org
per discussion w/ inferno@ and mbarbella@, we're not actually using the FYI builders for anything and so will shut them off instead.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2 2016

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/a3014a36fb46babfd47b166251df626902993437

commit a3014a36fb46babfd47b166251df626902993437
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 02 23:44:29 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/0c374c7a9333133257572ef462d6023c6c0100a5 Remove FYI DrMemory clusterfuzz builders. (dpranke@chromium.org)

R=dpranke@chromium.org,mbarbella@chromium.org,inferno@chromium.org
BUG= 633693 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2209523002

[modify] https://crrev.com/a3014a36fb46babfd47b166251df626902993437/infra/config/recipes.cfg

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f55a4dad80de4037ca91972d4288461e616e405

commit 3f55a4dad80de4037ca91972d4288461e616e405
Author: dpranke <dpranke@chromium.org>
Date: Wed Aug 03 00:21:19 2016

Add builder targets needed for DrMem builders.

TBR=groby@chromium.org
BUG= 633693 

Review-Url: https://codereview.chromium.org/2203093004
Cr-Commit-Position: refs/heads/master@{#409396}

[modify] https://crrev.com/3f55a4dad80de4037ca91972d4288461e616e405/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b8bb5421b65b1767d852b02be69001eccfe1818

commit 9b8bb5421b65b1767d852b02be69001eccfe1818
Author: dpranke <dpranke@chromium.org>
Date: Wed Aug 03 23:38:55 2016

Add binaries that were missing from the GN DrM build target.

I missed three when I added the target yesterday :(.

TBR=groby@chromium.org
BUG= 633693 

Review-Url: https://codereview.chromium.org/2210613002
Cr-Commit-Position: refs/heads/master@{#409664}

[modify] https://crrev.com/9b8bb5421b65b1767d852b02be69001eccfe1818/BUILD.gn

I think the builds are now at least kind of working (and will get better when #409664 cycles in); the testers are reporting errors.

I have not yet looked to see how many different errors there are (there's a huge amount of red), as a lot of the red will be due to missing test binaries.

It's likely that we're missing some flags that were set in the GYP build; I'm not sure how many of them are needed and/or the errors we're seeing will be due to those flags missing.

I think at some point very soon it'll make more sense to pass this to the memory sheriffs, who are more used to triaging and fixing issues, but I want to look at at least some more things before I'm ready to hand off.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c24d26fa15ce41e869c719d572e10deb3d21a31

commit 7c24d26fa15ce41e869c719d572e10deb3d21a31
Author: dpranke <dpranke@chromium.org>
Date: Thu Aug 04 19:44:09 2016

Skip most tests that are currently broken w/ DrMemory.

After the switch from GYP->GN, some tests are passing, but most
are still broken. In order to green up the tree, we'll skip the
broken tests for now, and gradually re-enable them.

R=thestig@chromium.org, groby@chromium.org
NOTRY=true
BUG= 633693 

Review-Url: https://codereview.chromium.org/2218513002
Cr-Commit-Position: refs/heads/master@{#409869}

[modify] https://crrev.com/7c24d26fa15ce41e869c719d572e10deb3d21a31/tools/valgrind/chrome_tests.py

Labels: Stability-Memory-DrMemory
Dirk, the link time of the Dr. Memory builder has regressed recently ( issue 640185 ), any idea what could have gone wrong?
Cc: brucedaw...@chromium.org
It's likely that the GN link time is slower than the GYP link time was, but I wouldn't expect it to be *too* much slower. 

Do we have any idea if the step time has been slow since the transition three weeks ago, or if this is a more recent thing?
That's a more recent thing, the first failing build is https://build.chromium.org/p/chromium.memory.full/builders/Chromium%20Windows%20Builder%20%28DrMemory%29/builds/403, but there's no obvious reason.
Hm, build25-m1 only has 12GB of memory. That's weirdly low, and certainly lower than we'd want a windows builder to normally have. That seems unlikely to have changed recently, but maybe something else has changed in the way we're scheduling concurrent links that is causing the builder to thrash down to its knees.

I'd file a sub-bug to have labs look into adding more memory.
Status: Fixed (was: Started)
I'm going to close this as working, since the core stuff more-or-less works.

Obviously, we should re-enable the test suites that are currently disabled at some point soon if we wish to continue supporting these builders, but that requires someone to be willing to spend more time on it. I've filed  bug 642217  for that.

Sign in to add a comment