Commit 96009d28 authored by Ross McIlroy's avatar Ross McIlroy Committed by Commit Bot

[Compiler] Avoid stepping a job in EnqueueAndStep if job is already enqueued.

If a job was already enqueued, EnqueueAndStep would still step the job one
more step. However, since it didn't take the job out of the
pending_background_jobs pool, the job could get picked up by a background
thread which would try to step it, but it the job is now at a step which
can't be run on the background.

BUG=v8:5203,chromium:685515

Change-Id: I2cee2a33625ba455aca49a8037601be9ff8bb73f
Reviewed-on: https://chromium-review.googlesource.com/441084
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarJochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43121}
parent 9675811a
......@@ -278,6 +278,7 @@ bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
}
bool CompilerDispatcher::EnqueueAndStep(Handle<SharedFunctionInfo> function) {
if (IsEnqueued(function)) return true;
if (!Enqueue(function)) return false;
if (trace_compiler_dispatcher_) {
......@@ -321,6 +322,7 @@ bool CompilerDispatcher::EnqueueAndStep(
FunctionLiteral* literal, std::shared_ptr<Zone> parse_zone,
std::shared_ptr<DeferredHandles> parse_handles,
std::shared_ptr<DeferredHandles> compile_handles) {
if (IsEnqueued(function)) return true;
if (!Enqueue(script, function, literal, parse_zone, parse_handles,
compile_handles)) {
return false;
......
......@@ -118,6 +118,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
private:
FRIEND_TEST(CompilerDispatcherTest, EnqueueAndStep);
FRIEND_TEST(CompilerDispatcherTest, EnqueueAndStepTwice);
FRIEND_TEST(CompilerDispatcherTest, EnqueueParsed);
FRIEND_TEST(CompilerDispatcherTest, EnqueueAndStepParsed);
FRIEND_TEST(CompilerDispatcherTest, IdleTaskSmallIdleTime);
......
......@@ -1060,5 +1060,47 @@ TEST_F(CompilerDispatcherTest, CompileLazy2FinishesDispatcherJob) {
ASSERT_FALSE(dispatcher->IsEnqueued(shared2));
}
TEST_F(CompilerDispatcherTest, EnqueueAndStepTwice) {
MockPlatform platform;
CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
const char source[] =
"function g() { var y = 1; function f18(x) { return x * y }; return f18; "
"} g();";
Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), source));
Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
Handle<Script> script(Script::cast(shared->script()), i_isolate());
ParseInfo parse_info(shared);
ASSERT_TRUE(Compiler::ParseAndAnalyze(&parse_info));
std::shared_ptr<DeferredHandles> handles;
ASSERT_FALSE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(dispatcher.EnqueueAndStep(script, shared, parse_info.literal(),
parse_info.zone_shared(), handles,
handles));
ASSERT_TRUE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToCompile);
// EnqueueAndStep of the same function again (either already parsed or for
// compile and parse) shouldn't step the job.
ASSERT_TRUE(dispatcher.EnqueueAndStep(script, shared, parse_info.literal(),
parse_info.zone_shared(), handles,
handles));
ASSERT_TRUE(dispatcher.IsEnqueued(shared));
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToCompile);
ASSERT_TRUE(dispatcher.EnqueueAndStep(shared));
ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
CompileJobStatus::kReadyToCompile);
ASSERT_TRUE(platform.IdleTaskPending());
ASSERT_TRUE(platform.BackgroundTasksPending());
platform.ClearIdleTask();
platform.ClearBackgroundTasks();
}
} // 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