Commit f46456a3 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

[heap] Add timeout to Scavenger barrier

Speculatively mitigation for renderer hangs in Scavenger
while waiting in a barrier.

Bug: 
Change-Id: I48520e0ffd99123dbe352d2012c911186c187e4b
Reviewed-on: https://chromium-review.googlesource.com/c/1296463
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarHannes Payer <hpayer@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57130}
parent 9f75c148
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "src/base/platform/condition-variable.h" #include "src/base/platform/condition-variable.h"
#include "src/base/platform/mutex.h" #include "src/base/platform/mutex.h"
#include "src/base/platform/time.h"
namespace v8 { namespace v8 {
namespace internal { namespace internal {
...@@ -14,6 +15,10 @@ namespace internal { ...@@ -14,6 +15,10 @@ namespace internal {
// Barrier that can be used once to synchronize a dynamic number of tasks // Barrier that can be used once to synchronize a dynamic number of tasks
// working concurrently. // working concurrently.
// //
// The barrier takes a timeout which is used to avoid waiting for too long. If
// any of the users ever reach the timeout they will disable the barrier and
// signal others to fall through.
//
// Usage: // Usage:
// void RunConcurrently(OneShotBarrier* shared_barrier) { // void RunConcurrently(OneShotBarrier* shared_barrier) {
// shared_barrier->Start(); // shared_barrier->Start();
...@@ -31,7 +36,7 @@ namespace internal { ...@@ -31,7 +36,7 @@ namespace internal {
// immediately. // immediately.
class OneshotBarrier { class OneshotBarrier {
public: public:
OneshotBarrier() : tasks_(0), waiting_(0), done_(false) {} explicit OneshotBarrier(base::TimeDelta timeout) : timeout_(timeout) {}
void Start() { void Start() {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&mutex_);
...@@ -54,7 +59,11 @@ class OneshotBarrier { ...@@ -54,7 +59,11 @@ class OneshotBarrier {
condition_.NotifyAll(); condition_.NotifyAll();
} else { } else {
// Spurious wakeup is ok here. // Spurious wakeup is ok here.
condition_.Wait(&mutex_); if (!condition_.WaitFor(&mutex_, timeout_)) {
// If predefined timeout was reached, Stop waiting and signal being done
// also to other tasks.
done_ = true;
}
} }
waiting_--; waiting_--;
return done_; return done_;
...@@ -66,9 +75,10 @@ class OneshotBarrier { ...@@ -66,9 +75,10 @@ class OneshotBarrier {
private: private:
base::ConditionVariable condition_; base::ConditionVariable condition_;
base::Mutex mutex_; base::Mutex mutex_;
int tasks_; base::TimeDelta timeout_;
int waiting_; int tasks_ = 0;
bool done_; int waiting_ = 0;
bool done_ = false;
}; };
} // namespace internal } // namespace internal
......
...@@ -160,7 +160,7 @@ void ScavengerCollector::CollectGarbage() { ...@@ -160,7 +160,7 @@ void ScavengerCollector::CollectGarbage() {
Scavenger* scavengers[kMaxScavengerTasks]; Scavenger* scavengers[kMaxScavengerTasks];
const bool is_logging = isolate_->LogObjectRelocation(); const bool is_logging = isolate_->LogObjectRelocation();
const int num_scavenge_tasks = NumberOfScavengeTasks(); const int num_scavenge_tasks = NumberOfScavengeTasks();
OneshotBarrier barrier; OneshotBarrier barrier(base::TimeDelta::FromMilliseconds(kMaxWaitTimeMs));
Scavenger::CopiedList copied_list(num_scavenge_tasks); Scavenger::CopiedList copied_list(num_scavenge_tasks);
Scavenger::PromotionList promotion_list(num_scavenge_tasks); Scavenger::PromotionList promotion_list(num_scavenge_tasks);
for (int i = 0; i < num_scavenge_tasks; i++) { for (int i = 0; i < num_scavenge_tasks; i++) {
......
...@@ -29,6 +29,7 @@ using SurvivingNewLargeObjectMapEntry = std::pair<HeapObject*, Map*>; ...@@ -29,6 +29,7 @@ using SurvivingNewLargeObjectMapEntry = std::pair<HeapObject*, Map*>;
class ScavengerCollector { class ScavengerCollector {
public: public:
static const int kMaxScavengerTasks = 8; static const int kMaxScavengerTasks = 8;
static const int kMaxWaitTimeMs = 2;
explicit ScavengerCollector(Heap* heap); explicit ScavengerCollector(Heap* heap);
......
...@@ -4,19 +4,27 @@ ...@@ -4,19 +4,27 @@
#include "src/heap/barrier.h" #include "src/heap/barrier.h"
#include "src/base/platform/platform.h" #include "src/base/platform/platform.h"
#include "src/base/platform/time.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace v8 { namespace v8 {
namespace internal { namespace internal {
namespace heap { namespace heap {
namespace {
// Large timeout that will not trigger in tests.
constexpr base::TimeDelta test_timeout = base::TimeDelta::FromHours(3);
} // namespace
TEST(OneshotBarrier, InitializeNotDone) { TEST(OneshotBarrier, InitializeNotDone) {
OneshotBarrier barrier; OneshotBarrier barrier(test_timeout);
EXPECT_FALSE(barrier.DoneForTesting()); EXPECT_FALSE(barrier.DoneForTesting());
} }
TEST(OneshotBarrier, DoneAfterWait_Sequential) { TEST(OneshotBarrier, DoneAfterWait_Sequential) {
OneshotBarrier barrier; OneshotBarrier barrier(test_timeout);
barrier.Start(); barrier.Start();
barrier.Wait(); barrier.Wait();
EXPECT_TRUE(barrier.DoneForTesting()); EXPECT_TRUE(barrier.DoneForTesting());
...@@ -41,7 +49,7 @@ class ThreadWaitingOnBarrier final : public base::Thread { ...@@ -41,7 +49,7 @@ class ThreadWaitingOnBarrier final : public base::Thread {
TEST(OneshotBarrier, DoneAfterWait_Concurrent) { TEST(OneshotBarrier, DoneAfterWait_Concurrent) {
const int kThreadCount = 2; const int kThreadCount = 2;
OneshotBarrier barrier; OneshotBarrier barrier(test_timeout);
ThreadWaitingOnBarrier threads[kThreadCount]; ThreadWaitingOnBarrier threads[kThreadCount];
for (int i = 0; i < kThreadCount; i++) { for (int i = 0; i < kThreadCount; i++) {
threads[i].Initialize(&barrier); threads[i].Initialize(&barrier);
...@@ -59,7 +67,7 @@ TEST(OneshotBarrier, DoneAfterWait_Concurrent) { ...@@ -59,7 +67,7 @@ TEST(OneshotBarrier, DoneAfterWait_Concurrent) {
TEST(OneshotBarrier, EarlyFinish_Concurrent) { TEST(OneshotBarrier, EarlyFinish_Concurrent) {
const int kThreadCount = 2; const int kThreadCount = 2;
OneshotBarrier barrier; OneshotBarrier barrier(test_timeout);
ThreadWaitingOnBarrier threads[kThreadCount]; ThreadWaitingOnBarrier threads[kThreadCount];
// Test that one thread that actually finishes processing work before other // Test that one thread that actually finishes processing work before other
// threads call Start() will move the barrier in Done state. // threads call Start() will move the barrier in Done state.
...@@ -118,7 +126,7 @@ class CountingThread final : public base::Thread { ...@@ -118,7 +126,7 @@ class CountingThread final : public base::Thread {
TEST(OneshotBarrier, Processing_Concurrent) { TEST(OneshotBarrier, Processing_Concurrent) {
const size_t kWorkCounter = 173173; const size_t kWorkCounter = 173173;
OneshotBarrier barrier; OneshotBarrier barrier(test_timeout);
base::Mutex mutex; base::Mutex mutex;
size_t work = 0; size_t work = 0;
CountingThread counting_thread(&barrier, &mutex, &work); CountingThread counting_thread(&barrier, &mutex, &work);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment