Revision: 21388
Author:   [email protected]
Date:     Tue May 20 14:03:38 2014 UTC
Log:      Revert "Fix Heap::IsHeapIterable."

This reverts commit r21387.

[email protected]

Review URL: https://codereview.chromium.org/291193002
http://code.google.com/p/v8/source/detail?r=21388

Deleted:
 /branches/bleeding_edge/test/mjsunit/regress/regress-373283.js
Modified:
 /branches/bleeding_edge/src/debug.cc
 /branches/bleeding_edge/src/heap-profiler.cc
 /branches/bleeding_edge/src/heap-snapshot-generator.cc
 /branches/bleeding_edge/src/heap.cc
 /branches/bleeding_edge/src/heap.h
 /branches/bleeding_edge/src/liveedit.cc
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/test/cctest/test-api.cc
 /branches/bleeding_edge/test/cctest/test-heap.cc
 /branches/bleeding_edge/test/cctest/test-object-observe.cc

=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-373283.js Tue May 20 13:19:21 2014 UTC
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Flags:  --allow-natives-syntax --deopt-every-n-times=1
-
-function __f_0() {
-  var x = [];
-  x[21] = 1;
-  x[21] + 0;
-}
-
-for (var i = 0; i < 3; i++) __f_0();
-%OptimizeFunctionOnNextCall(__f_0);
-for (var i = 0; i < 10; i++) __f_0();
-%OptimizeFunctionOnNextCall(__f_0);
-__f_0();
-%GetScript("foo");
=======================================
--- /branches/bleeding_edge/src/debug.cc        Tue May 20 13:19:21 2014 UTC
+++ /branches/bleeding_edge/src/debug.cc        Tue May 20 14:03:38 2014 UTC
@@ -2048,7 +2048,6 @@
       Heap* heap = isolate_->heap();
       heap->CollectAllGarbage(Heap::kMakeHeapIterableMask,
                               "preparing for breakpoints");
-      HeapIterator iterator(heap);

       // Ensure no GC in this scope as we are going to use gc_metadata
       // field in the Code object to mark active functions.
@@ -2068,6 +2067,7 @@
       // Scan the heap for all non-optimized functions which have no
       // debug break slots and are not active or inlined into an active
       // function and mark them for lazy compilation.
+      HeapIterator iterator(heap);
       HeapObject* obj = NULL;
       while (((obj = iterator.next()) != NULL)) {
         if (obj->IsJSFunction()) {
@@ -2192,7 +2192,9 @@
   Handle<SharedFunctionInfo> target;
   Heap* heap = isolate_->heap();
   while (!done) {
-    { // Extra scope for iterator.
+    { // Extra scope for iterator and no-allocation.
+      heap->EnsureHeapIsIterable();
+      DisallowHeapAllocation no_alloc_during_heap_iteration;
       HeapIterator iterator(heap);
       for (HeapObject* obj = iterator.next();
            obj != NULL; obj = iterator.next()) {
@@ -2522,6 +2524,8 @@
// scripts which are no longer referenced. The second also sweeps precisely,
   // which saves us doing yet another GC to make the heap iterable.
   heap->CollectAllGarbage(Heap::kNoGCFlags, "Debug::CreateScriptCache");
+  heap->CollectAllGarbage(Heap::kMakeHeapIterableMask,
+                          "Debug::CreateScriptCache");

   ASSERT(script_cache_ == NULL);
   script_cache_ = new ScriptCache(isolate_);
@@ -2529,6 +2533,7 @@
   // Scan heap for Script objects.
   int count = 0;
   HeapIterator iterator(heap);
+  DisallowHeapAllocation no_allocation;

for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
     if (obj->IsScript() && Script::cast(obj)->HasValidSource()) {
=======================================
--- /branches/bleeding_edge/src/heap-profiler.cc Tue May 20 13:19:21 2014 UTC +++ /branches/bleeding_edge/src/heap-profiler.cc Tue May 20 14:03:38 2014 UTC
@@ -173,6 +173,9 @@


 Handle<HeapObject> HeapProfiler::FindHeapObjectById(SnapshotObjectId id) {
+  heap()->CollectAllGarbage(Heap::kMakeHeapIterableMask,
+                            "HeapProfiler::FindHeapObjectById");
+  DisallowHeapAllocation no_allocation;
   HeapObject* object = NULL;
   HeapIterator iterator(heap(), HeapIterator::kFilterUnreachable);
   // Make sure that object with the given id is still reachable.
=======================================
--- /branches/bleeding_edge/src/heap-snapshot-generator.cc Tue May 20 13:19:21 2014 UTC +++ /branches/bleeding_edge/src/heap-snapshot-generator.cc Tue May 20 14:03:38 2014 UTC
@@ -2569,6 +2569,10 @@
   CHECK(!debug_heap->map_space()->was_swept_conservatively());
 #endif

+  // The following code uses heap iterators, so we want the heap to be
+  // stable. It should follow TagGlobalObjects as that can allocate.
+  DisallowHeapAllocation no_alloc;
+
 #ifdef VERIFY_HEAP
   debug_heap->Verify();
 #endif
=======================================
--- /branches/bleeding_edge/src/heap.cc Tue May 20 13:19:21 2014 UTC
+++ /branches/bleeding_edge/src/heap.cc Tue May 20 14:03:38 2014 UTC
@@ -4321,15 +4321,14 @@

 bool Heap::IsHeapIterable() {
   return (!old_pointer_space()->was_swept_conservatively() &&
-          !old_data_space()->was_swept_conservatively() &&
-          new_space()->bottom() == new_space()->top());
+          !old_data_space()->was_swept_conservatively());
 }


-void Heap::MakeHeapIterable() {
+void Heap::EnsureHeapIsIterable() {
   ASSERT(AllowHeapAllocation::IsAllowed());
   if (!IsHeapIterable()) {
-    CollectAllGarbage(kMakeHeapIterableMask, "Heap::MakeHeapIterable");
+    CollectAllGarbage(kMakeHeapIterableMask, "Heap::EnsureHeapIsIterable");
   }
   ASSERT(IsHeapIterable());
 }
@@ -5733,9 +5732,7 @@


 HeapIterator::HeapIterator(Heap* heap)
-    : make_heap_iterable_helper_(heap),
-      no_heap_allocation_(),
-      heap_(heap),
+    : heap_(heap),
       filtering_(HeapIterator::kNoFiltering),
       filter_(NULL) {
   Init();
@@ -5744,9 +5741,7 @@

 HeapIterator::HeapIterator(Heap* heap,
                            HeapIterator::HeapObjectsFiltering filtering)
-    : make_heap_iterable_helper_(heap),
-      no_heap_allocation_(),
-      heap_(heap),
+    : heap_(heap),
       filtering_(filtering),
       filter_(NULL) {
   Init();
=======================================
--- /branches/bleeding_edge/src/heap.h  Tue May 20 13:19:21 2014 UTC
+++ /branches/bleeding_edge/src/heap.h  Tue May 20 14:03:38 2014 UTC
@@ -767,7 +767,7 @@

   // Ensure that we have swept all spaces in such a way that we can iterate
   // over all objects.  May cause a GC.
-  void MakeHeapIterable();
+  void EnsureHeapIsIterable();

   // Notify the heap that a context has been disposed.
   int NotifyContextDisposed();
@@ -2391,9 +2391,6 @@
 // aggregates the specific iterators for the different spaces as
 // these can only iterate over one space only.
 //
-// HeapIterator ensures there is no allocation during its lifetime
-// (using an embedded DisallowHeapAllocation instance).
-//
 // HeapIterator can skip free list nodes (that is, de-allocated heap
 // objects that still remain in the heap). As implementation of free
 // nodes filtering uses GC marks, it can't be used during MS/MC GC
@@ -2416,18 +2413,12 @@
   void reset();

  private:
-  struct MakeHeapIterableHelper {
- explicit MakeHeapIterableHelper(Heap* heap) { heap->MakeHeapIterable(); }
-  };
-
   // Perform the initialization.
   void Init();
   // Perform all necessary shutdown (destruction) work.
   void Shutdown();
   HeapObject* NextObject();

-  MakeHeapIterableHelper make_heap_iterable_helper_;
-  DisallowHeapAllocation no_heap_allocation_;
   Heap* heap_;
   HeapObjectsFiltering filtering_;
   HeapObjectsFilter* filter_;
=======================================
--- /branches/bleeding_edge/src/liveedit.cc     Tue May 20 13:19:21 2014 UTC
+++ /branches/bleeding_edge/src/liveedit.cc     Tue May 20 14:03:38 2014 UTC
@@ -939,10 +939,13 @@
   // to code objects (that are never in new space) without worrying about
   // write barriers.
   Heap* heap = original->GetHeap();
-  HeapIterator iterator(heap);
+  heap->CollectAllGarbage(Heap::kMakeHeapIterableMask,
+                          "liveedit.cc ReplaceCodeObject");

   ASSERT(!heap->InNewSpace(*substitution));

+  DisallowHeapAllocation no_allocation;
+
   ReplacingVisitor visitor(*original, *substitution);

// Iterate over all roots. Stack frames may have pointer into original code,
@@ -952,6 +955,7 @@

   // Now iterate over all pointers of all objects, including code_target
   // implicit pointers.
+  HeapIterator iterator(heap);
for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
     obj->Iterate(&visitor);
   }
@@ -1015,6 +1019,8 @@
   template<typename Visitor>
   static void IterateJSFunctions(SharedFunctionInfo* shared_info,
                                  Visitor* visitor) {
+    DisallowHeapAllocation no_allocation;
+
     HeapIterator iterator(shared_info->GetHeap());
     for (HeapObject* obj = iterator.next(); obj != NULL;
         obj = iterator.next()) {
@@ -1159,7 +1165,7 @@

   Handle<SharedFunctionInfo> shared_info = shared_info_wrapper.GetInfo();

-  isolate->heap()->MakeHeapIterable();
+  isolate->heap()->EnsureHeapIsIterable();

   if (IsJSFunctionCode(shared_info->code())) {
     Handle<Code> code = compile_info_wrapper.GetFunctionCode();
@@ -1396,7 +1402,7 @@
   info->set_end_position(new_function_end);
   info->set_function_token_position(new_function_token_pos);

-  info->GetIsolate()->heap()->MakeHeapIterable();
+  info->GetIsolate()->heap()->EnsureHeapIsIterable();

   if (IsJSFunctionCode(info->code())) {
     // Patch relocation info section of the code.
=======================================
--- /branches/bleeding_edge/src/runtime.cc      Tue May 20 13:19:21 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc      Tue May 20 14:03:38 2014 UTC
@@ -13129,6 +13129,14 @@
   HandleScope scope(isolate);
   ASSERT(args.length() == 3);

+ // First perform a full GC in order to avoid references from dead objects.
+  Heap* heap = isolate->heap();
+ heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "%DebugReferencedBy"); + // The heap iterator reserves the right to do a GC to make the heap iterable. + // Due to the GC above we know it won't need to do that, but it seems cleaner + // to get the heap iterator constructed before we start having unprotected
+  // Object* locals that are not protected by handles.
+
   // Check parameters.
   CONVERT_ARG_HANDLE_CHECKED(JSObject, target, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, instance_filter, 1);
@@ -13146,27 +13154,21 @@

   // Get the number of referencing objects.
   int count;
- // First perform a full GC in order to avoid dead objects and to make the heap
-  // iterable.
-  Heap* heap = isolate->heap();
- heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "%DebugConstructedBy");
-  {
-    HeapIterator heap_iterator(heap);
-    count = DebugReferencedBy(&heap_iterator,
-                              *target, *instance_filter, max_references,
-                              NULL, 0, *arguments_function);
-  }
+  HeapIterator heap_iterator(heap);
+  count = DebugReferencedBy(&heap_iterator,
+                            *target, *instance_filter, max_references,
+                            NULL, 0, *arguments_function);

   // Allocate an array to hold the result.
   Handle<FixedArray> instances = isolate->factory()->NewFixedArray(count);

   // Fill the referencing objects.
-  {
-    HeapIterator heap_iterator(heap);
-    count = DebugReferencedBy(&heap_iterator,
-                              *target, *instance_filter, max_references,
-                              *instances, count, *arguments_function);
-  }
+  // AllocateFixedArray above does not make the heap non-iterable.
+  ASSERT(heap->IsHeapIterable());
+  HeapIterator heap_iterator2(heap);
+  count = DebugReferencedBy(&heap_iterator2,
+                            *target, *instance_filter, max_references,
+                            *instances, count, *arguments_function);

   // Return result as JS array.
   Handle<JSFunction> constructor(
@@ -13217,6 +13219,9 @@
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);

+  // First perform a full GC in order to avoid dead objects.
+  Heap* heap = isolate->heap();
+ heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "%DebugConstructedBy");

   // Check parameters.
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, constructor, 0);
@@ -13225,31 +13230,24 @@

   // Get the number of referencing objects.
   int count;
- // First perform a full GC in order to avoid dead objects and to make the heap
-  // iterable.
-  Heap* heap = isolate->heap();
- heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "%DebugConstructedBy");
-  {
-    HeapIterator heap_iterator(heap);
-    count = DebugConstructedBy(&heap_iterator,
-                               *constructor,
-                               max_references,
-                               NULL,
-                               0);
-  }
+  HeapIterator heap_iterator(heap);
+  count = DebugConstructedBy(&heap_iterator,
+                             *constructor,
+                             max_references,
+                             NULL,
+                             0);

   // Allocate an array to hold the result.
   Handle<FixedArray> instances = isolate->factory()->NewFixedArray(count);

+  ASSERT(heap->IsHeapIterable());
   // Fill the referencing objects.
-  {
-    HeapIterator heap_iterator2(heap);
-    count = DebugConstructedBy(&heap_iterator2,
-                               *constructor,
-                               max_references,
-                               *instances,
-                               count);
-  }
+  HeapIterator heap_iterator2(heap);
+  count = DebugConstructedBy(&heap_iterator2,
+                             *constructor,
+                             max_references,
+                             *instances,
+                             count);

   // Return result as JS array.
   Handle<JSFunction> array_function(
@@ -13381,6 +13379,8 @@
   int number;
   Heap* heap = isolate->heap();
   {
+    heap->EnsureHeapIsIterable();
+    DisallowHeapAllocation no_allocation;
     HeapIterator heap_iterator(heap);
     Script* scr = *script;
     FixedArray* arr = *array;
@@ -13388,6 +13388,8 @@
   }
   if (number > kBufferSize) {
     array = isolate->factory()->NewFixedArray(number);
+    heap->EnsureHeapIsIterable();
+    DisallowHeapAllocation no_allocation;
     HeapIterator heap_iterator(heap);
     Script* scr = *script;
     FixedArray* arr = *array;
@@ -14470,6 +14472,8 @@
   Handle<Script> script;
   Factory* factory = script_name->GetIsolate()->factory();
   Heap* heap = script_name->GetHeap();
+  heap->EnsureHeapIsIterable();
+  DisallowHeapAllocation no_allocation_during_heap_iteration;
   HeapIterator iterator(heap);
   HeapObject* obj = NULL;
   while (script.is_null() && ((obj = iterator.next()) != NULL)) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue May 20 13:19:21 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-api.cc Tue May 20 14:03:38 2014 UTC
@@ -13514,6 +13514,7 @@


 static int GetGlobalObjectsCount() {
+  CcTest::heap()->EnsureHeapIsIterable();
   int count = 0;
   i::HeapIterator it(CcTest::heap());
for (i::HeapObject* object = it.next(); object != NULL; object = it.next())
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Tue May 20 13:19:21 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-heap.cc Tue May 20 14:03:38 2014 UTC
@@ -890,6 +890,7 @@
static int ObjectsFoundInHeap(Heap* heap, Handle<Object> objs[], int size) {
   // Count the number of objects found in the heap.
   int found_count = 0;
+  heap->EnsureHeapIsIterable();
   HeapIterator iterator(heap);
for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
     for (int i = 0; i < size; i++) {
@@ -1628,8 +1629,9 @@

 TEST(TestSizeOfObjectsVsHeapIteratorPrecision) {
   CcTest::InitializeVM();
-  HeapIterator iterator(CcTest::heap());
+  CcTest::heap()->EnsureHeapIsIterable();
   intptr_t size_of_objects_1 = CcTest::heap()->SizeOfObjects();
+  HeapIterator iterator(CcTest::heap());
   intptr_t size_of_objects_2 = 0;
   for (HeapObject* obj = iterator.next();
        obj != NULL;
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Tue May 20 13:19:21 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Tue May 20 14:03:38 2014 UTC
@@ -616,6 +616,7 @@


 static int GetGlobalObjectsCount() {
+  CcTest::heap()->EnsureHeapIsIterable();
   int count = 0;
   i::HeapIterator it(CcTest::heap());
for (i::HeapObject* object = it.next(); object != NULL; object = it.next())

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to