Skip to content

V
V8
Commit d4b5bfe3 authored by rmcilroy's avatar rmcilroy Committed by Commit bot
Browse files
Options

Revert of Use background tasks for the compiler dispatcher (patchset #5... 

Revert of Use background tasks for the compiler dispatcher (patchset #5 id:80001 of https://codereview.chromium.org/2606263002/ )

Reason for revert:
Causes IgnitionCompilerDispatcherTest.FinishNowWithBackgroundTask to fail.

https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%20debug/builds/11209

Original issue's description:
> Use background tasks for the compiler dispatcher
>
> BUG=v8:5215
> R=marja@chromium.org,vogelheim@chromium.org
>
> Review-Url: https://codereview.chromium.org/2606263002
> Cr-Commit-Position: refs/heads/master@{#42035}
> Committed: https://chromium.googlesource.com/v8/v8/+/7a1b3a7bebbef88e72c9f7747e1930f10ee10c80

TBR=marja@chromium.org,vogelheim@chromium.org,jochen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5215

Review-Url: https://codereview.chromium.org/2614433002
Cr-Commit-Position: refs/heads/master@{#42037}
parent 37de62c9
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 42 additions and 432 deletions
src/cancelable-task.cc
View file @ d4b5bfe3
......@@ -93,18 +93,13 @@ void CancelableTaskManager::CancelAndWait() {
}
}
CancelableTask::CancelableTask(Isolate* isolate)
: CancelableTask(isolate, isolate->cancelable_task_manager()) {}
: Cancelable(isolate->cancelable_task_manager()), isolate_(isolate) {}
CancelableTask::CancelableTask(Isolate* isolate, CancelableTaskManager* manager)
: Cancelable(manager), isolate_(isolate) {}
CancelableIdleTask::CancelableIdleTask(Isolate* isolate)
: CancelableIdleTask(isolate, isolate->cancelable_task_manager()) {}
CancelableIdleTask::CancelableIdleTask(Isolate* isolate,
CancelableTaskManager* manager)
: Cancelable(manager), isolate_(isolate) {}
: Cancelable(isolate->cancelable_task_manager()), isolate_(isolate) {}
} // namespace internal
} // namespace v8
src/cancelable-task.h
View file @ d4b5bfe3
......@@ -126,7 +126,6 @@ class V8_EXPORT_PRIVATE Cancelable {
class CancelableTask : public Cancelable, public Task {
public:
explicit CancelableTask(Isolate* isolate);
CancelableTask(Isolate* isolate, CancelableTaskManager* manager);
// Task overrides.
void Run() final {
......@@ -149,7 +148,6 @@ class CancelableTask : public Cancelable, public Task {
class CancelableIdleTask : public Cancelable, public IdleTask {
public:
explicit CancelableIdleTask(Isolate* isolate);
CancelableIdleTask(Isolate* isolate, CancelableTaskManager* manager);
// IdleTask overrides.
void Run(double deadline_in_seconds) final {
......
src/compiler-dispatcher/compiler-dispatcher.h
View file @ d4b5bfe3
......@@ -7,12 +7,9 @@
#include <map>
#include <memory>
#include <unordered_set>
#include <utility>
#include "src/base/macros.h"
#include "src/base/platform/condition-variable.h"
#include "src/base/platform/mutex.h"
#include "src/globals.h"
#include "testing/gtest/include/gtest/gtest_prod.h"
......@@ -22,7 +19,6 @@ class Platform;
namespace internal {
class CancelableTaskManager;
class CompilerDispatcherJob;
class CompilerDispatcherTracer;
class Isolate;
......@@ -33,30 +29,6 @@ class Handle;
// The CompilerDispatcher uses a combination of idle tasks and background tasks
// to parse and compile lazily parsed functions.
//
// As both parsing and compilation currently requires a preparation and
// finalization step that happens on the main thread, every task has to be
// advanced during idle time first. Depending on the properties of the task, it
// can then be parsed or compiled on either background threads, or during idle
// time. Last, it has to be finalized during idle time again.
//
// CompilerDispatcher::jobs_ maintains the list of all CompilerDispatcherJobs
// the CompilerDispatcher knows about.
//
// CompilerDispatcher::pending_background_jobs_ contains the set of
// CompilerDispatcherJobs that can be processed on a background thread.
//
// CompilerDispatcher::running_background_jobs_ contains the set of
// CompilerDispatcherJobs that are currently being processed on a background
// thread.
//
// CompilerDispatcher::DoIdleWork tries to advance as many jobs out of jobs_ as
// possible during idle time. If a job can't be advanced, but is suitable for
// background processing, it fires off background threads.
//
// CompilerDispatcher::DoBackgroundWork advances one of the pending jobs, and
// then spins of another idle task to potentially do the final step on the main
// thread.
class V8_EXPORT_PRIVATE CompilerDispatcher {
public:
enum class BlockingBehavior { kBlock, kDontBlock };
......@@ -83,23 +55,15 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
private:
FRIEND_TEST(CompilerDispatcherTest, IdleTaskSmallIdleTime);
FRIEND_TEST(IgnitionCompilerDispatcherTest, CompileOnBackgroundThread);
FRIEND_TEST(IgnitionCompilerDispatcherTest, FinishNowWithBackgroundTask);
typedef std::multimap<std::pair<int, int>,
std::unique_ptr<CompilerDispatcherJob>>
JobMap;
class BackgroundTask;
class IdleTask;
void WaitForJobIfRunningOnBackground(CompilerDispatcherJob* job);
bool IsEnabled() const;
JobMap::const_iterator GetJobFor(Handle<SharedFunctionInfo> shared) const;
void ConsiderJobForBackgroundProcessing(CompilerDispatcherJob* job);
void ScheduleMoreBackgroundTasksIfNeeded();
void ScheduleIdleTaskFromAnyThread();
void ScheduleIdleTaskIfNeeded();
void DoBackgroundWork();
void DoIdleWork(double deadline_in_seconds);
Isolate* isolate_;
......@@ -107,33 +71,12 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
size_t max_stack_size_;
std::unique_ptr<CompilerDispatcherTracer> tracer_;
std::unique_ptr<CancelableTaskManager> task_manager_;
bool idle_task_scheduled_;
// Mapping from (script id, function literal id) to job. We use a multimap,
// as script id is not necessarily unique.
JobMap jobs_;
// The following members can be accessed from any thread. Methods need to hold
// the mutex |mutex_| while accessing them.
base::Mutex mutex_;
bool idle_task_scheduled_;
// Number of currently scheduled BackgroundTask objects.
size_t num_scheduled_background_tasks_;
// The set of CompilerDispatcherJobs that can be advanced on any thread.
std::unordered_set<CompilerDispatcherJob*> pending_background_jobs_;
// The set of CompilerDispatcherJobs currently processed on background
// threads.
std::unordered_set<CompilerDispatcherJob*> running_background_jobs_;
// If not nullptr, then the main thread waits for the task processing
// this job, and blocks on the ConditionVariable main_thread_blocking_signal_.
CompilerDispatcherJob* main_thread_blocking_on_job_;
base::ConditionVariable main_thread_blocking_signal_;
DISALLOW_COPY_AND_ASSIGN(CompilerDispatcher);
};
......
test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
View file @ d4b5bfe3
......@@ -5,13 +5,10 @@
#include "src/compiler-dispatcher/compiler-dispatcher.h"
#include "include/v8-platform.h"
#include "src/base/platform/semaphore.h"
#include "src/compiler-dispatcher/compiler-dispatcher-job.h"
#include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
#include "src/flags.h"
#include "src/handles.h"
#include "src/objects-inl.h"
#include "src/v8.h"
#include "test/unittests/compiler-dispatcher/compiler-dispatcher-helper.h"
#include "test/unittests/test-utils.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -43,44 +40,16 @@ class CompilerDispatcherTest : public TestWithContext {
bool CompilerDispatcherTest::old_flag_;
class IgnitionCompilerDispatcherTest : public CompilerDispatcherTest {
public:
IgnitionCompilerDispatcherTest() = default;
~IgnitionCompilerDispatcherTest() override = default;
static void SetUpTestCase() {
old_flag_ = i::FLAG_ignition;
i::FLAG_ignition = true;
CompilerDispatcherTest::SetUpTestCase();
}
static void TearDownTestCase() {
CompilerDispatcherTest::TearDownTestCase();
i::FLAG_ignition = old_flag_;
}
private:
static bool old_flag_;
DISALLOW_COPY_AND_ASSIGN(IgnitionCompilerDispatcherTest);
};
bool IgnitionCompilerDispatcherTest::old_flag_;
namespace {
class MockPlatform : public v8::Platform {
public:
MockPlatform() : idle_task_(nullptr), time_(0.0), time_step_(0.0), sem_(0) {}
~MockPlatform() override {
EXPECT_TRUE(tasks_.empty());
EXPECT_TRUE(idle_task_ == nullptr);
}
size_t NumberOfAvailableBackgroundThreads() override { return 1; }
MockPlatform() : task_(nullptr), time_(0.0), time_step_(0.0) {}
~MockPlatform() override = default;
void CallOnBackgroundThread(Task* task,
ExpectedRuntime expected_runtime) override {
tasks_.push_back(task);
UNREACHABLE();
}
void CallOnForegroundThread(v8::Isolate* isolate, Task* task) override {
......@@ -94,8 +63,7 @@ class MockPlatform : public v8::Platform {
void CallIdleOnForegroundThread(v8::Isolate* isolate,
IdleTask* task) override {
ASSERT_TRUE(idle_task_ == nullptr);
idle_task_ = task;
task_ = task;
}
bool IdleTasksEnabled(v8::Isolate* isolate) override { return true; }
......@@ -106,78 +74,21 @@ class MockPlatform : public v8::Platform {
}
void RunIdleTask(double deadline_in_seconds, double time_step) {
ASSERT_TRUE(idle_task_ != nullptr);
ASSERT_TRUE(task_ != nullptr);
time_step_ = time_step;
IdleTask* task = idle_task_;
idle_task_ = nullptr;
IdleTask* task = task_;
task_ = nullptr;
task->Run(deadline_in_seconds);
delete task;
}
bool IdleTaskPending() const { return idle_task_; }
bool BackgroundTasksPending() const { return !tasks_.empty(); }
void RunBackgroundTasksAndBlock(Platform* platform) {
std::vector<Task*> tasks;
tasks.swap(tasks_);
platform->CallOnBackgroundThread(new TaskWrapper(this, tasks, true),
kShortRunningTask);
sem_.Wait();
}
void RunBackgroundTasks(Platform* platform) {
std::vector<Task*> tasks;
tasks.swap(tasks_);
platform->CallOnBackgroundThread(new TaskWrapper(this, tasks, false),
kShortRunningTask);
}
void ClearBackgroundTasks() {
std::vector<Task*> tasks;
tasks.swap(tasks_);
for (auto& task : tasks) {
delete task;
}
}
void ClearIdleTask() {
ASSERT_TRUE(idle_task_ != nullptr);
delete idle_task_;
idle_task_ = nullptr;
}
bool IdleTaskPending() const { return !!task_; }
private:
class TaskWrapper : public Task {
public:
TaskWrapper(MockPlatform* platform, const std::vector<Task*>& tasks,
bool signal)
: platform_(platform), tasks_(tasks), signal_(signal) {}
~TaskWrapper() = default;
void Run() override {
for (auto& task : tasks_) {
task->Run();
delete task;
}
if (signal_) platform_->sem_.Signal();
}
private:
MockPlatform* platform_;
std::vector<Task*> tasks_;
bool signal_;
DISALLOW_COPY_AND_ASSIGN(TaskWrapper);
};
IdleTask* idle_task_;
IdleTask* task_;
double time_;
double time_step_;
std::vector<Task*> tasks_;
base::Semaphore sem_;
DISALLOW_COPY_AND_ASSIGN(MockPlatform);
};
......@@ -201,10 +112,8 @@ TEST_F(CompilerDispatcherTest, IsEnqueued) {
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(dispatcher.Enqueue(shared));
ASSERT_TRUE(dispatcher.IsEnqueued(shared));
dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kBlock);
dispatcher.Abort(shared, CompilerDispatcher::BlockingBehavior::kBlock);
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(platform.IdleTaskPending());
platform.ClearIdleTask();
}
TEST_F(CompilerDispatcherTest, FinishNow) {
......@@ -223,8 +132,6 @@ TEST_F(CompilerDispatcherTest, FinishNow) {
// Finishing removes the SFI from the queue.
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(shared->is_compiled());
ASSERT_TRUE(platform.IdleTaskPending());
platform.ClearIdleTask();
}
TEST_F(CompilerDispatcherTest, IdleTask) {
......@@ -281,7 +188,7 @@ TEST_F(CompilerDispatcherTest, IdleTaskSmallIdleTime) {
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToParse);
// Now grant a lot of idle time and freeze time.
// Only grant a lot of idle time and freeze time.
platform.RunIdleTask(1000.0, 0.0);
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
......@@ -318,91 +225,5 @@ TEST_F(CompilerDispatcherTest, IdleTaskException) {
ASSERT_FALSE(try_catch.HasCaught());
}
TEST_F(IgnitionCompilerDispatcherTest, CompileOnBackgroundThread) {
MockPlatform platform;
CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
const char script[] =
"function g() { var y = 1; function f6(x) { return x * y }; return f6; } "
"g();";
Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
ASSERT_FALSE(platform.IdleTaskPending());
ASSERT_TRUE(dispatcher.Enqueue(shared));
ASSERT_TRUE(platform.IdleTaskPending());
ASSERT_EQ(dispatcher.jobs_.size(), 1u);
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kInitial);
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
platform.RunIdleTask(10.0, 0.0);
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToCompile);
ASSERT_TRUE(dispatcher.IsEnqueued(shared));
ASSERT_FALSE(shared->is_compiled());
ASSERT_FALSE(platform.IdleTaskPending());
ASSERT_TRUE(platform.BackgroundTasksPending());
platform.RunBackgroundTasksAndBlock(V8::GetCurrentPlatform());
ASSERT_TRUE(platform.IdleTaskPending());
ASSERT_FALSE(platform.BackgroundTasksPending());
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kCompiled);
// Now grant a lot of idle time and freeze time.
platform.RunIdleTask(1000.0, 0.0);
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(shared->is_compiled());
ASSERT_FALSE(platform.IdleTaskPending());
}
TEST_F(IgnitionCompilerDispatcherTest, FinishNowWithBackgroundTask) {
MockPlatform platform;
CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
const char script[] =
"function g() { var y = 1; function f7(x) { return x * y }; return f7; } "
"g();";
Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
ASSERT_FALSE(platform.IdleTaskPending());
ASSERT_TRUE(dispatcher.Enqueue(shared));
ASSERT_TRUE(platform.IdleTaskPending());
ASSERT_EQ(dispatcher.jobs_.size(), 1u);
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kInitial);
// Make compiling super expensive, and advance job as much as possible on the
// foreground thread.
dispatcher.tracer_->RecordCompile(50000.0, 1);
platform.RunIdleTask(10.0, 0.0);
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToCompile);
ASSERT_TRUE(dispatcher.IsEnqueued(shared));
ASSERT_FALSE(shared->is_compiled());
ASSERT_FALSE(platform.IdleTaskPending());
ASSERT_TRUE(platform.BackgroundTasksPending());
// This does not block, but races with the FinishNow() call below.
platform.RunBackgroundTasks(V8::GetCurrentPlatform());
ASSERT_TRUE(dispatcher.FinishNow(shared));
// Finishing removes the SFI from the queue.
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(shared->is_compiled());
ASSERT_FALSE(platform.IdleTaskPending());
ASSERT_FALSE(platform.BackgroundTasksPending());
}
} // namespace internal
} // namespace v8
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