Enable lazy compilation for non-trivial outer contexts.

This changes the compiler to be more aggressive about lazy compilation
of closures with non-trivial outer context. Compilation can only be
triggered with a valid outer context now. One exception is the debugger,
which can request compilation of arbitrary shared code, but it ensures
to trigger compilation only at points where no context is needed.

This relands r11782, r11783, r11790 and a minor fix.

R=ulan@chromium.org
TEST=mjsunit/debug-script-breakpoints-nested

Review URL: https://chromiumcodereview.appspot.com/10543141

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11866 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ast.cc

@@ -156,6 +156,11 @@ bool FunctionLiteral::AllowsLazyCompilation() {
 }
 
 
+bool FunctionLiteral::AllowsLazyCompilationWithoutContext() {
+  return scope()->AllowsLazyCompilationWithoutContext();
+}
+
+
 int FunctionLiteral::start_position() const {
   return scope()->start_position();
 }

src/ast.h

@@ -2065,6 +2065,7 @@ class FunctionLiteral: public Expression {
   int parameter_count() { return parameter_count_; }
 
   bool AllowsLazyCompilation();
+  bool AllowsLazyCompilationWithoutContext();
 
   Handle<String> debug_name() const {
     if (name_->length() > 0) return name_;

src/compiler.cc

@@ -118,7 +118,7 @@ bool CompilationInfo::ShouldSelfOptimize() {
       FLAG_crankshaft &&
       !function()->flags()->Contains(kDontSelfOptimize) &&
       !function()->flags()->Contains(kDontOptimize) &&
-      function()->scope()->AllowsLazyRecompilation() &&
+      function()->scope()->AllowsLazyCompilation() &&
       (shared_info().is_null() || !shared_info()->optimization_disabled());
 }
 
@@ -137,9 +137,8 @@ void CompilationInfo::AbortOptimization() {
 // all. However crankshaft support recompilation of functions, so in this case
 // the full compiler need not be be used if a debugger is attached, but only if
 // break points has actually been set.
-static bool is_debugging_active() {
+static bool IsDebuggerActive(Isolate* isolate) {
 #ifdef ENABLE_DEBUGGER_SUPPORT
-  Isolate* isolate = Isolate::Current();
   return V8::UseCrankshaft() ?
     isolate->debug()->has_break_points() :
     isolate->debugger()->IsDebuggerActive();
@@ -149,8 +148,8 @@ static bool is_debugging_active() {
 }
 
 
-static bool AlwaysFullCompiler() {
-  return FLAG_always_full_compiler || is_debugging_active();
+static bool AlwaysFullCompiler(Isolate* isolate) {
+  return FLAG_always_full_compiler || IsDebuggerActive(isolate);
 }
 
 
@@ -205,7 +204,7 @@ static bool MakeCrankshaftCode(CompilationInfo* info) {
   // Fall back to using the full code generator if it's not possible
   // to use the Hydrogen-based optimizing compiler. We already have
   // generated code for this from the shared function object.
-  if (AlwaysFullCompiler()) {
+  if (AlwaysFullCompiler(info->isolate())) {
     info->SetCode(code);
     return true;
   }
@@ -758,8 +757,14 @@ Handle<SharedFunctionInfo> Compiler::BuildFunctionInfo(FunctionLiteral* literal,
   // builtins cannot be handled lazily by the parser, since we have to know
   // if a function uses the special natives syntax, which is something the
   // parser records.
+  // If the debugger requests compilation for break points, we cannot be
+  // aggressive about lazy compilation, because it might trigger compilation
+  // of functions without an outer context when setting a breakpoint through
+  // Runtime::FindSharedFunctionInfoInScript.
+  bool allow_lazy_without_ctx = literal->AllowsLazyCompilationWithoutContext();
   bool allow_lazy = literal->AllowsLazyCompilation() &&
-      !LiveEditFunctionTracker::IsActive(info.isolate());
+      !LiveEditFunctionTracker::IsActive(info.isolate()) &&
+      (!info.isolate()->DebuggerHasBreakPoints() || allow_lazy_without_ctx);
 
   Handle<ScopeInfo> scope_info(ScopeInfo::Empty());
 
@@ -784,6 +789,7 @@ Handle<SharedFunctionInfo> Compiler::BuildFunctionInfo(FunctionLiteral* literal,
   SetFunctionInfo(result, literal, false, script);
   RecordFunctionCompilation(Logger::FUNCTION_TAG, &info, result);
   result->set_allows_lazy_compilation(allow_lazy);
+  result->set_allows_lazy_compilation_without_context(allow_lazy_without_ctx);
 
   // Set the expected number of properties for instances and return
   // the resulting function.
@@ -816,6 +822,8 @@ void Compiler::SetFunctionInfo(Handle<SharedFunctionInfo> function_info,
       lit->has_only_simple_this_property_assignments(),
       *lit->this_property_assignments());
   function_info->set_allows_lazy_compilation(lit->AllowsLazyCompilation());
+  function_info->set_allows_lazy_compilation_without_context(
+      lit->AllowsLazyCompilationWithoutContext());
   function_info->set_language_mode(lit->language_mode());
   function_info->set_uses_arguments(lit->scope()->arguments() != NULL);
   function_info->set_has_duplicate_parameters(lit->has_duplicate_parameters());

src/debug.h

@@ -239,12 +239,15 @@ class Debug {
                                         int count,
                                         int end));
   Object* Break(Arguments args);
-  void SetBreakPoint(Handle<SharedFunctionInfo> shared,
+  void SetBreakPoint(Handle<JSFunction> function,
+                     Handle<Object> break_point_object,
+                     int* source_position);
+  bool SetBreakPointForScript(Handle<Script> script,
                               Handle<Object> break_point_object,
                               int* source_position);
   void ClearBreakPoint(Handle<Object> break_point_object);
   void ClearAllBreakPoints();
-  void FloodWithOneShot(Handle<SharedFunctionInfo> shared);
+  void FloodWithOneShot(Handle<JSFunction> function);
   void FloodBoundFunctionWithOneShot(Handle<JSFunction> function);
   void FloodHandlerWithOneShot();
   void ChangeBreakOnException(ExceptionBreakType type, bool enable);
@@ -260,8 +263,11 @@ class Debug {
 
   void PrepareForBreakPoints();
 
-  // Returns whether the operation succeeded.
-  bool EnsureDebugInfo(Handle<SharedFunctionInfo> shared);
+  // Returns whether the operation succeeded. Compilation can only be triggered
+  // if a valid closure is passed as the second argument, otherwise the shared
+  // function needs to be compiled already.
+  bool EnsureDebugInfo(Handle<SharedFunctionInfo> shared,
+                       Handle<JSFunction> function);
 
   // Returns true if the current stub call is patched to call the debugger.
   static bool IsDebugBreak(Address addr);

src/full-codegen.cc

@@ -315,7 +315,7 @@ bool FullCodeGenerator::MakeCode(CompilationInfo* info) {
   Handle<Code> code = CodeGenerator::MakeCodeEpilogue(&masm, flags, info);
   code->set_optimizable(info->IsOptimizable() &&
                         !info->function()->flags()->Contains(kDontOptimize) &&
-                        info->function()->scope()->AllowsLazyRecompilation());
+                        info->function()->scope()->AllowsLazyCompilation());
   cgen.PopulateDeoptimizationData(code);
   cgen.PopulateTypeFeedbackInfo(code);
   cgen.PopulateTypeFeedbackCells(code);

src/objects-inl.h

@@ -3686,6 +3686,10 @@ BOOL_ACCESSORS(SharedFunctionInfo,
                compiler_hints,
                allows_lazy_compilation,
                kAllowLazyCompilation)
+BOOL_ACCESSORS(SharedFunctionInfo,
+               compiler_hints,
+               allows_lazy_compilation_without_context,
+               kAllowLazyCompilationWithoutContext)
 BOOL_ACCESSORS(SharedFunctionInfo,
                compiler_hints,
                uses_arguments,

src/objects.cc

@@ -7472,12 +7472,6 @@ void JSFunction::MarkForLazyRecompilation() {
 }
 
 
-bool SharedFunctionInfo::EnsureCompiled(Handle<SharedFunctionInfo> shared,
-                                        ClearExceptionFlag flag) {
-  return shared->is_compiled() || CompileLazy(shared, flag);
-}
-
-
 static bool CompileLazyHelper(CompilationInfo* info,
                               ClearExceptionFlag flag) {
   // Compile the source information to a code object.
@@ -7494,6 +7488,7 @@ static bool CompileLazyHelper(CompilationInfo* info,
 
 bool SharedFunctionInfo::CompileLazy(Handle<SharedFunctionInfo> shared,
                                      ClearExceptionFlag flag) {
+  ASSERT(shared->allows_lazy_compilation_without_context());
   CompilationInfo info(shared);
   return CompileLazyHelper(&info, flag);
 }
@@ -7554,6 +7549,7 @@ bool JSFunction::CompileLazy(Handle<JSFunction> function,
     function->ReplaceCode(function->shared()->code());
     function->shared()->set_code_age(0);
   } else {
+    ASSERT(function->shared()->allows_lazy_compilation());
     CompilationInfo info(function);
     result = CompileLazyHelper(&info, flag);
     ASSERT(!result || function->is_compiled());
@@ -7571,6 +7567,12 @@ bool JSFunction::CompileOptimized(Handle<JSFunction> function,
 }
 
 
+bool JSFunction::EnsureCompiled(Handle<JSFunction> function,
+                                ClearExceptionFlag flag) {
+  return function->is_compiled() || CompileLazy(function, flag);
+}
+
+
 bool JSFunction::IsInlineable() {
   if (IsBuiltin()) return false;
   SharedFunctionInfo* shared_info = shared();

src/objects.h

@@ -5460,6 +5460,12 @@ class SharedFunctionInfo: public HeapObject {
   // when doing GC if we expect that the function will no longer be used.
   DECL_BOOLEAN_ACCESSORS(allows_lazy_compilation)
 
+  // Indicates if this function can be lazy compiled without a context.
+  // This is used to determine if we can force compilation without reaching
+  // the function through program execution but through other means (e.g. heap
+  // iteration by the debugger).
+  DECL_BOOLEAN_ACCESSORS(allows_lazy_compilation_without_context)
+
   // Indicates how many full GCs this function has survived with assigned
   // code object. Used to determine when it is relatively safe to flush
   // this code object and replace it with lazy compilation stub.
@@ -5606,10 +5612,9 @@ class SharedFunctionInfo: public HeapObject {
 
   void ResetForNewContext(int new_ic_age);
 
-  // Helpers to compile the shared code.  Returns true on success, false on
-  // failure (e.g., stack overflow during compilation).
-  static bool EnsureCompiled(Handle<SharedFunctionInfo> shared,
-                             ClearExceptionFlag flag);
+  // Helper to compile the shared code.  Returns true on success, false on
+  // failure (e.g., stack overflow during compilation). This is only used by
+  // the debugger, it is not possible to compile without a context otherwise.
   static bool CompileLazy(Handle<SharedFunctionInfo> shared,
                           ClearExceptionFlag flag);
 
@@ -5744,6 +5749,7 @@ class SharedFunctionInfo: public HeapObject {
   enum CompilerHints {
     kHasOnlySimpleThisPropertyAssignments,
     kAllowLazyCompilation,
+    kAllowLazyCompilationWithoutContext,
     kLiveObjectsMayExist,
     kCodeAgeShift,
     kOptimizationDisabled = kCodeAgeShift + kCodeAgeSize,
@@ -5892,6 +5898,8 @@ class JSFunction: public JSObject {
 
   // Helpers to compile this function.  Returns true on success, false on
   // failure (e.g., stack overflow during compilation).
+  static bool EnsureCompiled(Handle<JSFunction> function,
+                             ClearExceptionFlag flag);
   static bool CompileLazy(Handle<JSFunction> function,
                           ClearExceptionFlag flag);
   static bool CompileOptimized(Handle<JSFunction> function,

src/parser.cc

@@ -4521,7 +4521,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
     // The heuristics are:
     // - It must not have been prohibited by the caller to Parse (some callers
     //   need a full AST).
-    // - The outer scope must be trivial (only global variables in scope).
     // - The function mustn't be a function expression with an open parenthesis
     //   before; we consider that a hint that the function will be called
     //   immediately, and it would be a waste of time to make it lazily
@@ -4529,8 +4528,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
     // These are all things we can know at this point, without looking at the
     // function itself.
     bool is_lazily_compiled = (mode() == PARSE_LAZILY &&
-                               top_scope_->outer_scope()->is_global_scope() &&
-                               top_scope_->HasTrivialOuterContext() &&
                                !parenthesized_function_);
     parenthesized_function_ = false;  // The bit was set for this function only.
 

src/scopes.cc

@@ -637,11 +637,6 @@ bool Scope::AllocateVariables(CompilationInfo* info,
 }
 
 
-bool Scope::AllowsLazyCompilation() const {
-  return !force_eager_compilation_ && HasTrivialOuterContext();
-}
-
-
 bool Scope::HasTrivialContext() const {
   // A function scope has a trivial context if it always is the global
   // context. We iteratively scan out the context chain to see if
@@ -666,12 +661,17 @@ bool Scope::HasTrivialOuterContext() const {
 }
 
 
-bool Scope::AllowsLazyRecompilation() const {
+bool Scope::AllowsLazyCompilation() const {
   return !force_eager_compilation_ &&
          !TrivialDeclarationScopesBeforeWithScope();
 }
 
 
+bool Scope::AllowsLazyCompilationWithoutContext() const {
+  return !force_eager_compilation_ && HasTrivialOuterContext();
+}
+
+
 bool Scope::TrivialDeclarationScopesBeforeWithScope() const {
   Scope* outer = outer_scope_;
   if (outer == NULL) return false;

src/scopes.h

@@ -374,8 +374,8 @@ class Scope: public ZoneObject {
   // Determine if we can use lazy compilation for this scope.
   bool AllowsLazyCompilation() const;
 
-  // True if we can lazily recompile functions with this scope.
-  bool AllowsLazyRecompilation() const;
+  // Determine if we can use lazy compilation for this scope without a context.
+  bool AllowsLazyCompilationWithoutContext() const;
 
   // True if the outer context of this scope is always the global context.
   bool HasTrivialOuterContext() const;

test/cctest/test-debug.cc

@@ -197,10 +197,9 @@ static bool HasDebugInfo(v8::Handle<v8::Function> fun) {
 // number.
 static int SetBreakPoint(Handle<v8::internal::JSFunction> fun, int position) {
   static int break_point = 0;
-  Handle<v8::internal::SharedFunctionInfo> shared(fun->shared());
   v8::internal::Debug* debug = v8::internal::Isolate::Current()->debug();
   debug->SetBreakPoint(
-      shared,
+      fun,
       Handle<Object>(v8::internal::Smi::FromInt(++break_point)),
       &position);
   return break_point;
@@ -515,7 +514,7 @@ void CheckDebugBreakFunction(DebugLocalContext* env,
   // there
   ClearBreakPoint(bp);
   CHECK(!debug->HasDebugInfo(shared));
-  CHECK(debug->EnsureDebugInfo(shared));
+  CHECK(debug->EnsureDebugInfo(shared, fun));
   TestBreakLocationIterator it2(Debug::GetDebugInfo(shared));
   it2.FindBreakLocationFromPosition(position);
   actual_mode = it2.it()->rinfo()->rmode();

test/cctest/test-heap.cc

@@ -1198,6 +1198,7 @@ TEST(TestSizeOfObjects) {
   HEAP->CollectAllGarbage(Heap::kNoGCFlags);
   HEAP->CollectAllGarbage(Heap::kNoGCFlags);
   HEAP->CollectAllGarbage(Heap::kNoGCFlags);
+  HEAP->CollectAllGarbage(Heap::kNoGCFlags);
   CHECK(HEAP->old_pointer_space()->IsSweepingComplete());
   int initial_size = static_cast<int>(HEAP->SizeOfObjects());
 

test/mjsunit/debug-script-breakpoints-closure.js

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+Debug = debug.Debug
+
+// Simple debug event handler which just counts the number of break points hit.
+var break_point_hit_count = 0;
+
+function listener(event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.Break) {
+    break_point_hit_count++;
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+function makeClosure() {
+  var x;
+  return function() {
+    return x;                // Breakpoint line ( #47 )
+  };
+}
+
+// Create closure before break point is set.
+var closure = makeClosure();
+
+// The debugger triggers re-compilation.
+assertEquals(0, Debug.scriptBreakPoints().length);
+var scr = Debug.findScript(makeClosure);
+var sbp = Debug.setScriptBreakPointById(scr.id, 47);
+assertEquals(1, Debug.scriptBreakPoints().length);
+
+// Ensure the closure actually triggers a break point hit.
+closure();
+assertEquals(1, break_point_hit_count);
+
+// Remove script break point.
+assertEquals(1, Debug.scriptBreakPoints().length);
+Debug.clearBreakPoint(sbp);
+assertEquals(0, Debug.scriptBreakPoints().length);

test/mjsunit/debug-script-breakpoints-nested.js

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+Debug = debug.Debug
+
+// Simple debug event handler which just counts the number of break points hit.
+var break_point_hit_count = 0;
+
+function listener(event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.Break) {
+    break_point_hit_count++;
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+eval(
+  "var inner;\n" +
+  "function outer() {\n" +         // Non-trivial outer closure.
+  "  var x = 5;\n" +
+  "  function a() {\n" +
+  "    var foo = 0, y = 7;\n" +
+  "    function b() {\n" +
+  "      var bar = 0, baz = 0, z = 11;\n" +
+  "      function c() {\n" +
+  "        return x + y + z;\n" +  // Breakpoint line ( #8 )
+  "      }\n" +
+  "      inner = c;\n" +
+  "      return c();\n" +
+  "    }\n" +
+  "    return b();\n" +
+  "  }\n" +
+  "  return a();\n" +
+  "}"
+);
+
+var script = Debug.findScript(outer);
+
+// The debugger triggers compilation of inner closures.
+assertEquals(0, Debug.scriptBreakPoints().length);
+var sbp = Debug.setScriptBreakPointById(script.id, 8);
+assertEquals(1, Debug.scriptBreakPoints().length);
+
+// The compiled outer closure should behave correctly.
+assertEquals(23, outer());
+assertEquals(1, break_point_hit_count);
+
+// The compiled inner closure should behave correctly.
+assertEquals(23, inner());
+assertEquals(2, break_point_hit_count);
+
+// Remove script break point.
+assertEquals(1, Debug.scriptBreakPoints().length);
+Debug.clearBreakPoint(sbp);
+assertEquals(0, Debug.scriptBreakPoints().length);