Simpler, more consistent triggering of old GC; also, use external in policy.

runtime/vm/dart_api_impl_test.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 // TODO(zra): Remove when tests are ready to enable.
 #include "platform/globals.h"
 #if !defined(TARGET_ARCH_ARM64)
 
 #include "bin/builtin.h"
 #include "include/dart_api.h"
@@ skipping 2293 matching lines @@
   // Promoting the string should transfer the external size to old.
   GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
   GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
   EXPECT(heap->ExternalInWords(Heap::kNew) == 0);
   EXPECT(heap->ExternalInWords(Heap::kOld) == kWeakExternalSize / kWordSize);
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   EXPECT(heap->ExternalInWords(Heap::kOld) == 0);
 }
 
 
-TEST_CASE(WeakPersistentHandleExternalAllocationSizeOversized) {
+TEST_CASE(WeakPersistentHandleExternalAllocationSizeNewspaceGC) {
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Heap* heap = Isolate::Current()->heap();
   Dart_WeakPersistentHandle weak1 = NULL;
   // Large enough to exceed any new space limit. Not actually allocated.
   const intptr_t kWeak1ExternalSize = 500 * MB;
   {
     Dart_EnterScope();
     Dart_Handle obj = NewString("weakly referenced string");
     EXPECT_VALID(obj);
     // Triggers a scavenge immediately, since kWeak1ExternalSize is above limit.
@@ skipping 20 matching lines @@
     EXPECT(heap->ExternalInWords(Heap::kNew) == 0);
     EXPECT(heap->ExternalInWords(Heap::kOld) == kWeak1ExternalSize / kWordSize);
     Dart_ExitScope();
   }
   Dart_DeleteWeakPersistentHandle(isolate, weak1);
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   EXPECT(heap->ExternalInWords(Heap::kOld) == 0);
 }
 
 
+TEST_CASE(WeakPersistentHandleExternalAllocationSizeOldspaceGC) {
+  // Check that external allocation in old space can trigger GC.
+  Isolate* isolate = Isolate::Current();
+  Dart_EnterScope();
+  Dart_Handle live = Api::NewHandle(isolate, String::New("live", Heap::kOld));
+  EXPECT_VALID(live);
+  Dart_WeakPersistentHandle weak = NULL;
+  const intptr_t kSmallExternalSize = 1 * KB;
+  {
+    Dart_EnterScope();
+    Dart_Handle dead = Api::NewHandle(isolate, String::New("dead", Heap::kOld));
+    EXPECT_VALID(dead);
+    weak = Dart_NewWeakPersistentHandle(dead,
+                                        NULL,
+                                        kSmallExternalSize,
+                                        NopCallback);
+    EXPECT_VALID(AsHandle(weak));
+    Dart_ExitScope();
+  }
+  EXPECT_EQ(kSmallExternalSize,

[Ivan Posva, 2014/04/18 16:11:26]

You might want to verify how much external size there was before starting the
allocation.

[koda, 2014/04/18 16:38:18]

Done.

+            isolate->heap()->ExternalInWords(Heap::kOld) * kWordSize);
+  // Large enough to trigger GC in old space. Not actually allocated.
+  const intptr_t kHugeExternalSize = 1000 * MB;
+  Dart_NewWeakPersistentHandle(live,
+                               NULL,
+                               kHugeExternalSize,
+                               NopCallback);
+  // Expect small garbage to be collected.
+  EXPECT_EQ(kHugeExternalSize,
+            isolate->heap()->ExternalInWords(Heap::kOld) * kWordSize);
+  Dart_DeleteWeakPersistentHandle(reinterpret_cast<Dart_Isolate>(isolate),
+                                  weak);
+  Dart_ExitScope();
+}
+
+
 TEST_CASE(WeakPersistentHandleExternalAllocationSizeOddReferents) {
   Heap* heap = Isolate::Current()->heap();
   Dart_WeakPersistentHandle weak1 = NULL;
   static const intptr_t kWeak1ExternalSize = 1 * KB;
   Dart_WeakPersistentHandle weak2 = NULL;
   static const intptr_t kWeak2ExternalSize = 2 * KB;
   {
     Dart_EnterScope();
-    Dart_Handle dart_null = Dart_Null();  // VM heap object.
+    Dart_Handle dart_null = Dart_True();  // VM heap object.

[Ivan Posva, 2014/04/18 16:11:26]

Why?

[koda, 2014/04/18 16:38:18]

I assume you mean "Why the change from 'null' to 'true'?"

I learned that 'null' is (counter-intuitively) considered always unreachable
during GC. Therefore, using it as a referent leads to strange behavior. True
just serves as an example of a VM heap object. Although VM heap object referents
are unlikely in practice, they constitute a corner case that should be tested
(or explicitly forbidden).

I now also renamed this local variable to the more suitable 'dart_true'.

     EXPECT_VALID(dart_null);
     weak1 = Dart_NewWeakPersistentHandle(
         dart_null, NULL, kWeak1ExternalSize, NopCallback);
     EXPECT_VALID(AsHandle(weak1));
     Dart_Handle zero = Dart_NewInteger(0);  // Smi.
     EXPECT_VALID(dart_null);
     weak2 = Dart_NewWeakPersistentHandle(
         zero, NULL, kWeak2ExternalSize, NopCallback);
     EXPECT_VALID(AsHandle(weak2));
     // Both should be charged to old space.
     EXPECT(heap->ExternalInWords(Heap::kOld) ==
            (kWeak1ExternalSize + kWeak2ExternalSize) / kWordSize);
     Dart_ExitScope();
   }
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Dart_DeleteWeakPersistentHandle(isolate, weak1);
   Dart_DeleteWeakPersistentHandle(isolate, weak2);
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-  EXPECT(heap->ExternalInWords(Heap::kOld) == 0);
+  EXPECT_EQ(0, heap->ExternalInWords(Heap::kOld));
 }
 
 
 static Dart_WeakPersistentHandle weak1 = NULL;
 static Dart_WeakPersistentHandle weak2 = NULL;
 static Dart_WeakPersistentHandle weak3 = NULL;
 static Dart_WeakPersistentHandle weak4 = NULL;
 
 
 static void ObjectGroupsCallback(void* isolate_callback_data,
@@ skipping 292 matching lines @@
   {
     Dart_EnterScope();
     // Both prologue weak handles should be preserved.
     EXPECT(!Dart_IsNull(AsHandle(new_pwph)));
     EXPECT(!Dart_IsNull(AsHandle(old_pwph)));
     Dart_ExitScope();
   }
 
   // Garbage collect old space without invoking API callbacks.
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld,
-                                             Heap::kIgnoreApiCallbacks);
+                                             Heap::kIgnoreApiCallbacks,
+                                             Heap::kGCTestCase);
 
   {
     Dart_EnterScope();
     // Both prologue weak handles should be preserved.
     EXPECT(!Dart_IsNull(AsHandle(new_pwph)));
     EXPECT(!Dart_IsNull(AsHandle(old_pwph)));
     Dart_ExitScope();
   }
 
   // Garbage collect new space invoking API callbacks.
   GCTestHelper::CollectNewSpace(Heap::kInvokeApiCallbacks);
 
   {
     Dart_EnterScope();
     // The prologue weak handle with a new space referent should now be
     // cleared.  The old space referent should be preserved.
     EXPECT(new_pwph == NULL);
     EXPECT(!Dart_IsNull(AsHandle(old_pwph)));
     Dart_ExitScope();
   }
 
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld,
-                                             Heap::kInvokeApiCallbacks);
+                                             Heap::kInvokeApiCallbacks,
+                                             Heap::kGCTestCase);
 
   {
     Dart_EnterScope();
     // The prologue weak handle with an old space referent should now be
     // cleared.  The new space referent should remain cleared.
     EXPECT(new_pwph == NULL);
     EXPECT(old_pwph == NULL);
     Dart_ExitScope();
   }
 }
@@ skipping 286 matching lines @@
   global_epilogue_callback_status = 7;
   GCTestHelper::CollectNewSpace(Heap::kInvokeApiCallbacks);
   EXPECT_EQ(6, global_prologue_callback_status);
   EXPECT_EQ(7, global_epilogue_callback_status);
 
   // Garbage collect old space ignoring callbacks.  This should invoke
   // the prologue callback.  The prologue status value should change.
   global_prologue_callback_status = 3;
   global_epilogue_callback_status = 7;
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld,
-                                             Heap::kIgnoreApiCallbacks);
+                                             Heap::kIgnoreApiCallbacks,
+                                             Heap::kGCTestCase);
   EXPECT_EQ(3, global_prologue_callback_status);
   EXPECT_EQ(7, global_epilogue_callback_status);
 
   // Garbage collect old space.  This should invoke the prologue
   // callback.  The prologue status value should change.
   global_prologue_callback_status = 3;
   global_epilogue_callback_status = 7;
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   EXPECT_EQ(6, global_prologue_callback_status);
   EXPECT_EQ(7, global_epilogue_callback_status);
@@ skipping 29 matching lines @@
   // should change.
   global_prologue_callback_status = 3;
   global_epilogue_callback_status = 7;
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   EXPECT_EQ(6, global_prologue_callback_status);
   EXPECT_EQ(28, global_epilogue_callback_status);
 
   // Garbage collect old space again without invoking callbacks.
   // Nothing should change.
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld,
-                                             Heap::kIgnoreApiCallbacks);
+                                             Heap::kIgnoreApiCallbacks,
+                                             Heap::kGCTestCase);
   EXPECT_EQ(6, global_prologue_callback_status);
   EXPECT_EQ(28, global_epilogue_callback_status);
 
   // Garbage collect old space again.  Callbacks are persistent so the
   // prologue and epilogue status values should change again.
   Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   EXPECT_EQ(12, global_prologue_callback_status);
   EXPECT_EQ(112, global_epilogue_callback_status);
 
   // Remove the prologue and epilogue callbacks
@@ skipping 5010 matching lines @@
                                    dart_args);
   int64_t value = 0;
   result = Dart_IntegerToInt64(result, &value);
   EXPECT_VALID(result);
   EXPECT_EQ(6, value);
 }
 
 }  // namespace dart
 
 #endif  // !defined(TARGET_ARCH_ARM64)