[Compiler] Avoid blocking on inner function parallel compilation.

test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc

Index: test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
diff --git a/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
index 420f9b71fbfeb264b7983ce4fd96eac2244f049b..b79388baf1ef6b0111e90575afb34762cbe8b062 100644
--- a/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
+++ b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
@@ -809,18 +809,19 @@ TEST_F(CompilerDispatcherTest, EnqueueParsed) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
-  const char script[] =
+  const char source[] =
       "function g() { var y = 1; function f17(x) { return x * y }; return f17; "
       "} g();";
-  Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
+  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.Enqueue(shared, parse_info.literal(),
+  ASSERT_TRUE(dispatcher.Enqueue(script, shared, parse_info.literal(),
                                  parse_info.zone_shared(), handles, handles));
   ASSERT_TRUE(dispatcher.IsEnqueued(shared));
 
@@ -836,18 +837,19 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStepParsed) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
-  const char script[] =
+  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(), script));
+  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(shared, parse_info.literal(),
+  ASSERT_TRUE(dispatcher.EnqueueAndStep(script, shared, parse_info.literal(),
                                         parse_info.zone_shared(), handles,
                                         handles));
   ASSERT_TRUE(dispatcher.IsEnqueued(shared));
@@ -861,56 +863,17 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStepParsed) {
   platform.ClearBackgroundTasks();
 }
 
-TEST_F(CompilerDispatcherTest, FinishAllNow) {
-  MockPlatform platform;
-  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
-
-  const char script1[] =
-      "function g() { var y = 1; function f19(x) { return x + y }; return f19; "
-      "} g();";
-  Handle<JSFunction> f1 = Handle<JSFunction>::cast(RunJS(isolate(), script1));
-  Handle<SharedFunctionInfo> shared1(f1->shared(), i_isolate());
-
-  const char script2[] =
-      "function g() { var y = 1; function f20(x) { return x * y }; return f20; "
-      "} g();";
-  Handle<JSFunction> f2 = Handle<JSFunction>::cast(RunJS(isolate(), script2));
-  Handle<SharedFunctionInfo> shared2(f2->shared(), i_isolate());
-
-  ASSERT_FALSE(shared1->is_compiled());
-  ASSERT_FALSE(shared2->is_compiled());
-
-  // Enqueue shared1 as already parsed.
-  ParseInfo parse_info(shared1);
-  ASSERT_TRUE(Compiler::ParseAndAnalyze(&parse_info));
-  std::shared_ptr<DeferredHandles> handles;
-  ASSERT_TRUE(dispatcher.Enqueue(shared1, parse_info.literal(),
-                                 parse_info.zone_shared(), handles, handles));
-
-  // Enqueue shared2 for parsing and compiling
-  ASSERT_TRUE(dispatcher.Enqueue(shared2));
-
-  ASSERT_TRUE(dispatcher.FinishAllNow());
-
-  // Finishing removes the SFI from the queue.
-  ASSERT_FALSE(dispatcher.IsEnqueued(shared1));
-  ASSERT_FALSE(dispatcher.IsEnqueued(shared2));
-  ASSERT_TRUE(shared1->is_compiled());
-  ASSERT_TRUE(shared2->is_compiled());
-  ASSERT_TRUE(platform.IdleTaskPending());
-  platform.ClearIdleTask();
-}
-
 TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
   Local<v8::Context> context = v8::Context::New(isolate());
 
-  const char script[] =
+  const char source[] =
       "function g() { var y = 1; function f20(x) { return x + y }; return f20; "
       "} g();";
-  Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
+  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());
 
   {
     HandleScope scope(i_isolate());  // Create handles scope for parsing.
@@ -926,7 +889,7 @@ TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) {
 
     ASSERT_FALSE(platform.IdleTaskPending());
     ASSERT_TRUE(dispatcher.Enqueue(
-        shared, parse_info.literal(), parse_info.zone_shared(),
+        script, shared, parse_info.literal(), parse_info.zone_shared(),
         parse_info.deferred_handles(), compilation_handles));
     ASSERT_TRUE(platform.IdleTaskPending());
   }
@@ -940,5 +903,28 @@ TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) {
   ASSERT_TRUE(shared->is_compiled());
 }
 
+TEST_F(CompilerDispatcherTest, CompileLazyFinishesDispatcherJob) {
+  // Use the real dispatcher so that CompileLazy checks the same one for
+  // enqueued functions.
+  CompilerDispatcher* dispatcher = i_isolate()->compiler_dispatcher();
+
+  const char source[] =
+      "function g() { var y = 1; function f16(x) { return x * y }; return f16; "
+      "} g();";

[marja, 2017/02/09 09:09:48]

Is this g() now unnecessary? It's a bit confusing...

[rmcilroy, 2017/02/09 10:37:57]

This is still needed (it returns f16 as the return value of the statement, which
is used to initialize Handle<JSFunction> f. f16 isn't actually called until the
"g()()" below.

[marja, 2017/02/09 16:22:01]

Acknowledged.

+  Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), source));
+  Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
+
+  ASSERT_FALSE(shared->is_compiled());
+  ASSERT_FALSE(dispatcher->IsEnqueued(shared));
+  ASSERT_TRUE(dispatcher->Enqueue(shared));
+  ASSERT_TRUE(dispatcher->IsEnqueued(shared));
+
+  // Now force the function to run and ensure CompileLazy finished and dequeues
+  // it from the dispatcher.
+  RunJS(isolate(), "g()();");
+  ASSERT_TRUE(shared->is_compiled());
+  ASSERT_FALSE(dispatcher->IsEnqueued(shared));
+}

[marja, 2017/02/09 16:22:01]

Would it be interesting to add a test like this:

function lazy() { lazy2(); }
function lazy2() { ... }

enqueue(lazy);
enqueue(lazy2); << need some trickery to get handles to both funcs
RunJS(lazy);
assert_false(isenqueued(lazy2));

[rmcilroy, 2017/02/10 11:21:16]

Sure, added this test. It ended up being slightly more complicated (I needed to
make sure lazy1 and lazy2 captured a variable from an outer scope due to the:
DCHECK(!shared_->outer_scope_info()->IsTheHole(isolate_));
in compiler-dispatcher-job.cc constructor. (see comment on new patch set).

+
 }  // namespace internal
 }  // namespace v8