Add a TSAN bot that runs V8 with concurrent marking enabled |
||||||||||||
Issue descriptionWe need TSAN coverage for concurrent marking in V8. ulan@ will add a GN flag that enables concurrent marking in V8. machenbach@ will add a TSAN bot build and runs V8 with the flag on.
,
May 17 2017
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608 commit f1e82a2ee9d339f86b47cd4507c0a58f9c06e608 Author: ulan <ulan@chromium.org> Date: Wed May 17 15:22:38 2017 [heap] Add GN flag for enabling concurrent marking. BUG= chromium:723600 Review-Url: https://codereview.chromium.org/2888093003 Cr-Commit-Position: refs/heads/master@{#45379} [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/BUILD.gn [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/flag-definitions.h [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/globals.h [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/heap/concurrent-marking.cc [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/heap/heap.cc [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/heap/incremental-marking.cc [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/heap/incremental-marking.h [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/heap/mark-compact.h [modify] https://crrev.com/f1e82a2ee9d339f86b47cd4507c0a58f9c06e608/src/objects-inl.h
,
May 17 2017
GN flag has landed: v8_enable_concurrent_marking = true
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/9fd599ef98ce3dbe978732761b77c51eb41a506d commit 9fd599ef98ce3dbe978732761b77c51eb41a506d Author: Michael Achenbach <machenbach@chromium.org> Date: Thu May 18 12:11:20 2017 [MB] Add concurrent marking bots Add configs for: https://chromium-review.googlesource.com/c/508349 NOTRY=true TBR=ulan@chromium.org Bug: chromium:723600 Change-Id: Ie0be3d34cc35a72c012c601d0bf8c8b707e69f32 Reviewed-on: https://chromium-review.googlesource.com/508628 Commit-Queue: Michael Achenbach <machenbach@chromium.org> Reviewed-by: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#45396} [modify] https://crrev.com/9fd599ef98ce3dbe978732761b77c51eb41a506d/infra/mb/mb_config.pyl
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/8e9cb844bc627074c4aa50e1255cb468241dc2f6 commit 8e9cb844bc627074c4aa50e1255cb468241dc2f6 Author: Michael Achenbach <machenbach@chromium.org> Date: Thu May 18 13:06:33 2017 V8: Add concurrent marking tsan bots This adds bots to CI and CQ. The CI slaves are leftovers from: http://crbug.com/682634 V8-side MB change: https://chromium-review.googlesource.com/c/508628/ Bug: chromium:723600 Change-Id: Iba8af65b578a943eada7cd9b28b170a86cfe6908 Reviewed-on: https://chromium-review.googlesource.com/508349 Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> [add] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_concurrent_marking_rel_ng.json [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/masters/master.client.v8/slaves.cfg [add] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/scripts/slave/recipes/v8.expected/full_client_v8_V8_Linux64_TSAN___concurrent_marking.json [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/masters/master.client.v8/master.cfg [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/scripts/slave/recipe_modules/v8/builders.py [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/scripts/slave/v8/gatekeeper_v8_tree_closers.json [add] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_concurrent_marking_rel_ng_triggered.json [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/masters/master.tryserver.v8/master.cfg [modify] https://crrev.com/8e9cb844bc627074c4aa50e1255cb468241dc2f6/masters/master.tryserver.v8/slaves.cfg
,
May 18 2017
,
May 19 2017
Bot is live but red: https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN%20-%20concurrent%20marking We can make it tree-closing as soon as it's reliable. Optional trybot is here: https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_tsan_concurrent_marking_rel_ng E.g. git cl try -b v8_linux64_tsan_concurrent_marking_rel_ng -m tryserver.v8
,
May 19 2017
Thanks a lot!
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/898eb142cea3ee727d9b642faf7f382162d80881 commit 898eb142cea3ee727d9b642faf7f382162d80881 Author: Ulan Degenbaev <ulan@chromium.org> Date: Tue May 30 12:27:00 2017 [heap] Adjust live bytes atomically when concurrent marking is on. BUG= chromium:723600 Change-Id: I7fbc9cbeac2bd3d826d81808c0f3c2c24a21a562 Reviewed-on: https://chromium-review.googlesource.com/518013 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#45596} [modify] https://crrev.com/898eb142cea3ee727d9b642faf7f382162d80881/src/heap/heap.cc
,
Jun 14 2017
Grabbing this again to extend test coverage on the bot...
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9 commit cc5074b3d9e5a81efde449ddc4673d03f34c8ee9 Author: Michael Achenbach <machenbach@chromium.org> Date: Wed Jun 14 11:55:52 2017 V8: Add more tests to TSAN bots TBR=tandrii@chromium.org Bug: 723600 Change-Id: I7c506d88071235421b149b10835d204fbf53f288 Reviewed-on: https://chromium-review.googlesource.com/535557 Reviewed-by: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_concurrent_marking_rel_ng.json [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipes/v8.expected/full_client_v8_V8_Linux64_TSAN___concurrent_marking.json [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_rel.json [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipe_modules/v8/builders.py [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipes/v8.expected/full_client_v8_V8_Linux64_TSAN.json [modify] https://crrev.com/cc5074b3d9e5a81efde449ddc4673d03f34c8ee9/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_concurrent_marking_rel_ng_triggered.json
,
Jun 14 2017
@kbr: Would there be GPU capacity to add a V8 low-prio FYI bot similar to: https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20GPU%20TSAN%20Release We'd like to run this bot side-by-side with the others on https://build.chromium.org/p/client.v8.fyi/console and with concurrent marking enabled.
,
Jun 15 2017
machenbach@: yes, adding a new waterfall bot is generally not a problem, as long as you specify: 'serialize_tests': True, as in for example https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/chromium_gpu_fyi.py?q=chromium_gpu_fyi+package:%5Echromium$&l=1 . Feel free to send me CLs modifying tools/build/scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py and src/content/test/gpu/generate_buildbot_json.py (and the files it generates).
,
Jun 26 2017
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/16793f9b7ca677b0db3510a573051a95e120dd6e commit 16793f9b7ca677b0db3510a573051a95e120dd6e Author: Michael Achenbach <machenbach@chromium.org> Date: Mon Jun 26 09:30:33 2017 V8: Add GPU TSAN fyi bot for testing concurrent marking Bug: 723600 Change-Id: I730168ecedf2957a30ac96893bff136113b040ca Reviewed-on: https://chromium-review.googlesource.com/548015 Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> [modify] https://crrev.com/16793f9b7ca677b0db3510a573051a95e120dd6e/scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py [modify] https://crrev.com/16793f9b7ca677b0db3510a573051a95e120dd6e/masters/master.client.v8.fyi/master.cfg [modify] https://crrev.com/16793f9b7ca677b0db3510a573051a95e120dd6e/masters/master.client.v8.fyi/slaves.cfg
,
Jun 26 2017
Note that we had difficulty deploying any of the browser-based GPU tests on the TSAN bot on the chromium.gpu.fyi waterfall; see Issue 691029 . Let me block this on the other bug though we may want to deploy non-browser-based tests on the client.v8.fyi waterfall and turn around the blocked on/blocking relationship.
,
Jun 26 2017
Note that if you have d8-based tests you'd like to run on this new bot, let's modify generate_buildbot_json.py to autogenerate the entries for it. These wouldn't / shouldn't run on the normal GPU bots though.
,
Jun 26 2017
Unfortunately the browser still doesn't start successfully under TSAN, at least as far as I can tell; please see https://bugs.chromium.org/p/chromium/issues/detail?id=691029#c38 . Not sure what binaries the V8 team wants to run; the full browser, or a smaller test harness.
,
Jun 27 2017
As long as this is blocked, Ulan, how about a normal chromium linux non-tsan gpu tester with concurrent marking? I.e. a copy of: https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20(NVIDIA) It'd have dchecks enabled and maybe flushes out some bugs...
,
Jun 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/44ac69e01170e5be129cef2b255c98114b5460e3 commit 44ac69e01170e5be129cef2b255c98114b5460e3 Author: Michael Achenbach <machenbach@chromium.org> Date: Tue Jun 27 07:58:16 2017 V8: Turn GPU TSAN bot into normal linux bot This is just a rename of the bot. The actual configs live in chromium. The bot is now an exact copy of Linux Release (NVIDIA). This also uses the right slave now from http://crbug.com/736711. TBR=phajdan.jr@chromium.org Bug: 723600 ,736711 Change-Id: I06e4d33e8a05a6d368f1239ecf7037eff87d4321 Reviewed-on: https://chromium-review.googlesource.com/549357 Commit-Queue: Michael Achenbach <machenbach@chromium.org> Reviewed-by: Michael Achenbach <machenbach@chromium.org> [modify] https://crrev.com/44ac69e01170e5be129cef2b255c98114b5460e3/scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py [modify] https://crrev.com/44ac69e01170e5be129cef2b255c98114b5460e3/masters/master.client.v8.fyi/master.cfg [modify] https://crrev.com/44ac69e01170e5be129cef2b255c98114b5460e3/masters/master.client.v8.fyi/slaves.cfg
,
Jun 27 2017
> As long as this is blocked, Ulan, how about a normal chromium linux non-tsan gpu tester with concurrent marking? Sounds good to me. Thanks a lot for working on this!
,
Jun 28 2017
FYI: The first run with CM and a standard GPU bot is green: https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20-%20concurrent%20marking%20%28NVIDIA%29/builds/44 so... unfortunately no new bugs found. Lowering prio here until the TSAN blockers are resolved or until we find other means of testing.
,
Jun 29 2017
Though we don't see any dcheck failures or crashes on the CM GPU bot, it has a bunch of flaky failures compared to its non-GM twin, e.g. compare: https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20-%20concurrent%20marking%20(NVIDIA) https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20(NVIDIA) Maybe the test failures stem from timing differences? In any way, I assume they need to get fixed before the feature can go off the ground.
,
Jun 29 2017
I just landed correctness fix for concurrent marker: https://chromium.googlesource.com/v8/v8/+/8b97f512ac4eb4fa41102a3de8f303395e306173 That might fix some failures. > Maybe the test failures stem from timing differences? Concurrent marking flag implies stressing of incremental marking (i.e. it does a lot more GCs than vanilla TOT). Maybe we should run GPU and perf bots with --nostress-incremental-marking runtime flag? > In any way, I assume they need to get fixed before the feature can go off the ground. Yep, all bots must be green before we can enable the feature.
,
Jun 30 2017
Re 25: Both the perf bot and the Chromium bot are controlled by a compile-time flag. @Ulan: Could you introduce a DEFINE for --nostress-incremental-marking, changing the default value of the flag? Then you could add a new v8 gn arg, side by side with v8_enable_concurrent_marking. Set it here for the v8 stand-alone builder: https://cs.chromium.org/chromium/src/v8/infra/mb/mb_config.pyl?q=mb_config+package:%5Echromium$&l=383 And here for the chromium bot: https://cs.chromium.org/chromium/src/tools/mb/mb_config.pyl?q=mb_config.pyl&dr&l=1297 The v8 tsan bot would not be affected. It uses its own config.
,
Jun 30 2017
Re 26: Different plan: We will add the --stress-incremental-marking flag explicitly to the v8 tsan bots. Then ulan will switch the default to false, which'll impact the perf and the chromium bot.
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/7964fe6872be0963275f6db104a9300998b3e4dc commit 7964fe6872be0963275f6db104a9300998b3e4dc Author: Michael Achenbach <machenbach@chromium.org> Date: Fri Jun 30 17:27:48 2017 V8: Explicitly pass stress-incremental-marking flag on tsan bots Bug: 723600 Change-Id: I526a6ba9750f42c842475afa2edd3a02a512e46f Reviewed-on: https://chromium-review.googlesource.com/558079 Reviewed-by: Henrik Kjellander <kjellander@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> [modify] https://crrev.com/7964fe6872be0963275f6db104a9300998b3e4dc/scripts/slave/recipe_modules/v8/builders.py [modify] https://crrev.com/7964fe6872be0963275f6db104a9300998b3e4dc/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_tsan_concurrent_marking_rel_ng_triggered.json [modify] https://crrev.com/7964fe6872be0963275f6db104a9300998b3e4dc/scripts/slave/recipe_modules/v8/config.py [modify] https://crrev.com/7964fe6872be0963275f6db104a9300998b3e4dc/scripts/slave/recipes/v8.expected/full_client_v8_V8_Linux64_TSAN___concurrent_marking.json
,
Jul 3 2017
Back to ulan for making stress-incremental-marking false by default.
,
Jul 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/3aa9b26a8e26f834ca74d5e6eefb1f4d3d020ca8 commit 3aa9b26a8e26f834ca74d5e6eefb1f4d3d020ca8 Author: Ulan Degenbaev <ulan@chromium.org> Date: Fri Jul 07 11:16:01 2017 [heap] Decouple stress-incremental-marking flag from concurrent-marking. BUG= chromium:723600 Change-Id: Ic933159c973ccfe2a67fe8a4e2639c2ebd21b8d4 Reviewed-on: https://chromium-review.googlesource.com/563396 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#46471} [modify] https://crrev.com/3aa9b26a8e26f834ca74d5e6eefb1f4d3d020ca8/src/flag-definitions.h
,
Jul 10 2017
Note there's also a dcheck failure on the GPU bot with CM now: https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20-%20concurrent%20marking%20%28NVIDIA%29/builds/173 # # Fatal error in ../../v8/src/heap/heap.cc, line 4270 # Check failed: ObjectMarking::IsBlackOrGrey<IncrementalMarking::kAtomicity>( obj, MarkingState::Internal(obj)). #
,
Jul 10 2017
Thanks, investigating.
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/250ba28a57b1a1f50ed00248122258b1ba066ab0 commit 250ba28a57b1a1f50ed00248122258b1ba066ab0 Author: Ulan Degenbaev <ulan@chromium.org> Date: Fri Jul 21 10:36:57 2017 [heap, deserializer] Restore marking invariant for deserialized maps when black allocation is on. The scenario: 1) Incremental marking is off. 2) Partial deserialization starts and calls Heap::ReserveSpace. 2) ReserveSpace creates (white) reservations in old space. 3) ReserveSpace allocates map placeholders. One of these allocations starts incremental marking, which starts black allocation (currently when concurrent marking is on). Subsequent maps are black allocated. 4) ReserveSpace succeeds without triggering a GC. 5) Deserialization continues. Some maps are black. Note that deserialization emits only old->new write barriers and skips marking write barriers. 6) Deserialization finishes and re-visits the black allocated reservations and large object. This misses black allocated maps. 7) There is black->white descriptor array pointer in one of these map. BUG= chromium:723600 Change-Id: Ifffe46f22a7d7dbc5cff2e882190234fcc722ccb Reviewed-on: https://chromium-review.googlesource.com/581187 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#46816} [modify] https://crrev.com/250ba28a57b1a1f50ed00248122258b1ba066ab0/src/heap/heap.cc [modify] https://crrev.com/250ba28a57b1a1f50ed00248122258b1ba066ab0/src/heap/heap.h [modify] https://crrev.com/250ba28a57b1a1f50ed00248122258b1ba066ab0/src/snapshot/deserializer.cc
,
Jul 24 2017
The bot has been green for many runs now |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by u...@chromium.org
, May 17 2017