Reviewers: Hannes Payer,

Description:
Remove Heap::IsHeapIterable.

Heap::IsHeapIterable does not (and cannot) take into account new space
fragmentation caused by allocation folding, so it does not really
determine iterability. This change list simply removes the method
(and forces GC whenever we need iterable heap).

[email protected]
BUG=373283
LOG=N

Please review this at https://codereview.chromium.org/285693006/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+29, -27 lines):
  M src/heap.h
  M src/heap.cc
  M src/runtime.cc
  A test/mjsunit/regress/regress-373283.js


Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index f0c9154b5e9aa15cb45f00df5b5e43533b966979..8119eaf61395f93abfeec7c92dabff320b7eec28 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -4307,18 +4307,9 @@ STRUCT_LIST(MAKE_CASE)
 }


-bool Heap::IsHeapIterable() {
-  return (!old_pointer_space()->was_swept_conservatively() &&
-          !old_data_space()->was_swept_conservatively());
-}
-
-
 void Heap::EnsureHeapIsIterable() {
   ASSERT(AllowHeapAllocation::IsAllowed());
-  if (!IsHeapIterable()) {
-    CollectAllGarbage(kMakeHeapIterableMask, "Heap::EnsureHeapIsIterable");
-  }
-  ASSERT(IsHeapIterable());
+  CollectAllGarbage(kMakeHeapIterableMask, "Heap::EnsureHeapIsIterable");
 }


Index: src/heap.h
diff --git a/src/heap.h b/src/heap.h
index 8e3cd3f2b05724860dea12d18b3a1329701b210d..5267f9515aeabe2c0e9a2f6eb5d3f8456f5106b8 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -762,9 +762,6 @@ class Heap {
   // Last hope GC, should try to squeeze as much as possible.
   void CollectAllAvailableGarbage(const char* gc_reason = NULL);

-  // Check whether the heap is currently iterable.
-  bool IsHeapIterable();
-
   // Ensure that we have swept all spaces in such a way that we can iterate
   // over all objects.  May cause a GC.
   void EnsureHeapIsIterable();
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index a63fd65d4bc6b2298a08bcdad7a62e160867ede8..6e50c980294c73f70f81cea18184a34e13a37332 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -13126,14 +13126,6 @@ RUNTIME_FUNCTION(Runtime_DebugReferencedBy) {
   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);
@@ -13151,6 +13143,10 @@ RUNTIME_FUNCTION(Runtime_DebugReferencedBy) {

   // 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,
@@ -13160,8 +13156,7 @@ RUNTIME_FUNCTION(Runtime_DebugReferencedBy) {
   Handle<FixedArray> instances = isolate->factory()->NewFixedArray(count);

   // Fill the referencing objects.
-  // AllocateFixedArray above does not make the heap non-iterable.
-  ASSERT(heap->IsHeapIterable());
+  heap->EnsureHeapIsIterable();
   HeapIterator heap_iterator2(heap);
   count = DebugReferencedBy(&heap_iterator2,
                             *target, *instance_filter, max_references,
@@ -13216,9 +13211,6 @@ RUNTIME_FUNCTION(Runtime_DebugConstructedBy) {
   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);
@@ -13227,6 +13219,10 @@ RUNTIME_FUNCTION(Runtime_DebugConstructedBy) {

   // 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,
@@ -13237,8 +13233,8 @@ RUNTIME_FUNCTION(Runtime_DebugConstructedBy) {
   // Allocate an array to hold the result.
   Handle<FixedArray> instances = isolate->factory()->NewFixedArray(count);

-  ASSERT(heap->IsHeapIterable());
   // Fill the referencing objects.
+  heap->EnsureHeapIsIterable();
   HeapIterator heap_iterator2(heap);
   count = DebugConstructedBy(&heap_iterator2,
                              *constructor,
Index: test/mjsunit/regress/regress-373283.js
diff --git a/test/mjsunit/regress/regress-373283.js b/test/mjsunit/regress/regress-373283.js
new file mode 100644
index 0000000000000000000000000000000000000000..20cee4d808ed681786503ea9e2c5f876daadd63d
--- /dev/null
+++ b/test/mjsunit/regress/regress-373283.js
@@ -0,0 +1,18 @@
+// 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");


--
--
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