[vm/ffi] Remove hacks and instability from FFI GC tests.
This is based on https://dart-review.googlesource.com/c/sdk/+/100583/2,
but instead of relying on private symbols, we expose an API function which
is a no-op outside specific tests.
Fixes https://github.com/dart-lang/sdk/issues/36723
Change-Id: Ieb78caa2d14956a2b5f985533b9b73e941d2d9c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103801
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
diff --git a/runtime/bin/ffi_test_functions.cc b/runtime/bin/ffi_test_functions.cc
index 200d6c2..52b4512 100644
--- a/runtime/bin/ffi_test_functions.cc
+++ b/runtime/bin/ffi_test_functions.cc
@@ -22,6 +22,7 @@
#include <limits>
#include "include/dart_api.h"
+#include "include/dart_native_api.h"
namespace dart {
@@ -458,42 +459,39 @@
// Functions for stress-testing.
DART_EXPORT int64_t MinInt64() {
+ Dart_ExecuteInternalCommand("gc-on-next-allocation");
return 0x8000000000000000;
}
DART_EXPORT int64_t MinInt32() {
+ Dart_ExecuteInternalCommand("gc-on-next-allocation");
return 0x80000000;
}
DART_EXPORT double SmallDouble() {
+ Dart_ExecuteInternalCommand("gc-on-next-allocation");
return 0x80000000 * -1.0;
}
// Requires boxing on 32-bit and 64-bit systems, even if the top 32-bits are
// truncated.
DART_EXPORT void* LargePointer() {
+ Dart_ExecuteInternalCommand("gc-on-next-allocation");
uint64_t origin = 0x8100000082000000;
return reinterpret_cast<void*>(origin);
}
-// Allocates 'count'-many Mint boxes, to stress-test GC during an FFI call.
-DART_EXPORT void AllocateMints(uint64_t count) {
- Dart_EnterScope();
- for (uint64_t i = 0; i < count; ++i) {
- Dart_NewInteger(0x8000000000000001);
- }
- Dart_ExitScope();
+DART_EXPORT void TriggerGC(uint64_t count) {
+ Dart_ExecuteInternalCommand("gc-now");
}
// Calls a Dart function to allocate 'count' objects.
// Used for stress-testing GC when re-entering the API.
-DART_EXPORT void AllocateThroughDart(uint64_t count) {
+DART_EXPORT void AllocateThroughDart() {
Dart_EnterScope();
Dart_Handle root = Dart_RootLibrary();
- Dart_Handle arguments[1] = {Dart_NewIntegerFromUint64(count)};
Dart_Handle result = Dart_Invoke(
- root, Dart_NewStringFromCString("testAllocationsInDartHelper"), 1,
- arguments);
+ root, Dart_NewStringFromCString("testAllocationsInDartHelper"), 0, NULL);
const char* error;
if (Dart_IsError(result)) {
Dart_StringToCString(Dart_ToString(result), &error);
@@ -506,21 +504,6 @@
Dart_ExitScope();
}
-#if !defined(_WIN32)
-DART_EXPORT int RedirectStderr() {
- char filename[256];
- snprintf(filename, sizeof(filename), "/tmp/captured_stderr_%d", getpid());
- FILE* f = freopen(filename, "w", stderr);
- if (f == nullptr) {
- fprintf(stderr, "Could not open temp file.\n");
- Dart_PrepareToAbort();
- abort();
- }
- printf("Got file %s\n", filename);
- return getpid();
-}
-#endif
-
////////////////////////////////////////////////////////////////////////////////
// Tests for callbacks.
diff --git a/runtime/include/dart_native_api.h b/runtime/include/dart_native_api.h
index 3b8f4d2..e45c010 100644
--- a/runtime/include/dart_native_api.h
+++ b/runtime/include/dart_native_api.h
@@ -175,4 +175,10 @@
*/
DART_EXPORT DART_WARN_UNUSED_RESULT Dart_Handle Dart_FinalizeAllClasses();
+/* This function is intentionally undocumented.
+ *
+ * It should not be used outside internal tests.
+ */
+DART_EXPORT void Dart_ExecuteInternalCommand(const char* command);
+
#endif /* INCLUDE_DART_NATIVE_API_H_ */ /* NOLINT */
diff --git a/runtime/vm/heap/heap.cc b/runtime/vm/heap/heap.cc
index 2a53317..c8a1bdf 100644
--- a/runtime/vm/heap/heap.cc
+++ b/runtime/vm/heap/heap.cc
@@ -43,7 +43,8 @@
barrier_done_(),
read_only_(false),
gc_new_space_in_progress_(false),
- gc_old_space_in_progress_(false) {
+ gc_old_space_in_progress_(false),
+ gc_on_next_allocation_(false) {
UpdateGlobalMaxUsed();
for (int sel = 0; sel < kNumWeakSelectors; sel++) {
new_weak_tables_[sel] = new WeakTable();
@@ -80,6 +81,7 @@
uword Heap::AllocateNew(intptr_t size) {
ASSERT(Thread::Current()->no_safepoint_scope_depth() == 0);
+ CollectForDebugging();
Thread* thread = Thread::Current();
uword addr = new_space_.TryAllocateInTLAB(thread, size);
if (addr != 0) {
@@ -125,6 +127,7 @@
uword Heap::AllocateOld(intptr_t size, HeapPage::PageType type) {
ASSERT(Thread::Current()->no_safepoint_scope_depth() == 0);
+ CollectForDebugging();
uword addr = old_space_.TryAllocate(size, type);
if (addr != 0) {
return addr;
@@ -652,6 +655,17 @@
old_space_.AddRegionsToObjectSet(set);
}
+void Heap::CollectOnNextAllocation() {
+ AbandonRemainingTLAB(Thread::Current());
+ gc_on_next_allocation_ = true;
+}
+
+void Heap::CollectForDebugging() {
+ if (!gc_on_next_allocation_) return;
+ CollectAllGarbage(kDebugging);
+ gc_on_next_allocation_ = false;
+}
+
ObjectSet* Heap::CreateAllocatedObjectSet(
Zone* zone,
MarkExpectation mark_expectation) const {
diff --git a/runtime/vm/heap/heap.h b/runtime/vm/heap/heap.h
index 837f2c9..6bbcb19 100644
--- a/runtime/vm/heap/heap.h
+++ b/runtime/vm/heap/heap.h
@@ -302,6 +302,8 @@
void MakeTLABIterable(Thread* thread);
void AbandonRemainingTLAB(Thread* thread);
+ void CollectOnNextAllocation();
+
private:
class GCStats : public ValueObject {
public:
@@ -373,6 +375,9 @@
void AddRegionsToObjectSet(ObjectSet* set) const;
+ // Trigger major GC if 'gc_on_next_allocation_' is set.
+ void CollectForDebugging();
+
Isolate* isolate_;
// The different spaces used for allocation.
@@ -396,6 +401,11 @@
bool gc_new_space_in_progress_;
bool gc_old_space_in_progress_;
+ // Whether the next heap allocation (new or old) should trigger
+ // CollectAllGarbage. Used within unit tests for testing GC on certain
+ // sensitive codepaths.
+ bool gc_on_next_allocation_;
+
friend class Become; // VisitObjectPointers
friend class GCCompactor; // VisitObjectPointers
friend class Precompiler; // VisitObjects
diff --git a/runtime/vm/lockers.h b/runtime/vm/lockers.h
index 8cce8c3..de62b6c 100644
--- a/runtime/vm/lockers.h
+++ b/runtime/vm/lockers.h
@@ -46,7 +46,11 @@
class MutexLocker : public ValueObject {
public:
explicit MutexLocker(Mutex* mutex, bool no_safepoint_scope = true)
- : mutex_(mutex), no_safepoint_scope_(no_safepoint_scope) {
+ :
+#if defined(DEBUG)
+ no_safepoint_scope_(no_safepoint_scope),
+#endif
+ mutex_(mutex) {
ASSERT(mutex != NULL);
#if defined(DEBUG)
if (no_safepoint_scope_) {
@@ -88,8 +92,8 @@
}
private:
+ DEBUG_ONLY(bool no_safepoint_scope_;)
Mutex* const mutex_;
- bool no_safepoint_scope_;
DISALLOW_COPY_AND_ASSIGN(MutexLocker);
};
diff --git a/runtime/vm/native_api_impl.cc b/runtime/vm/native_api_impl.cc
index df1859d..c225bff 100644
--- a/runtime/vm/native_api_impl.cc
+++ b/runtime/vm/native_api_impl.cc
@@ -237,4 +237,17 @@
#endif // defined(DART_PRECOMPILED_RUNTIME)
}
+DART_EXPORT void Dart_ExecuteInternalCommand(const char* command) {
+ if (!FLAG_enable_testing_pragmas) return;
+
+ TransitionNativeToVM _(Thread::Current());
+ if (!strcmp(command, "gc-on-next-allocation")) {
+ Isolate::Current()->heap()->CollectOnNextAllocation();
+ } else if (!strcmp(command, "gc-now")) {
+ Isolate::Current()->heap()->CollectAllGarbage(Heap::kDebugging);
+ } else {
+ UNREACHABLE();
+ }
+}
+
} // namespace dart
diff --git a/tests/ffi/function_stress_test.dart b/tests/ffi/function_gc_test.dart
similarity index 61%
rename from tests/ffi/function_stress_test.dart
rename to tests/ffi/function_gc_test.dart
index 92e807c..1799ce1 100644
--- a/tests/ffi/function_stress_test.dart
+++ b/tests/ffi/function_gc_test.dart
@@ -2,8 +2,8 @@
// 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.
//
-// VMOptions=--deterministic --optimization-counter-threshold=500 --verbose-gc
-// VMOptions=--deterministic --optimization-counter-threshold=-1 --verbose-gc
+// VMOptions=--deterministic --optimization-counter-threshold=500 --enable-testing-pragmas
+// VMOptions=--deterministic --optimization-counter-threshold=-1 --enable-testing-pragmas
//
// Dart test program for stress-testing boxing and GC in return paths from FFI
// trampolines.
@@ -16,45 +16,14 @@
import 'dart:ffi' as ffi;
import 'dylib_utils.dart';
import "package:expect/expect.dart";
-import 'gc_helper.dart';
-
-test(GCWatcher watcher, Function testee,
- {bool mustTriggerGC: true, bool batched: false}) async {
- // Warmup.
- for (int i = 0; i < 1000; ++i) {
- batched ? testee(1) : testee();
- }
- int size = await watcher.size();
- for (int i = 0; i < 1000000;) {
- if (batched) {
- testee(1000);
- i += 1000;
- } else {
- testee();
- i++;
- }
- }
- int new_size = await watcher.size();
- if (mustTriggerGC) {
- print("Expect $new_size > $size.");
- Expect.isTrue(new_size > size);
- }
-}
main() async {
- final watcher = GCWatcher.ifAvailable();
- try {
- await test(watcher, testBoxInt64);
- // On 64-bit platforms this won't trigger GC because the result fits into a
- // Smi.
- await test(watcher, testBoxInt32, mustTriggerGC: false);
- await test(watcher, testBoxDouble);
- await test(watcher, testBoxPointer);
- await test(watcher, testAllocateMints, batched: true);
- await test(watcher, testAllocationsInDart, batched: true);
- } finally {
- watcher.dispose();
- }
+ testBoxInt64();
+ testBoxInt32();
+ testBoxDouble();
+ testBoxPointer();
+ testAllocateInNative();
+ testAllocateInDart();
}
ffi.DynamicLibrary ffiTestFunctions =
@@ -64,11 +33,13 @@
typedef NativeNullaryOp32 = ffi.Int32 Function();
typedef NativeNullaryOpDouble = ffi.Double Function();
typedef NativeNullaryOpPtr = ffi.Pointer<ffi.Void> Function();
+typedef NativeNullaryOp = ffi.Void Function();
typedef NativeUnaryOp = ffi.Void Function(ffi.Uint64);
typedef NullaryOp = int Function();
typedef NullaryOpDbl = double Function();
typedef NullaryOpPtr = ffi.Pointer<ffi.Void> Function();
typedef UnaryOp = void Function(int);
+typedef NullaryOpVoid = void Function();
//// These functions return values that require boxing into different types.
@@ -111,33 +82,19 @@
}
}
-final allocateMint =
- ffiTestFunctions.lookupFunction<NativeUnaryOp, UnaryOp>("AllocateMints");
+final triggerGc = ffiTestFunctions
+ .lookupFunction<NativeNullaryOp, NullaryOpVoid>("TriggerGC");
-// Test GC in the FFI call path by calling a C function which allocates through
-// the Dart API.
-void testAllocateMints(int batchSize) {
- allocateMint(batchSize);
-}
+// Test GC in the FFI call path by calling a C function which triggers GC
+// directly.
+void testAllocateInNative() => triggerGc();
-class C {
- final int i;
- C(this.i);
-}
-
-C c = null;
@pragma("vm:entry-point", "call")
-void testAllocationsInDartHelper(int count) {
- for (int i = 0; i < count; ++i) {
- c = C(i);
- }
-}
+void testAllocationsInDartHelper() => triggerGc();
final allocateThroughDart = ffiTestFunctions
- .lookupFunction<NativeUnaryOp, UnaryOp>("AllocateThroughDart");
+ .lookupFunction<NativeNullaryOp, NullaryOpVoid>("AllocateThroughDart");
// Test GC in the FFI call path by calling a C function which allocates by
// calling back into Dart ('testAllocationsInDartHelper').
-void testAllocationsInDart(int batchSize) {
- allocateThroughDart(batchSize * 10);
-}
+void testAllocateInDart() => allocateThroughDart();
diff --git a/tests/ffi/gc_helper.dart b/tests/ffi/gc_helper.dart
deleted file mode 100644
index abe0454..0000000
--- a/tests/ffi/gc_helper.dart
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright (c) 2019, 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.
-
-import 'dart:io';
-import 'dylib_utils.dart';
-import 'dart:ffi';
-import 'dart:io' show Platform;
-
-DynamicLibrary ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions");
-
-const bool isProduct = const bool.fromEnvironment("dart.vm.product");
-
-abstract class GCWatcher {
- factory GCWatcher() => _GCWatcherImpl();
- factory GCWatcher.dummy() => _MockGCWatcher();
- factory GCWatcher.ifAvailable() =>
- (Platform.isWindows || Platform.isAndroid || isProduct)
- ? GCWatcher.dummy()
- : GCWatcher();
-
- Future<int> size();
- void dispose();
-}
-
-// Requires --verbose-gc.
-class _GCWatcherImpl implements GCWatcher {
- int _suffix;
-
- Future<int> size() async {
- return await File("/tmp/captured_stderr_$_suffix").length();
- }
-
- _GCWatcherImpl() {
- print("Starting...");
- _suffix = ffiTestFunctions
- .lookupFunction<Int32 Function(), int Function()>("RedirectStderr")();
- }
-
- dispose() {
- try {
- File("/tmp/captured_stderr_$_suffix").deleteSync();
- } catch (e) {
- print("deleting file failed");
- }
- }
-}
-
-class _MockGCWatcher implements GCWatcher {
- int _ctr = 0;
-
- Future<int> size() async => ++_ctr;
- dispose() {}
-}
diff --git a/tests/ffi/subtype_test.dart b/tests/ffi/subtype_test.dart
index 22dbe13..54977b5 100644
--- a/tests/ffi/subtype_test.dart
+++ b/tests/ffi/subtype_test.dart
@@ -4,6 +4,7 @@
//
// Dart test program for testing dart:ffi Pointer subtypes.
//
+// SharedObjects=ffi_test_functions
// VMOptions=--verbose-gc
library FfiTest;
@@ -12,21 +13,21 @@
import "package:expect/expect.dart";
-import 'gc_helper.dart';
import 'cstring.dart';
+import 'dylib_utils.dart';
+
+ffi.DynamicLibrary ffiTestFunctions =
+ dlopenPlatformSpecific("ffi_test_functions");
+
+final triggerGc = ffiTestFunctions
+ .lookupFunction<ffi.Void Function(), void Function()>("TriggerGC");
void main() async {
testAllocate();
testSizeOf();
- await testGC();
+ testGC();
}
-class X {
- X(this.i);
- int i;
-}
-
-dynamic foo;
dynamic bar;
void testAllocate() {
@@ -36,15 +37,10 @@
}
Future<void> testGC() async {
- CString cs = ffi.fromAddress<CString>(11);
- bar = cs;
- foo = "";
- final watcher = GCWatcher.ifAvailable();
- int counts = await watcher.size();
- for (int i = 0; i < 1000000; ++i) {
- foo = new X(i);
- }
- Expect.isTrue(await watcher.size() > counts);
+ bar = ffi.fromAddress<CString>(11);
+ // Verify that the objects manufactured by 'fromAddress' can be scanned by the
+ // GC.
+ triggerGc();
}
void testSizeOf() {
