Concurrent marking doesn't take advantage of all available worker threads |
||||
Issue descriptionIt uses num_cores/2. This was done previously because using num_availabe_workers regressed performance on Mac. We speculate that this may have been because the scheduler previously provided num_available_workers == num_cores which could result in stealing cycles from the main thread. We will try again now that num_available_workers == num_cores - 1. Using num_cores - 1 concurrent marking tasks is expected to help have significant positive impact as a recent regression that made it use (num_cores-1)/2 (ref. issue 808028 ) had a significant negative impact (10% regression in many core metrics, ref. issue 809961 ). On 4 cores machine (the typical configuration in the wild). This translates to : current == 2 cores used, regression == 1 core used, proposal == 3 cores used. Edit: another identified cause is that using all workers prevents parallel steps on the main thread from getting any workers. ConcurrentMarking::PauseScope() is used to prevent workers from touching the heap while other main thread GC operations are happening but it currently only blocks the workers without preempting the tasks. The workers not going back to the pool means that GC operations happening during a ConcurrentMarking::PauseScope() only get half the cores. I will fix PauseScope to force tasks to yield and schedule a continuation task from ~PauseScope(), giving the worker back to the pool. This will allow ConcurrentMarking to use num_cores-1 (1.5X to 2X more parallelism) as well as let competing main thread GC phases reclaim all cores for their phase (2x more parallelism) :)!
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/330fa940c2cca336cac755a9d284fb68a2b374d0 commit 330fa940c2cca336cac755a9d284fb68a2b374d0 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 08:09:50 2018 Extract the trivial cleanup bits from https://chromium-review.googlesource.com/c/v8/v8/+/924073/10 This is an attempt to isolate what's causing the hard-to-diagnose bots only failures with that CL. Bug: chromium:812178 Change-Id: I50ffe8953bebbbc6b5a5e2f689718662a537acb4 Reviewed-on: https://chromium-review.googlesource.com/924864 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51348} [modify] https://crrev.com/330fa940c2cca336cac755a9d284fb68a2b374d0/src/heap/concurrent-marking.cc [modify] https://crrev.com/330fa940c2cca336cac755a9d284fb68a2b374d0/src/heap/concurrent-marking.h
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ba37a5f64aa03aaff32f69cd4e554b6d0e86c44d commit ba37a5f64aa03aaff32f69cd4e554b6d0e86c44d Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 08:13:45 2018 Add DCHECKs to confirm there are no pending tasks when calling ConcurrentMarking::ScheduleTasks() This was extracted from https://chromium-review.googlesource.com/c/v8/v8/+/924073/7 in an attempt to isolate hard-to-diagnose bots-only failures there. Bug: chromium:812178 Change-Id: I980b25ec7d775b74ade75e9166806740b93eea8e Reviewed-on: https://chromium-review.googlesource.com/924026 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51349} [modify] https://crrev.com/ba37a5f64aa03aaff32f69cd4e554b6d0e86c44d/src/heap/concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f4b4109936400c57b79ad015fdcb1fcac979999f commit f4b4109936400c57b79ad015fdcb1fcac979999f Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 10:39:08 2018 Introduce ConcurrentMarking::StopRequest API. This was extracted from https://chromium-review.googlesource.com/c/v8/v8/+/924073/10 after it became clear that using COMPLETE_TASKS/PREEMPT_TASKS where it should make sense to doesn't work in practice for now. Experimental CLs which led to the above conclusion: - https://chromium-review.googlesource.com/c/v8/v8/+/924865 (COMPLETE or CANCEL -- still broken) - https://chromium-review.googlesource.com/c/v8/v8/+/924866 (CANCEL only, as before, works) - https://chromium-review.googlesource.com/c/v8/v8/+/924028 (CANCEL and PREEMPT -- broken as well) Introducing this unittested API allows to reduce the size of the CLs causing hard-to-diagnose bots-only failures and fix them individually follow-ups @ 1) https://chromium-review.googlesource.com/c/v8/v8/+/924029 2) https://chromium-review.googlesource.com/c/v8/v8/+/924031 3) https://chromium-review.googlesource.com/c/v8/v8/+/924030 Bug: chromium:812178 Change-Id: Icdac456e9f7874b0c4b321ccdb8898297dad7d73 Reviewed-on: https://chromium-review.googlesource.com/924867 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51353} [modify] https://crrev.com/f4b4109936400c57b79ad015fdcb1fcac979999f/src/heap/concurrent-marking.cc [modify] https://crrev.com/f4b4109936400c57b79ad015fdcb1fcac979999f/src/heap/concurrent-marking.h [modify] https://crrev.com/f4b4109936400c57b79ad015fdcb1fcac979999f/src/heap/mark-compact.cc [modify] https://crrev.com/f4b4109936400c57b79ad015fdcb1fcac979999f/src/heap/mark-compact.h [modify] https://crrev.com/f4b4109936400c57b79ad015fdcb1fcac979999f/test/cctest/heap/test-concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/4b49f84434c3098699a51e51c20ec623cc9de404 commit 4b49f84434c3098699a51e51c20ec623cc9de404 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 10:48:19 2018 Add a trace event when pausing/preempting concurrent marking. This will let us identify in traces whether unwinding after receiving the preemption event is slower than desired and should be optimized. Adding it to pausing while working on removing it in https://chromium-review.googlesource.com/c/v8/v8/+/922103 will allow gathering traces that highlight the issue. R=mlippautz@chromium.org Bug: chromium:812178 Change-Id: I0555c6825e0792769c9ae2d748d7cc35df4f6fed Reviewed-on: https://chromium-review.googlesource.com/924122 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#51354} [modify] https://crrev.com/4b49f84434c3098699a51e51c20ec623cc9de404/src/heap/concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/e9750cb80653827a0c685fe65b9572315fe1d4e4 commit e9750cb80653827a0c685fe65b9572315fe1d4e4 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 10:54:34 2018 Preempt ConcurrentMarking tasks instead of merely pausing in PauseScope. Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 This is the core goal of the initial CL @ https://chromium-review.googlesource.com/c/v8/v8/+/922103 which was since split into multiple to diagnose a bots-only failure. Bug: chromium:812178 Change-Id: I4c4e0b517737e020862917bd89fa6ce38244e597 Reviewed-on: https://chromium-review.googlesource.com/924031 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#51356} [modify] https://crrev.com/e9750cb80653827a0c685fe65b9572315fe1d4e4/src/heap/concurrent-marking.cc [modify] https://crrev.com/e9750cb80653827a0c685fe65b9572315fe1d4e4/src/heap/concurrent-marking.h
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/8b53b9d9086feb00da168ca9d82055a379447101 commit 8b53b9d9086feb00da168ca9d82055a379447101 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 10:59:42 2018 Preempt ConcurrentMarking tasks ASAP when cancelling marking. Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 Bug: chromium:812178 Change-Id: I2abe28c6e953df42cffdcbd7ea35df9d29849905 Reviewed-on: https://chromium-review.googlesource.com/924030 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51357} [modify] https://crrev.com/8b53b9d9086feb00da168ca9d82055a379447101/src/heap/mark-compact.cc
,
Feb 19 2018
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/773c70b65ca065449662b7c11d3da02797e5519b commit 773c70b65ca065449662b7c11d3da02797e5519b Author: Clemens Hammacher <clemensh@chromium.org> Date: Mon Feb 19 13:56:07 2018 Revert "Preempt ConcurrentMarking tasks ASAP when cancelling marking." This reverts commit 8b53b9d9086feb00da168ca9d82055a379447101. Reason for revert: Several GC failures, e.g. https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/23236, https://build.chromium.org/p/client.v8/builders/V8%20Mac/builds/18390 Original change's description: > Preempt ConcurrentMarking tasks ASAP when cancelling marking. > > Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 > > Bug: chromium:812178 > Change-Id: I2abe28c6e953df42cffdcbd7ea35df9d29849905 > Reviewed-on: https://chromium-review.googlesource.com/924030 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51357} TBR=gab@chromium.org,ulan@chromium.org,mlippautz@chromium.org Change-Id: Ic4e226fdd02d8259244cef46e9923c95e6606cc4 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:812178 Reviewed-on: https://chromium-review.googlesource.com/924425 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#51362} [modify] https://crrev.com/773c70b65ca065449662b7c11d3da02797e5519b/src/heap/mark-compact.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/027b012d67304fb5c835ac4662624ed581cb45bf commit 027b012d67304fb5c835ac4662624ed581cb45bf Author: Clemens Hammacher <clemensh@chromium.org> Date: Mon Feb 19 13:57:12 2018 Revert "Preempt ConcurrentMarking tasks instead of merely pausing in PauseScope." This reverts commit e9750cb80653827a0c685fe65b9572315fe1d4e4. Reason for revert: Several GC failures, e.g. https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/23236, https://build.chromium.org/p/client.v8/builders/V8%20Mac/builds/18390 Original change's description: > Preempt ConcurrentMarking tasks instead of merely pausing in PauseScope. > > Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 > > This is the core goal of the initial CL @ > https://chromium-review.googlesource.com/c/v8/v8/+/922103 > which was since split into multiple to diagnose a bots-only failure. > > Bug: chromium:812178 > Change-Id: I4c4e0b517737e020862917bd89fa6ce38244e597 > Reviewed-on: https://chromium-review.googlesource.com/924031 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51356} TBR=gab@chromium.org,ulan@chromium.org,mlippautz@chromium.org Change-Id: Ic095e32708e58acbe5955bf29e65af34c59d321e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:812178 Reviewed-on: https://chromium-review.googlesource.com/925301 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#51363} [modify] https://crrev.com/027b012d67304fb5c835ac4662624ed581cb45bf/src/heap/concurrent-marking.cc [modify] https://crrev.com/027b012d67304fb5c835ac4662624ed581cb45bf/src/heap/concurrent-marking.h
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/1986ee4867652aa8a47f3220d924570e66fd48bc commit 1986ee4867652aa8a47f3220d924570e66fd48bc Author: Clemens Hammacher <clemensh@chromium.org> Date: Mon Feb 19 13:58:16 2018 Revert "Add a trace event when pausing/preempting concurrent marking." This reverts commit 4b49f84434c3098699a51e51c20ec623cc9de404. Reason for revert: Several GC failures, e.g. https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/23236, https://build.chromium.org/p/client.v8/builders/V8%20Mac/builds/18390 Original change's description: > Add a trace event when pausing/preempting concurrent marking. > > This will let us identify in traces whether unwinding after receiving > the preemption event is slower than desired and should be optimized. > > Adding it to pausing while working on removing it in > https://chromium-review.googlesource.com/c/v8/v8/+/922103 > will allow gathering traces that highlight the issue. > > R=mlippautz@chromium.org > > Bug: chromium:812178 > Change-Id: I0555c6825e0792769c9ae2d748d7cc35df4f6fed > Reviewed-on: https://chromium-review.googlesource.com/924122 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51354} TBR=gab@chromium.org,mlippautz@chromium.org Change-Id: I37a82e488de51d5ae4d7ed795b82ea9649c4a5f9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:812178 Reviewed-on: https://chromium-review.googlesource.com/924426 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#51364} [modify] https://crrev.com/1986ee4867652aa8a47f3220d924570e66fd48bc/src/heap/concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/b8a727e14c668399a4c3befc9e2a483507ef0f89 commit b8a727e14c668399a4c3befc9e2a483507ef0f89 Author: Clemens Hammacher <clemensh@chromium.org> Date: Mon Feb 19 13:59:18 2018 Revert "Introduce ConcurrentMarking::StopRequest API." This reverts commit f4b4109936400c57b79ad015fdcb1fcac979999f. Reason for revert: Several GC failures, e.g. https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/23236, https://build.chromium.org/p/client.v8/builders/V8%20Mac/builds/18390 Original change's description: > Introduce ConcurrentMarking::StopRequest API. > > This was extracted from https://chromium-review.googlesource.com/c/v8/v8/+/924073/10 > after it became clear that using COMPLETE_TASKS/PREEMPT_TASKS where > it should make sense to doesn't work in practice for now. > > Experimental CLs which led to the above conclusion: > - https://chromium-review.googlesource.com/c/v8/v8/+/924865 > (COMPLETE or CANCEL -- still broken) > - https://chromium-review.googlesource.com/c/v8/v8/+/924866 > (CANCEL only, as before, works) > - https://chromium-review.googlesource.com/c/v8/v8/+/924028 > (CANCEL and PREEMPT -- broken as well) > > Introducing this unittested API allows to reduce the size > of the CLs causing hard-to-diagnose bots-only failures > and fix them individually follow-ups @ > > 1) https://chromium-review.googlesource.com/c/v8/v8/+/924029 > 2) https://chromium-review.googlesource.com/c/v8/v8/+/924031 > 3) https://chromium-review.googlesource.com/c/v8/v8/+/924030 > > Bug: chromium:812178 > Change-Id: Icdac456e9f7874b0c4b321ccdb8898297dad7d73 > Reviewed-on: https://chromium-review.googlesource.com/924867 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51353} TBR=gab@chromium.org,ulan@chromium.org,mlippautz@chromium.org Change-Id: Ia001cc81c6a7bc030b54d3aa9b9bcecc833300e6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:812178 Reviewed-on: https://chromium-review.googlesource.com/925302 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#51365} [modify] https://crrev.com/b8a727e14c668399a4c3befc9e2a483507ef0f89/src/heap/concurrent-marking.cc [modify] https://crrev.com/b8a727e14c668399a4c3befc9e2a483507ef0f89/src/heap/concurrent-marking.h [modify] https://crrev.com/b8a727e14c668399a4c3befc9e2a483507ef0f89/src/heap/mark-compact.cc [modify] https://crrev.com/b8a727e14c668399a4c3befc9e2a483507ef0f89/src/heap/mark-compact.h [modify] https://crrev.com/b8a727e14c668399a4c3befc9e2a483507ef0f89/test/cctest/heap/test-concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642 commit ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 14:46:44 2018 Reland "Introduce ConcurrentMarking::StopRequest API." This is a reland of f4b4109936400c57b79ad015fdcb1fcac979999f. Not expected to be the culprit of the 4 CL revert. Original change's description: > Introduce ConcurrentMarking::StopRequest API. > > This was extracted from https://chromium-review.googlesource.com/c/v8/v8/+/924073/10 > after it became clear that using COMPLETE_TASKS/PREEMPT_TASKS where > it should make sense to doesn't work in practice for now. > > Experimental CLs which led to the above conclusion: > - https://chromium-review.googlesource.com/c/v8/v8/+/924865 > (COMPLETE or CANCEL -- still broken) > - https://chromium-review.googlesource.com/c/v8/v8/+/924866 > (CANCEL only, as before, works) > - https://chromium-review.googlesource.com/c/v8/v8/+/924028 > (CANCEL and PREEMPT -- broken as well) > > Introducing this unittested API allows to reduce the size > of the CLs causing hard-to-diagnose bots-only failures > and fix them individually follow-ups @ > > 1) https://chromium-review.googlesource.com/c/v8/v8/+/924029 > 2) https://chromium-review.googlesource.com/c/v8/v8/+/924031 > 3) https://chromium-review.googlesource.com/c/v8/v8/+/924030 > > Bug: chromium:812178 > Change-Id: Icdac456e9f7874b0c4b321ccdb8898297dad7d73 > Reviewed-on: https://chromium-review.googlesource.com/924867 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51353} Bug: chromium:812178 Change-Id: Iaa32f9cc6b2fa7004c7fae1f79aa4b00f5f8f34c Reviewed-on: https://chromium-review.googlesource.com/924006 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51371} [modify] https://crrev.com/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642/src/heap/concurrent-marking.cc [modify] https://crrev.com/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642/src/heap/concurrent-marking.h [modify] https://crrev.com/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642/src/heap/mark-compact.cc [modify] https://crrev.com/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642/src/heap/mark-compact.h [modify] https://crrev.com/ac17ba0e56b8d7a48068ad8a01c41e7c0cb79642/test/cctest/heap/test-concurrent-marking.cc
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/431c473b74c2dbd63cce24d13e92aee50a48783b commit 431c473b74c2dbd63cce24d13e92aee50a48783b Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 19 15:04:51 2018 Reland: Add a trace event when pausing/preempting concurrent marking. Reland reason : not the culprit. This will let us identify in traces whether unwinding after receiving the preemption event is slower than desired and should be optimized. Adding it to pausing while working on removing it in https://chromium-review.googlesource.com/c/v8/v8/+/922103 will allow gathering traces that highlight the issue. R=ulan@chromium.org Bug: chromium:812178 Change-Id: I0dc0f6754980157674968ba4a868f12c779e69bc Reviewed-on: https://chromium-review.googlesource.com/923989 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51372} [modify] https://crrev.com/431c473b74c2dbd63cce24d13e92aee50a48783b/src/heap/concurrent-marking.cc
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/78ff04c54f8dbfb9acea9dc0c4f1079edf1d6a48 commit 78ff04c54f8dbfb9acea9dc0c4f1079edf1d6a48 Author: Gabriel Charette <gab@chromium.org> Date: Wed Feb 21 11:39:30 2018 Reland: Preempt ConcurrentMarking tasks instead of merely pausing in PauseScope. Reland reason : fixed errors, see PS1->PS4 diff. Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 This is the core goal of the initial CL @ https://chromium-review.googlesource.com/c/v8/v8/+/922103 which was since split into multiple to diagnose a bots-only failure. R=ulan@chromium.org (cherry picked from commit e9750cb80653827a0c685fe65b9572315fe1d4e4) Bug: chromium:812178 Change-Id: Ib9474b5c90bf11f4741a93ac35c99b4979e8b4f9 Reviewed-on: https://chromium-review.googlesource.com/925267 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51423} [modify] https://crrev.com/78ff04c54f8dbfb9acea9dc0c4f1079edf1d6a48/src/heap/concurrent-marking.cc [modify] https://crrev.com/78ff04c54f8dbfb9acea9dc0c4f1079edf1d6a48/src/heap/concurrent-marking.h
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/7badc3f0c4917dff90ff4621a0d4092ba5aa4089 commit 7badc3f0c4917dff90ff4621a0d4092ba5aa4089 Author: Gabriel Charette <gab@chromium.org> Date: Wed Feb 21 13:09:39 2018 Reland : Preempt ConcurrentMarking tasks ASAP when cancelling marking. Reland reason : the failure was Check failed: IsGlobalEmpty(). v8::base::debug::StackTrace::StackTrace() v8::platform::(anonymous namespace)::PrintStackTrace() V8_Fatal(char const*, int, char const*, ...) v8::internal::Worklist<v8::internal::HeapObject*, 64>::~Worklist() v8::internal::MarkCompactCollector::~MarkCompactCollector() v8::internal::MarkCompactCollector::~MarkCompactCollector() v8::internal::Heap::TearDown() v8::internal::Isolate::Deinit() v8::internal::Isolate::TearDown() v8::Shell::OnExit(v8::Isolate*) v8::Shell::Main(int, char**) this is believed to be fixed by the change to flush the shared worklist to global in https://chromium-review.googlesource.com/c/v8/v8/+/925267 Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/924867 (cherry picked from commit 8b53b9d9086feb00da168ca9d82055a379447101) Bug: chromium:812178 Change-Id: I796204656e2c89e7efecda2c275a1888c31aba7b Reviewed-on: https://chromium-review.googlesource.com/925268 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51430} [modify] https://crrev.com/7badc3f0c4917dff90ff4621a0d4092ba5aa4089/src/heap/mark-compact.cc
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/3c62f7ae0744ebf95769429e5b2b3c83fc384fdf commit 3c62f7ae0744ebf95769429e5b2b3c83fc384fdf Author: Gabriel Charette <gab@chromium.org> Date: Thu Feb 22 09:39:10 2018 Use all available workers for concurrent marking. R=ulan@chromium.org Bug: chromium:812178 Change-Id: I35a727cb6c663bbd5f1beab98324e5d1b1ecf5c7 Reviewed-on: https://chromium-review.googlesource.com/918663 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#51458} [modify] https://crrev.com/3c62f7ae0744ebf95769429e5b2b3c83fc384fdf/src/heap/concurrent-marking.cc [modify] https://crrev.com/3c62f7ae0744ebf95769429e5b2b3c83fc384fdf/src/heap/concurrent-marking.h
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/4939463c778a146bdcc77454dbe530f1918aaa73 commit 4939463c778a146bdcc77454dbe530f1918aaa73 Author: Gabriel Charette <gab@chromium.org> Date: Thu Feb 22 13:17:07 2018 Cap ConcurrentMarking tasks at 7 for now. This is an unfortunate restriction imposed by Worklist::kMaxNumTasks for now. This CL unbreaks tests for developers. The CQ didn't catch this breakage because bots have 8 cores and concurrent marking uses num_cores-1. R=hpayer@chromium.org TEST=All tests passed on dev machine (was super broken without this change) NOTRY=True (to unbreak devs) Bug: v8:7477 , chromium:812178 Change-Id: I644613857c74d1ae00965f3e6d1d7692a4303062 Reviewed-on: https://chromium-review.googlesource.com/931461 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Hannes Payer <hpayer@chromium.org> Cr-Commit-Position: refs/heads/master@{#51473} [modify] https://crrev.com/4939463c778a146bdcc77454dbe530f1918aaa73/src/heap/concurrent-marking.h
,
Feb 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/c41c7a0943e02ac1161db7dfd500d277f294845d commit c41c7a0943e02ac1161db7dfd500d277f294845d Author: Gabriel Charette <gab@chromium.org> Date: Sun Feb 25 21:40:49 2018 Revert "Use all available workers for concurrent marking." This reverts commit 3c62f7ae0744ebf95769429e5b2b3c83fc384fdf. (and commit 4939463c778a146bdcc77454dbe530f1918aaa73) The goal of this revert is to contrast the effect on perf bots of landing it vs reverting it to more easily attribute its impact. R=hpayer@chromium.org Bug: chromium:812178 Change-Id: I7c977b1b0b587f787263272400d87f6aae7af634 Reviewed-on: https://chromium-review.googlesource.com/936761 Commit-Queue: Hannes Payer <hpayer@chromium.org> Reviewed-by: Hannes Payer <hpayer@chromium.org> Cr-Commit-Position: refs/heads/master@{#51546} [modify] https://crrev.com/c41c7a0943e02ac1161db7dfd500d277f294845d/src/heap/concurrent-marking.cc [modify] https://crrev.com/c41c7a0943e02ac1161db7dfd500d277f294845d/src/heap/concurrent-marking.h
,
Feb 26 2018
Relevant chromium commits FTR: - v8:51458 == r538530 (initial in r538439 but reverted in r538443) : https://chromeperf.appspot.com/group_report?rev=538530 - v8:51546 == r539055 (reverts improvement to see what it did help...)
,
Feb 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/2ba05d678150bf176daa060fab111b3337178ac4 commit 2ba05d678150bf176daa060fab111b3337178ac4 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 26 17:22:39 2018 Revert "Revert "Use all available workers for concurrent marking."" This reverts commit c41c7a0943e02ac1161db7dfd500d277f294845d. Reason for revert: relanding now that the perf waterfall has had a stab at this revert. Original change's description: > Revert "Use all available workers for concurrent marking." > > This reverts commit 3c62f7ae0744ebf95769429e5b2b3c83fc384fdf. > (and commit 4939463c778a146bdcc77454dbe530f1918aaa73) > > The goal of this revert is to contrast the effect on perf bots of > landing it vs reverting it to more easily attribute its impact. > > R=hpayer@chromium.org > > Bug: chromium:812178 > Change-Id: I7c977b1b0b587f787263272400d87f6aae7af634 > Reviewed-on: https://chromium-review.googlesource.com/936761 > Commit-Queue: Hannes Payer <hpayer@chromium.org> > Reviewed-by: Hannes Payer <hpayer@chromium.org> > Cr-Commit-Position: refs/heads/master@{#51546} TBR=gab@chromium.org,hpayer@chromium.org Change-Id: I1ecfc70867dc5424cba1a9ecd229ae031c3e9aa4 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:812178 Reviewed-on: https://chromium-review.googlesource.com/937725 Reviewed-by: Hannes Payer <hpayer@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51577} [modify] https://crrev.com/2ba05d678150bf176daa060fab111b3337178ac4/src/heap/concurrent-marking.cc [modify] https://crrev.com/2ba05d678150bf176daa060fab111b3337178ac4/src/heap/concurrent-marking.h
,
Feb 27 2018
- v8:51546 == r539378 (reland)
,
Feb 28 2018
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/226da60f4a6f3b46877282a43d3ef71044bc07aa commit 226da60f4a6f3b46877282a43d3ef71044bc07aa Author: Gabriel Charette <gab@chromium.org> Date: Fri Mar 02 15:36:54 2018 [v8] Do not do rely on hyper-threads for concurrent marking on Mac. This should recover https://chromeperf.appspot.com/report?sid=4d751475ba95911f865aed7a822d55dde18304bc0cfd2f8409d1de9fe9695343 and https://arewefastyet.com/#machine=28&view=single&suite=octane&subtest=Splay It will however regress this: https://chromeperf.appspot.com/report?sid=020744195cfb20c373344b86b76385ce2919b53796b5c0651ba71c0625e8de19&start_rev=531511&end_rev=540262 R=ulan@chromium.org Bug: chromium:812178 , chromium:816541 Change-Id: Ia367d24b013c3f16d1dc2ae56d4c5ef23342845f Reviewed-on: https://chromium-review.googlesource.com/946099 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51703} [modify] https://crrev.com/226da60f4a6f3b46877282a43d3ef71044bc07aa/src/heap/concurrent-marking.cc
,
May 4 2018
I think we can call this fixed. Feel free to open another bug for Mac if you want to keep investigating. The next possible improvement would be the jobs API IMO (tracked in long-term issue 839091) but that goes beyond this bug. |
||||
►
Sign in to add a comment |
||||
Comment 1 by gab@chromium.org
, Feb 15 2018