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

Issue 817779 link

Starred by 1 user

Issue metadata

Status: Fixed
Merged: issue 812533
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Feature

Blocked on:
issue 819049

Blocking:
issue 757440



Sign in to add a comment

Blink bot for heap verification

Project Member Reported by mlippautz@chromium.org, Mar 1 2018

Issue description

For out ongoing work on Oilpan we would like to request bots for heap verification as we don't want to introduce that burden on all the regular bots. The slowdown can be significant as we iterate and verify consistency of the managed heap after each garbage collection. The benefit can be huge, considering that we actually verify consistency of the full managed heap for all layout tests.

Build-time flag (gn)
-  enable_blink_heap_verification

Configuration:
- CI: Linux64 release
- Try: Linux64 release

Ideally, we would have them with DCHECKs enabled and an increased timeout limit on the tests. I assume that this might not be trivial, so regular release bots should be good enough.

As a starter, having them "FYI" would be good enough but eventually they should be blocking.

Please lmk of any issues with that.
 
Mergedinto: 812533
Status: Duplicate (was: Untriaged)
Already in the process of setting up the FYI bot.
Status: Untriaged (was: Duplicate)
That's a different flag. In the process of doing incremental marking, we would like to first land a verifier that is already useful on ToT.
oh, _verification and not _marking.

Curious: what's the difference, and do they need to be separate bots?
The verification is already useful on ToT but is too costly to put behind regular DEBUG. We just developed this as we see need to verify heap consistency.

The marking flag is for a new feature. This build flag for this one will eventually disappear. We put it behind a build flag for now as there are a few performance issues which we need to get to before shipping. After removing the build time flag for the flag, you can run it with our without the verifier.

Does that make sense?
Sort of, but it doesn't need to be perfectly clear to me if you think it makes sense to run the flags independently.

You mentioned in #0 that these should be blocking eventually; is there an expected EOL for this flag? When you say blocking, are you thinking about CI or try?
The flag mentioned here (verification) is independent of the new feature we are doing. It's a verification flag that is rather costly to execute and I have been running into timeouts and that's why I am requesting separate bots.

The flag should be blocking eventually once we have sorted out all stability issues with it. Depending on resources, I'd like to have it as strict as possible, i.e., blocking on try and closer on CI. If that is not possible it should be closer on CI and optional try bot.

It does not depend on the other (feature) flag.
sorry, I may have phrased #5 poorly -- my intention was to say that, if you think it requires two separate bots (as you clearly mentioned in #4), that's sufficient for us. we don't need to completely understand (though I appreciate the explanation). my question in #3 was mostly meant as a confirmation dialog.

Closer on CI and optional try is the most likely end state due to the capacity requirements of blocking try.
Don't worry :) Cool that this is possible.

The reason this comes up now is that we are working more actively on the garbage collector in Blink these days and would like to catch issues earlier.
Owner: bpastene@chromium.org
Status: Assigned (was: Untriaged)
I can take this. Since this would go on linux waterfall and tryserver, and since we're in the middle of converting both of those to luci, I'll spin these up as luci bots.

Won't be my highest priority, so it might take some time.
Cool, waht's the timeframe for "it might take some time"?
Best guess is ~2 weeks
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 1 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/9e7c7586aa63bac0b9b55f71460c1822792998fc

commit 9e7c7586aa63bac0b9b55f71460c1822792998fc
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 01 22:05:21 2018

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 5 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/dd05e8df286e2a3ed15cce726d49037f73faefa5

commit dd05e8df286e2a3ed15cce726d49037f73faefa5
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Mon Mar 05 22:20:27 2018

Blockedon: 819049
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 7 2018

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

commit 381fb8954bf18b6f82fd5751d772398dfb2410c2
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Wed Mar 07 01:37:59 2018

infra/config: LUCI config dump for new linux-blink bot.

TBR=jbudorick@chromium.org

Bug:  817779 
Change-Id: Idd7988a2a62635ed2155eaaab5c8e375b455f7f6
Reviewed-on: https://chromium-review.googlesource.com/952294
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541276}
[modify] https://crrev.com/381fb8954bf18b6f82fd5751d772398dfb2410c2/infra/config/global/cr-buildbucket.cfg
[modify] https://crrev.com/381fb8954bf18b6f82fd5751d772398dfb2410c2/infra/config/global/luci-milo.cfg
[modify] https://crrev.com/381fb8954bf18b6f82fd5751d772398dfb2410c2/infra/config/global/luci-scheduler.cfg

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 8 2018

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

commit f3d19a8afdeb0f66907e3406226726e98af66539
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 08 03:37:39 2018

Fix typo in luci-scheduler for blink-heap-verification bot.

TBR=jbudorick@chromium.org

Bug:  817779 
Change-Id: I99e51513bc6279ba98409952f20b0dec66a62cd4
Reviewed-on: https://chromium-review.googlesource.com/954607
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541712}
[modify] https://crrev.com/f3d19a8afdeb0f66907e3406226726e98af66539/infra/config/global/luci-scheduler.cfg

What test suites do you want running on the bot? Just the layout tests?
At least the common unit tests and layout tests. 

Ideally, also the GPU tests.
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 8 2018

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

commit 80c2aad9363c8c63e53f81409c72efcc8022afab
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 08 20:19:01 2018

Add chromium recipe config for blink-heap-verification bot.

TBR=jbudorick@chromium.org
Bug:  817779 
Change-Id: Id8b5c4de693f6e8a776a9ac4b3c53605b4c34d5d
Reviewed-on: https://chromium-review.googlesource.com/956302
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>

[modify] https://crrev.com/80c2aad9363c8c63e53f81409c72efcc8022afab/scripts/slave/recipe_modules/chromium_tests/chromium_fyi.py

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 8 2018

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

commit 6227a706f13f228422a335e6855b14f631d81707
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 08 20:34:20 2018

Add mb config for new blink-heap-verification bot.

Bug:  817779 
Change-Id: I42fc05273e682c82b280b0cad051a129c2dd892b
Reviewed-on: https://chromium-review.googlesource.com/954705
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541880}
[modify] https://crrev.com/6227a706f13f228422a335e6855b14f631d81707/tools/mb/mb_config.pyl

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 10 2018

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

commit bf1f4a66aba5aed5f7ea4e70e09384e5d264efaa
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Sat Mar 10 03:31:29 2018

Add layout tests to blink heap bot.

Bug:  817779 
Change-Id: Ie45ebfb14d2bdeeb80724ec94d1fce74cc64b8a8
Reviewed-on: https://chromium-review.googlesource.com/957648
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542336}
[modify] https://crrev.com/bf1f4a66aba5aed5f7ea4e70e09384e5d264efaa/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/bf1f4a66aba5aed5f7ea4e70e09384e5d264efaa/testing/buildbot/waterfalls.pyl

Just checked on the CI and it is already doing what it should do, awesome!

Two questions:
1. We improved performance quite a bit over the last two weeks and would now also get away with DCHECKs enabled. Can we do that?
2. Unit tests seem to be missing, is that expected?
Just added layout the tests for now. When you say you want the unit tests, are you referring to the suite "unit_tests" or a specific set of suites? If the latter, would the list defined here be sufficient? (I have to ask cause we have *a lot* of tests)
https://chromium.googlesource.com/chromium/src/+/b890ea37ba241e3d4dfc4189e59191c18b0d5dfd/testing/buildbot/test_suites.pyl#366

And I'll add dchecks.
Yep, I meant a subset of "chromium_gtests" but the group looks good.

Thanks for adding the dchecks!
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 12 2018

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

commit 6e82511e0b1bc110bff04becde1df18bdc061a2b
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Mon Mar 12 20:12:22 2018

Enable DCHECKS for blink-heap-verification bot.

Bug:  817779 
Change-Id: If4599943937ea5c628bfc3cc96f5df9b9b4065f1
Reviewed-on: https://chromium-review.googlesource.com/959345
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542578}
[modify] https://crrev.com/6e82511e0b1bc110bff04becde1df18bdc061a2b/tools/mb/mb_config.pyl

Project Member

Comment 26 by bugdroid1@chromium.org, Mar 13 2018

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

commit f09b5d038ae055a05caaf7da1fbc3a76168f50a7
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Tue Mar 13 00:02:12 2018

Add chromium gtests to blink heap verification bot.

Bug:  817779 
Change-Id: I652c928db764e106a5e94a3bbcce6f11cd9f2d87
Reviewed-on: https://chromium-review.googlesource.com/959593
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542659}
[modify] https://crrev.com/f09b5d038ae055a05caaf7da1fbc3a76168f50a7/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/f09b5d038ae055a05caaf7da1fbc3a76168f50a7/testing/buildbot/waterfalls.pyl

Thanks for all that work.

I noticed that the bot works but it doesn't give a bubble on the waterfall UI (https://ci.chromium.org/p/chromium/g/chromium.fyi/console). Is it because it is experimental?
That's why both of the linux-blink-heap-* bots have bubbles but no builds, yeah. I've marked both as non-experimental in luci-migration, so builds should start showing up.
The CI bot looks good: https://luci-milo.appspot.com/p/chromium/builders/luci.chromium.ci/linux-blink-heap-verification

Unless I'm missing something, I'll get the optional/non-CQ trybot up.
Yes, thanks a lot!
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 14 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/b917d51d8b19c0b0b4b64d635b78d409fe2a4c1d

commit b917d51d8b19c0b0b4b64d635b78d409fe2a4c1d
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Wed Mar 14 18:33:08 2018

Project Member

Comment 32 by bugdroid1@chromium.org, Mar 14 2018

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

commit e2add0099a3844985d9a37422fca4dfef68633bd
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Wed Mar 14 19:48:09 2018

Luci config dump for blink-heap-verification trybot.

Bug:  817779 
Change-Id: I9fc5b1f51d536262f22ba5f412e8cf94ed3dae42
Reviewed-on: https://chromium-review.googlesource.com/963301
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543163}
[modify] https://crrev.com/e2add0099a3844985d9a37422fca4dfef68633bd/infra/config/global/cr-buildbucket.cfg
[modify] https://crrev.com/e2add0099a3844985d9a37422fca4dfef68633bd/infra/config/global/luci-milo.cfg

Project Member

Comment 33 by bugdroid1@chromium.org, Mar 14 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/2e031250e7840111718cb828b5349324e1196d6d

commit 2e031250e7840111718cb828b5349324e1196d6d
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Wed Mar 14 20:42:44 2018

Project Member

Comment 34 by bugdroid1@chromium.org, Mar 15 2018

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

commit d726060ccf4a602fef8afd5943d36be1e00dc64a
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 15 18:33:49 2018

Add recipe config for blink-heap-verification trybot.

TBR=jbudorick@chromium.org
Bug:  817779 
Change-Id: Ie0e020748f92c202ad69f39a2a3debfd809548fb
Reviewed-on: https://chromium-review.googlesource.com/962968
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>

[modify] https://crrev.com/d726060ccf4a602fef8afd5943d36be1e00dc64a/tests/masters_recipes_test.py
[modify] https://crrev.com/d726060ccf4a602fef8afd5943d36be1e00dc64a/scripts/slave/recipe_modules/chromium_tests/trybots.py

Project Member

Comment 35 by bugdroid1@chromium.org, Mar 16 2018

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

commit e51edcd3b1c47458b660773d3fc10716881efe38
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Fri Mar 16 00:02:55 2018

Add mb config for blink heap trybot.

Bug:  817779 
Change-Id: I49f8b897014877b152d6790d41aad25e450433a3
Reviewed-on: https://chromium-review.googlesource.com/963091
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543560}
[modify] https://crrev.com/e51edcd3b1c47458b660773d3fc10716881efe38/tools/mb/mb_config.pyl

Ping :)
Cc: dpranke@chromium.org jbudorick@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/981046 adds the trybot to the gerrit "select trybot" popup

After that, both CI bot and trybot will be setup:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-blink-heap-verification
https://luci-milo.appspot.com/p/chromium/builders/luci.chromium.try/linux-blink-heap-verification-try

If you'd like to promote it to waterfall-closing CI, you'd have to get buy-in from leads (prob dpranke@ and jbudorick@). Once that's done I can move it over.
Thanks! 

It looks like it's mostly green (broke the mode locally a few times). I think it is fine as fyi for now but good to know who to talk to if we want to make it closing.
Status: Fixed (was: Assigned)
The trybot's up in the gerrit ui, and tested in https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-blink-heap-verification-try/1

Given that you're ok with this staying as fyi for now, I'm going to close this out. If/when you want to promote it, feel free to file another bug.

And one last thing: if you want to get email alerts when the CI bot fails, you can add an entry to gatekeeper:
https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/gatekeeper.json

That should be mostly self-service, so I'll leave it to you to follow up on that.
Project Member

Comment 40 by bugdroid1@chromium.org, Apr 3 2018

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

commit 127a5c5a4b331e8356d5e1108f9d10f0c1ad3107
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Tue Apr 03 11:21:35 2018

Add notification for linux-blink-heap-verification bot

Bug:  chromium:817779 
Change-Id: Ia6932a114806d6714df58c38ea5db9a7d79f489f
Reviewed-on: https://chromium-review.googlesource.com/986261
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>

[modify] https://crrev.com/127a5c5a4b331e8356d5e1108f9d10f0c1ad3107/scripts/slave/gatekeeper.json

Project Member

Comment 41 by bugdroid1@chromium.org, Jul 16

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

commit 0d4d0331a15e11f7d6ca819ca28521f24ca4647f
Author: Nico Weber <thakis@chromium.org>
Date: Mon Jul 16 17:51:50 2018

Move unit_tests from chromium_gtests to chromium_gtests_for_devices_with_graphical_output.

chromium_gtests_for_devices_with_graphical_output means "not cast".
The cast build doesn't include anything at the //chrome level, and
unit_tests lives at the //chrome level.

Currently we have unit_tests in chromium_gtests which runs on all bots,
and then an exception to remove it again on the cast bots.

Instead, put it in chromium_gtests_for_devices_with_graphical_output so that
it only runs on non-cast bots in the first place.

(It also ends up removing unit_tests from linux-blink-heap-incremental-marking
and linux-blink-heap-verification on the fyi waterfall, since these bots run
chromium_gtests instead of chromium_linux_gtests -- that looks like a bot
config bug, but it's only FYI bots anyways.)

This removes the last "unclear why" comment from test_suite*.pyl!

Bug: 843511, 817779 , 812533 
Change-Id: I76e0a2b30e0ca87c02c73cb46d8135d9ab7030d8
Reviewed-on: https://chromium-review.googlesource.com/1138333
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575339}
[modify] https://crrev.com/0d4d0331a15e11f7d6ca819ca28521f24ca4647f/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/0d4d0331a15e11f7d6ca819ca28521f24ca4647f/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/0d4d0331a15e11f7d6ca819ca28521f24ca4647f/testing/buildbot/test_suites.pyl

Sign in to add a comment