Revision: 12715
Author:   [email protected]
Date:     Fri Oct 12 06:49:12 2012
Log: Reland r12342: Flush monomorphic ICs on context disposal instead of context exit.

The crashes that caused r12342 to be reverted are fixed in r12563.

[email protected]

Review URL: https://chromiumcodereview.appspot.com/11092081
http://code.google.com/p/v8/source/detail?r=12715

Modified:
 /branches/bleeding_edge/src/heap.cc
 /branches/bleeding_edge/src/heap.h
 /branches/bleeding_edge/src/objects-visiting-inl.h
 /branches/bleeding_edge/test/cctest/test-api.cc
 /branches/bleeding_edge/test/cctest/test-heap.cc
 /branches/bleeding_edge/test/mjsunit/debug-script.js
 /branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js

=======================================
--- /branches/bleeding_edge/src/heap.cc Fri Oct 12 04:41:14 2012
+++ /branches/bleeding_edge/src/heap.cc Fri Oct 12 06:49:12 2012
@@ -98,6 +98,7 @@
       linear_allocation_scope_depth_(0),
       contexts_disposed_(0),
       global_ic_age_(0),
+      flush_monomorphic_ics_(false),
       scan_on_scavenge_pages_(0),
       new_space_(this),
       old_pointer_space_(NULL),
@@ -999,7 +1000,7 @@

   contexts_disposed_ = 0;

-  isolate_->set_context_exit_happened(false);
+  flush_monomorphic_ics_ = false;
 }


=======================================
--- /branches/bleeding_edge/src/heap.h  Fri Oct 12 04:41:14 2012
+++ /branches/bleeding_edge/src/heap.h  Fri Oct 12 06:49:12 2012
@@ -1103,7 +1103,10 @@
   void EnsureHeapIsIterable();

   // Notify the heap that a context has been disposed.
-  int NotifyContextDisposed() { return ++contexts_disposed_; }
+  int NotifyContextDisposed() {
+    flush_monomorphic_ics_ = true;
+    return ++contexts_disposed_;
+  }

   // Utility to invoke the scavenger. This is needed in test code to
   // ensure correct callback for weak global handles.
@@ -1634,6 +1637,8 @@
   void AgeInlineCaches() {
global_ic_age_ = (global_ic_age_ + 1) & SharedFunctionInfo::ICAgeBits::kMax;
   }
+
+  bool flush_monomorphic_ics() { return flush_monomorphic_ics_; }

   intptr_t amount_of_external_allocated_memory() {
     return amount_of_external_allocated_memory_;
@@ -1720,6 +1725,8 @@

   int global_ic_age_;

+  bool flush_monomorphic_ics_;
+
   int scan_on_scavenge_pages_;

 #if defined(V8_TARGET_ARCH_X64)
=======================================
--- /branches/bleeding_edge/src/objects-visiting-inl.h Fri Oct 12 05:41:29 2012 +++ /branches/bleeding_edge/src/objects-visiting-inl.h Fri Oct 12 06:49:12 2012
@@ -214,9 +214,8 @@
   // when they might be keeping a Context alive, or when the heap is about
   // to be serialized.
   if (FLAG_cleanup_code_caches_at_gc && target->is_inline_cache_stub()
-      && (target->ic_state() == MEGAMORPHIC || Serializer::enabled() ||
-          heap->isolate()->context_exit_happened() ||
-          target->ic_age() != heap->global_ic_age())) {
+ && (target->ic_state() == MEGAMORPHIC || heap->flush_monomorphic_ics() || + Serializer::enabled() || target->ic_age() != heap->global_ic_age())) {
     IC::Clear(rinfo->pc());
     target = Code::GetCodeFromTargetAddress(rinfo->target_address());
   }
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Fri Sep 21 01:09:34 2012
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Fri Oct 12 06:49:12 2012
@@ -10840,18 +10840,21 @@
     { v8::HandleScope scope;
       LocalContext context;
     }
+    v8::V8::ContextDisposedNotification();
     CheckSurvivingGlobalObjectsCount(0);

     { v8::HandleScope scope;
       LocalContext context;
       v8_compile("Date")->Run();
     }
+    v8::V8::ContextDisposedNotification();
     CheckSurvivingGlobalObjectsCount(0);

     { v8::HandleScope scope;
       LocalContext context;
       v8_compile("/aaa/")->Run();
     }
+    v8::V8::ContextDisposedNotification();
     CheckSurvivingGlobalObjectsCount(0);

     { v8::HandleScope scope;
@@ -10860,6 +10863,7 @@
       LocalContext context(&extensions);
       v8_compile("gc();")->Run();
     }
+    v8::V8::ContextDisposedNotification();
     CheckSurvivingGlobalObjectsCount(0);
   }
 }
@@ -15006,6 +15010,7 @@
     context->Exit();
   }
   context.Dispose();
+  v8::V8::ContextDisposedNotification();
   for (gc_count = 1; gc_count < 10; gc_count++) {
     other_context->Enter();
     CompileRun(source_exception);
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Fri Oct 12 04:41:14 2012 +++ /branches/bleeding_edge/test/cctest/test-heap.cc Fri Oct 12 06:49:12 2012
@@ -1067,6 +1067,7 @@
     }

     // Mark compact handles the weak references.
+    ISOLATE->compilation_cache()->Clear();
     HEAP->CollectAllGarbage(Heap::kNoGCFlags);
     CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctions(ctx[i]));

@@ -1398,6 +1399,7 @@
     ctx2->Exit();
     ctx1->Exit();
     ctx1.Dispose();
+    v8::V8::ContextDisposedNotification();
   }
   HEAP->CollectAllAvailableGarbage();
   CHECK_EQ(2, NumberOfGlobalObjects());
@@ -1435,6 +1437,7 @@
     ctx2->Exit();
     ctx1->Exit();
     ctx1.Dispose();
+    v8::V8::ContextDisposedNotification();
   }
   HEAP->CollectAllAvailableGarbage();
   CHECK_EQ(2, NumberOfGlobalObjects());
@@ -1470,6 +1473,7 @@
     ctx2->Exit();
     ctx1->Exit();
     ctx1.Dispose();
+    v8::V8::ContextDisposedNotification();
   }
   HEAP->CollectAllAvailableGarbage();
   CHECK_EQ(2, NumberOfGlobalObjects());
@@ -1509,6 +1513,7 @@
     ctx2->Exit();
     ctx1->Exit();
     ctx1.Dispose();
+    v8::V8::ContextDisposedNotification();
   }
   HEAP->CollectAllAvailableGarbage();
   CHECK_EQ(2, NumberOfGlobalObjects());
@@ -2111,8 +2116,6 @@
   Code* ic_before = FindFirstIC(f->shared()->code(), Code::LOAD_IC);
   CHECK(ic_before->ic_state() == MONOMORPHIC);

-  // Fire context dispose notification.
-  v8::V8::ContextDisposedNotification();
   SimulateIncrementalMarking();
   HEAP->CollectAllGarbage(Heap::kNoGCFlags);

=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-script.js Mon Aug 27 09:08:27 2012 +++ /branches/bleeding_edge/test/mjsunit/debug-script.js Fri Oct 12 06:49:12 2012
@@ -25,9 +25,11 @@
 // (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 --expose-gc  --noparallel-recompilation
+// Flags: --expose-debug-as debug --expose-gc --noparallel-recompilation
+// Flags: --send-idle-notification
+
 // Get the Debug object exposed from the debug context global object.
-Debug = debug.Debug
+Debug = debug.Debug;

 Date();
 RegExp();
=======================================
--- /branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js Mon Oct 1 09:22:43 2012 +++ /branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js Fri Oct 12 06:49:12 2012
@@ -25,8 +25,7 @@
 // (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: --allow-natives-syntax --smi-only-arrays --expose-gc
-// Flags: --noparallel-recompilation
+// Flags: --allow-natives-syntax --smi-only-arrays --noparallel-recompilation

// Ensure that ElementsKind transitions in various situations are hoisted (or // not hoisted) correctly, don't change the semantics programs and don't trigger
@@ -39,11 +38,6 @@
 } else {
   print("Tests do NOT include smi-only arrays.");
 }
-
-// Force existing ICs from previous stress runs to be flushed, otherwise the -// assumptions in this test about when deoptimizations get triggered are not
-// valid.
-gc();

 if (support_smi_only_arrays) {
// Make sure that a simple elements array transitions inside a loop before
@@ -66,6 +60,7 @@
   testDoubleConversion4(new Array(5));
   testDoubleConversion4(new Array(5));
   assertTrue(2 != %GetOptimizationStatus(testDoubleConversion4));
+  %ClearFunctionTypeFeedback(testDoubleConversion4);

   // Make sure that non-element related map checks that are not preceded by
// transitions in a loop still get hoisted in a way that doesn't generate a
@@ -91,6 +86,7 @@
   testExactMapHoisting(new Array(5));
   testExactMapHoisting(new Array(5));
   assertTrue(2 != %GetOptimizationStatus(testExactMapHoisting));
+  %ClearFunctionTypeFeedback(testExactMapHoisting);

// Make sure that non-element related map checks do NOT get hoisted if they // depend on an elements transition before them and it's not possible to hoist
@@ -122,6 +118,7 @@
   testExactMapHoisting2(new Array(5));
   // Temporarily disabled - see bug 2176.
   // assertTrue(2 != %GetOptimizationStatus(testExactMapHoisting2));
+  %ClearFunctionTypeFeedback(testExactMapHoisting2);

// Make sure that non-element related map checks do get hoisted if they use // the transitioned map for the check and all transitions that they depend
@@ -150,6 +147,7 @@
   testExactMapHoisting3(new Array(5));
   testExactMapHoisting3(new Array(5));
   assertTrue(2 != %GetOptimizationStatus(testExactMapHoisting3));
+  %ClearFunctionTypeFeedback(testExactMapHoisting3);

   function testDominatingTransitionHoisting1(a) {
     var object = new Object();
@@ -176,6 +174,7 @@
   // above the access, causing a deopt. We should update the type of access
   // rather than forbid hoisting the transition.
assertTrue(2 != %GetOptimizationStatus(testDominatingTransitionHoisting1));
+  %ClearFunctionTypeFeedback(testDominatingTransitionHoisting1);
   */

   function testHoistingWithSideEffect(a) {
@@ -196,6 +195,7 @@
   testHoistingWithSideEffect(new Array(5));
   testHoistingWithSideEffect(new Array(5));
   assertTrue(2 != %GetOptimizationStatus(testHoistingWithSideEffect));
+  %ClearFunctionTypeFeedback(testHoistingWithSideEffect);

   function testStraightLineDupeElinination(a,b,c,d,e,f) {
     var count = 3;
@@ -234,4 +234,5 @@
   testStraightLineDupeElinination(new Array(5),0,0,0,0,0);
   testStraightLineDupeElinination(new Array(5),0,0,0,0,0);
   assertTrue(2 != %GetOptimizationStatus(testStraightLineDupeElinination));
+  %ClearFunctionTypeFeedback(testStraightLineDupeElinination);
 }

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to