Skip to content

Commit eb69a47

Browse files
committed
[PerformanceTests] Really Fix ObjectArrayEnumerationTiming failure
Commit 67267da attempted to fix the JavaArrayTiming.ObjectArrayEnumerationTiming test by using GC.Collect() to ensure that nothing was collected during enumeration. This fix was DOOMED to failure, which is why it promptly failed again. :-) Instructive is [*why*][0] it was DOOMED. TL;DR: Finalizers *suck*, and a "fully managed" cross-VM bridge implementation likely isn't possible. The setup: modify JavaObject to contain a ZombieLevel field, and update the finalizer to update it: partial class JavaObject { public int ZombieLevel; ~JavaObject () { ZombieLevel++; // ...as before... } } Then take the `foreach` loop in ObjectArrayEnumerationTiming, and annotate: foreach (var p in ps) { var z = p.ZombieLevel; var h = p.PeerReference; Console.WriteLine ("# Original Zombie={0}; Current={1}", z, p.ZombieLevel); using (var pt = new JniType (ref h, JniObjectReferenceOptions.Copy)) {} } The result is that *sometimes* the ZombieLevel's differ, so here's what's happening: JavaObject has a finalizer, so: 1. JavaObject instance is constructed 2. Instance from (1) is eventually eligible for collection (no roots) and placed on the finalization queue. 3. JavaObject finalizer is run for (1), which sees that the corresponding Java instance is still alive, calls GC.ReRegisterForFinalize(). 4. Instance from (1), *while still alive*, is *also* still on the finalization queue! 5. The GC thread continues to run the finalizer, ~constantly, until the instance stops re-registering itself. (3), (4), and (5) are the killers here: while the GC is (usually) a stop-the-world collector, finalizer *execution* does not require that the world be stopped, and are thus executed "normally" by the finalizer thread, *in parallel* with normal code execution! This can introduce significant failures: `p` is thus *shared* between multiple threads -- the thread running the test, and the finalizer thread, attempting to finalize `p` -- and finalizer execution will *change* p.PeerReference, *invalidating* other copies like `h`! This is Horrifically Bad™. There are only two fixes here: 1. Don't use finalizers, or 2. Don't use finalizers by themselves. Which brings us to a question: Why's this work in Xamarin.Android? Java.Lang.Object has a finalizer, why doesn't this break ALL THE TIME? The answer is taht Xamarin.Android follows (2): it uses Mono's embedding API to use a custom GC bridge via `mono_gc_register_bridge_callbacks()`, and this GC bridge ensures that the Java.Lang.Object finalizer WILL NOT EXECUTE until the corresponding Java instance is also collected. Which means Xamarin.Android doesn't experience the scenario of a Java.Lang.Object instance on the finalization queue constantly having its finalizer executed concurrently with "normal" code, so things work (more or less) as expected. The fix here? Skip JavaObject entirely here, and stick with the lower-level JniObjectReference API. No objects, no finalizers, no problem. The *proper* long-term fix? Java.Interop with Xamarin.Android will use Xamarin.Android's GC bridge. Java.Interop with Desktop Mono is another matter; we either find a way to remove the JavaObject finalizer (yet somehow make it optional on Xamarin.Android?), or we copy Xamarin.Android'sx GC bridge. Copying the Xamarin.Android GC bridge works, but means we require a mono runtime environment; things won't work -- cannot work? -- on desktop .NET. Which raises a closesly related question: is it even *possible* to have a fully-managed VM+GC bridge implementation that doesn't involve using Mono's GC bridge API, something portable to e.g. CoreCLR? At present, that doesn't look to be possible. [0]: https://xamarinhq.slack.com/archives/D03DZPU5V/p1448469633000024
1 parent bb2068b commit eb69a47

File tree

1 file changed

+12
-14
lines changed

1 file changed

+12
-14
lines changed

tests/PerformanceTests/TimingTests.cs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,8 @@ public void ObjectArrayEnumerationTiming ()
537537
// throws an exception because `h` is NULL, when it really can't be.
538538
// I believe that this is due to the finalizer, which likewise makes
539539
// NO SENSE AT ALL, since `p` should be keeping the handle valid!
540-
GC.Collect ();
541-
GC.WaitForPendingFinalizers ();
540+
// GC.Collect ();
541+
// GC.WaitForPendingFinalizers ();
542542

543543
foreach (var method in methodHandles) {
544544
var lookupTiming = Stopwatch.StartNew ();
@@ -550,26 +550,24 @@ public void ObjectArrayEnumerationTiming ()
550550
var parameterTiming = Stopwatch.StartNew ();
551551
var enumTime = new TimeSpan ();
552552
var lrefPs = JniEnvironment.InstanceMethods.CallObjectMethod (method.PeerReference, Method_getParameterTypes);
553-
Stopwatch cleanup;
554-
using (var ps = new JavaObjectArray<JavaObject>(ref lrefPs, JniObjectReferenceOptions.CopyAndDispose)) {
555-
var enumSw = Stopwatch.StartNew ();
556-
foreach (var p in ps) {
557-
var h = p.PeerReference;
558-
using (var pt = new JniType (ref h, JniObjectReferenceOptions.Copy)) {
559-
}
553+
int len = JniEnvironment.Arrays.GetArrayLength (lrefPs);
554+
var enumSw = Stopwatch.StartNew ();
555+
for (int i = 0; i < len; ++i) {
556+
var p = JniEnvironment.Arrays.GetObjectArrayElement (lrefPs, i);
557+
using (var pt = new JniType (ref p, JniObjectReferenceOptions.Copy)) {
560558
}
561-
enumSw.Stop ();
562-
enumTime = enumSw.Elapsed;
563-
cleanup = Stopwatch.StartNew ();
559+
JniObjectReference.Dispose (ref p);
564560
}
565-
cleanup.Stop ();
561+
JniObjectReference.Dispose (ref lrefPs);
562+
enumSw.Stop ();
563+
enumTime = enumSw.Elapsed;
566564
parameterTiming.Stop ();
567565

568566
Console.WriteLine ("## method '{0}' timing: Total={1}; Parameters={2} Parameters.Dispose={3}",
569567
name,
570568
lookupTiming.Elapsed,
571569
enumTime,
572-
cleanup.Elapsed);
570+
parameterTiming.Elapsed);
573571
}
574572

575573
var mhDisposeTiming = Stopwatch.StartNew ();

0 commit comments

Comments
 (0)