diff --git a/src/mono/mono/metadata/sgen-bridge.c b/src/mono/mono/metadata/sgen-bridge.c index 579fc0d376cd6a..1f7dc31c9b4b11 100644 --- a/src/mono/mono/metadata/sgen-bridge.c +++ b/src/mono/mono/metadata/sgen-bridge.c @@ -316,24 +316,24 @@ dump_processor_state (SgenBridgeProcessor *p) { int i; - printf ("------\n"); - printf ("SCCS %d\n", p->num_sccs); + g_message ("------\n"); + g_message ("SCCS %d\n", p->num_sccs); for (i = 0; i < p->num_sccs; ++i) { int j; MonoGCBridgeSCC *scc = p->api_sccs [i]; - printf ("\tSCC %d:", i); + g_message ("\tSCC %d:", i); for (j = 0; j < scc->num_objs; ++j) { MonoObject *obj = scc->objs [j]; - printf (" %p(%s)", obj, SGEN_LOAD_VTABLE (obj)->klass->name); + g_message (" %p(%s)", obj, m_class_get_name (SGEN_LOAD_VTABLE (obj)->klass)); } - printf ("\n"); + g_message ("\n"); } - printf ("XREFS %d\n", p->num_xrefs); + g_message ("XREFS %d\n", p->num_xrefs); for (i = 0; i < p->num_xrefs; ++i) - printf ("\t%d -> %d\n", p->api_xrefs [i].src_scc_index, p->api_xrefs [i].dst_scc_index); + g_message ("\t%d -> %d\n", p->api_xrefs [i].src_scc_index, p->api_xrefs [i].dst_scc_index); - printf ("-------\n"); + g_message ("-------\n"); } */ @@ -352,7 +352,7 @@ sgen_compare_bridge_processor_results (SgenBridgeProcessor *a, SgenBridgeProcess if (a->num_sccs != b->num_sccs) g_error ("SCCS count expected %d but got %d", a->num_sccs, b->num_sccs); if (a->num_xrefs != b->num_xrefs) - g_error ("SCCS count expected %d but got %d", a->num_xrefs, b->num_xrefs); + g_error ("XREFS count expected %d but got %d", a->num_xrefs, b->num_xrefs); /* * First we build a hash of each object in `a` to its respective SCC index within diff --git a/src/mono/mono/metadata/sgen-tarjan-bridge.c b/src/mono/mono/metadata/sgen-tarjan-bridge.c index b0c9cf1f83baef..7a8ad5961d98e7 100644 --- a/src/mono/mono/metadata/sgen-tarjan-bridge.c +++ b/src/mono/mono/metadata/sgen-tarjan-bridge.c @@ -400,16 +400,7 @@ static const char* safe_name_bridge (GCObject *obj) { GCVTable vt = SGEN_LOAD_VTABLE (obj); - return vt->klass->name; -} - -static ScanData* -find_or_create_data (GCObject *obj) -{ - ScanData *entry = find_data (obj); - if (!entry) - entry = create_data (obj); - return entry; + return m_class_get_name (vt->klass); } #endif @@ -566,10 +557,15 @@ find_in_cache (int *insert_index) // Populate other_colors for a give color (other_colors represent the xrefs for this color) static void -add_other_colors (ColorData *color, DynPtrArray *other_colors) +add_other_colors (ColorData *color, DynPtrArray *other_colors, gboolean check_visited) { for (int i = 0; i < dyn_array_ptr_size (other_colors); ++i) { ColorData *points_to = (ColorData *)dyn_array_ptr_get (other_colors, i); + if (check_visited) { + if (points_to->visited) + continue; + points_to->visited = TRUE; + } dyn_array_ptr_add (&color->other_colors, points_to); // Inform targets points_to->incoming_colors = MIN (points_to->incoming_colors + 1, INCOMING_COLORS_MAX); @@ -593,7 +589,7 @@ new_color (gboolean has_bridges) cd = alloc_color_data (); cd->api_index = -1; - add_other_colors (cd, &color_merge_array); + add_other_colors (cd, &color_merge_array, FALSE); /* if cacheSlot >= 0, it means we prepared a given slot to receive the new color */ if (cacheSlot >= 0) @@ -700,11 +696,11 @@ compute_low_index (ScanData *data, GCObject *obj) obj = bridge_object_forward (obj); other = find_data (obj); -#if DUMP_GRAPH - printf ("\tcompute low %p ->%p (%s) %p (%d / %d, color %p)\n", data->obj, obj, safe_name_bridge (obj), other, other ? other->index : -2, other ? other->low_index : -2, other->color); -#endif if (!other) return; +#if DUMP_GRAPH + printf ("\tcompute low %p ->%p (%s) %p (%d / %d, color %p)\n", data->obj, obj, safe_name_bridge (obj), other, other ? other->index : -2, other->low_index, other->color); +#endif g_assert (other->state != INITIAL); @@ -777,10 +773,16 @@ create_scc (ScanData *data) gboolean found = FALSE; gboolean found_bridge = FALSE; ColorData *color_data = NULL; + gboolean can_reduce_color = TRUE; for (i = dyn_array_ptr_size (&loop_stack) - 1; i >= 0; --i) { ScanData *other = (ScanData *)dyn_array_ptr_get (&loop_stack, i); found_bridge |= other->is_bridge; + if (dyn_array_ptr_size (&other->xrefs) > 0 || found_bridge) { + // This scc will have more xrefs than the ones from the color_merge_array, + // we will need to create a new color to store this information. + can_reduce_color = FALSE; + } if (found_bridge || other == data) break; } @@ -788,13 +790,15 @@ create_scc (ScanData *data) if (found_bridge) { color_data = new_color (TRUE); ++num_colors_with_bridges; - } else { + } else if (can_reduce_color) { color_data = reduce_color (); + } else { + color_data = new_color (FALSE); } #if DUMP_GRAPH printf ("|SCC %p rooted in %s (%p) has bridge %d\n", color_data, safe_name_bridge (data->obj), data->obj, found_bridge); printf ("\tloop stack: "); - for (int i = 0; i < dyn_array_ptr_size (&loop_stack); ++i) { + for (i = 0; i < dyn_array_ptr_size (&loop_stack); ++i) { ScanData *other = dyn_array_ptr_get (&loop_stack, i); printf ("(%d/%d)", other->index, other->low_index); } @@ -824,10 +828,19 @@ create_scc (ScanData *data) dyn_array_ptr_add (&color_data->bridges, other->obj); } - // Maybe we should make sure we are not adding duplicates here. It is not really a problem - // since we will get rid of duplicates before submitting the SCCs to the client in gather_xrefs - if (color_data) - add_other_colors (color_data, &other->xrefs); + if (dyn_array_ptr_size (&other->xrefs) > 0) { + g_assert (color_data != NULL); + g_assert (can_reduce_color == FALSE); + // We need to eliminate duplicates early otherwise the heaviness property + // can change in gather_xrefs and it breaks down the loop that reports the + // xrefs to the client. + // + // We reuse the visited flag to mark the objects that are already part of + // the color_data array. The array was created above with the new_color call + // and xrefs were populated from color_merge_array, which is already + // deduplicated and every entry is marked as visited. + add_other_colors (color_data, &other->xrefs, TRUE); + } dyn_array_ptr_uninit (&other->xrefs); if (other == data) { @@ -837,11 +850,22 @@ create_scc (ScanData *data) } g_assert (found); + // Clear the visited flag on nodes that were added with add_other_colors in the loop above + if (!can_reduce_color) { + for (i = dyn_array_ptr_size (&color_merge_array); i < dyn_array_ptr_size (&color_data->other_colors); i++) { + ColorData *cd = (ColorData *)dyn_array_ptr_get (&color_data->other_colors, i); + g_assert (cd->visited); + cd->visited = FALSE; + } + } + #if DUMP_GRAPH - printf ("\tpoints-to-colors: "); - for (int i = 0; i < dyn_array_ptr_size (&color_data->other_colors); i++) - printf ("%p ", dyn_array_ptr_get (&color_data->other_colors, i)); - printf ("\n"); + if (color_data) { + printf ("\tpoints-to-colors: "); + for (i = 0; i < dyn_array_ptr_size (&color_data->other_colors); i++) + printf ("%p ", dyn_array_ptr_get (&color_data->other_colors, i)); + printf ("\n"); + } #endif } @@ -966,8 +990,11 @@ dump_color_table (const char *why, gboolean do_index) printf (" bridges: "); for (j = 0; j < dyn_array_ptr_size (&cd->bridges); ++j) { GCObject *obj = dyn_array_ptr_get (&cd->bridges, j); - ScanData *data = find_or_create_data (obj); - printf ("%d ", data->index); + ScanData *data = find_data (obj); + if (!data) + printf ("%p ", obj); + else + printf ("%p(%d) ", obj, data->index); } } printf ("\n"); @@ -1027,7 +1054,7 @@ processing_stw_step (void) #if defined (DUMP_GRAPH) printf ("----summary----\n"); printf ("bridges:\n"); - for (int i = 0; i < bridge_count; ++i) { + for (i = 0; i < bridge_count; ++i) { ScanData *sd = find_data (dyn_array_ptr_get (®istered_bridges, i)); printf ("\t%s (%p) index %d color %p\n", safe_name_bridge (sd->obj), sd->obj, sd->index, sd->color); } @@ -1141,6 +1168,7 @@ processing_build_callback_data (int generation) gather_xrefs (cd); reset_xrefs (cd); dyn_array_ptr_set_all (&cd->other_colors, &color_merge_array); + g_assert (color_visible_to_client (cd)); xref_count += dyn_array_ptr_size (&cd->other_colors); } } diff --git a/src/tests/GC/Features/Bridge/Bridge.cs b/src/tests/GC/Features/Bridge/Bridge.cs new file mode 100644 index 00000000000000..c481087943efcd --- /dev/null +++ b/src/tests/GC/Features/Bridge/Bridge.cs @@ -0,0 +1,422 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Threading; + +// False pinning cases are still possible. For example the thread can die +// and its stack reused by another thread. It also seems that a thread that +// does a GC can keep on the stack references to objects it encountered +// during the collection which are never released afterwards. This would +// be more likely to happen with the interpreter which reuses more stack. +public static class FinalizerHelpers +{ + private static IntPtr aptr; + + private static unsafe void NoPinActionHelper(int depth, Action act) + { + // Avoid tail calls + int* values = stackalloc int[20]; + aptr = new IntPtr(values); + + if (depth <= 0) + { + // + // When the action is called, this new thread might have not allocated + // anything yet in the nursery. This means that the address of the first + // object that would be allocated would be at the start of the tlab and + // implicitly the end of the previous tlab (address which can be in use + // when allocating on another thread, at checking if an object fits in + // this other tlab). We allocate a new dummy object to avoid this type + // of false pinning for most common cases. + // + new object(); + act(); + ClearStack(); + } + else + { + NoPinActionHelper(depth - 1, act); + } + } + + private static unsafe void ClearStack() + { + int* values = stackalloc int[25000]; + for (int i = 0; i < 25000; i++) + values[i] = 0; + } + + public static void PerformNoPinAction(Action act) + { + Thread thr = new Thread(() => NoPinActionHelper (128, act)); + thr.Start(); + thr.Join(); + } +} + +public class BridgeBase +{ + public static int fin_count; + + ~BridgeBase() + { + fin_count++; + } +} + +public class Bridge : BridgeBase +{ + public List Links = new List(); + public int __test; + + ~Bridge() + { + Links = null; + } +} + +public class Bridge1 : BridgeBase +{ + public object Link; + ~Bridge1() + { + Link = null; + } +} + +// 128 size +public class Bridge14 : BridgeBase +{ + public object a,b,c,d,e,f,g,h,i,j,k,l,m,n; +} + +public class NonBridge +{ + public object Link; +} + +public class NonBridge2 : NonBridge +{ + public object Link2; +} + +public class NonBridge14 +{ + public object a,b,c,d,e,f,g,h,i,j,k,l,m,n; +} + + +public class BridgeTest +{ + const int OBJ_COUNT = 100 * 1000; + const int LINK_COUNT = 2; + const int EXTRAS_COUNT = 0; + const double survival_rate = 0.1; + + // Pathological case for the original old algorithm. Goes + // away when merging is replaced by appending with flag + // checking. + static void SetupLinks() + { + var list = new List(); + for (int i = 0; i < OBJ_COUNT; ++i) + { + var bridge = new Bridge(); + list.Add(bridge); + } + + var r = new Random(100); + for (int i = 0; i < OBJ_COUNT; ++i) + { + var n = list[i]; + for (int j = 0; j < LINK_COUNT; ++j) + n.Links.Add(list[r.Next (OBJ_COUNT)]); + for (int j = 0; j < EXTRAS_COUNT; ++j) + n.Links.Add(j); + if (r.NextDouble() <= survival_rate) + n.__test = 1; + } + } + + const int LIST_LENGTH = 10000; + const int FAN_OUT = 10000; + + // Pathological case for the new algorithm. Goes away with + // the single-node elimination optimization, but will still + // persist if modified by using a ladder instead of the single + // list. + static void SetupLinkedFan() + { + var head = new Bridge(); + var tail = new NonBridge(); + head.Links.Add(tail); + for (int i = 0; i < LIST_LENGTH; ++i) + { + var obj = new NonBridge (); + tail.Link = obj; + tail = obj; + } + var list = new List(); + tail.Link = list; + for (int i = 0; i < FAN_OUT; ++i) + list.Add (new Bridge()); + } + + // Pathological case for the improved old algorithm. Goes + // away with copy-on-write DynArrays, but will still persist + // if modified by using a ladder instead of the single list. + static void SetupInverseFan() + { + var tail = new Bridge(); + object list = tail; + for (int i = 0; i < LIST_LENGTH; ++i) + { + var obj = new NonBridge(); + obj.Link = list; + list = obj; + } + var heads = new Bridge[FAN_OUT]; + for (int i = 0; i < FAN_OUT; ++i) + { + var obj = new Bridge(); + obj.Links.Add(list); + heads[i] = obj; + } + } + + // Not necessarily a pathology, but a special case of where we + // generate lots of "dead" SCCs. A non-bridge object that + // can't reach a bridge object can safely be removed from the + // graph. In this special case it's a linked list hanging off + // a bridge object. We can handle this by "forwarding" edges + // going to non-bridge nodes that have only a single outgoing + // edge. That collapses the whole list into a single node. + // We could remove that node, too, by removing non-bridge + // nodes with no outgoing edges. + static void SetupDeadList() + { + var head = new Bridge(); + var tail = new NonBridge(); + head.Links.Add(tail); + for (int i = 0; i < LIST_LENGTH; ++i) + { + var obj = new NonBridge(); + tail.Link = obj; + tail = obj; + } + } + + // Triggered a bug in the forwarding mechanic. + static void SetupSelfLinks() + { + var head = new Bridge(); + var tail = new NonBridge(); + head.Links.Add(tail); + tail.Link = tail; + } + + const int L0_COUNT = 50000; + const int L1_COUNT = 50000; + const int EXTRA_LEVELS = 4; + + // Set a complex graph from one bridge to a couple. + // The graph is designed to expose naive coloring on + // tarjan and SCC explosion on classic. + static void Spider() + { + Bridge a = new Bridge(); + Bridge b = new Bridge(); + + var l1 = new List(); + for (int i = 0; i < L0_COUNT; ++i) { + var l0 = new List(); + l0.Add(a); + l0.Add(b); + l1.Add(l0); + } + var last_level = l1; + for (int l = 0; l < EXTRA_LEVELS; ++l) { + int j = 0; + var l2 = new List(); + for (int i = 0; i < L1_COUNT; ++i) { + var tmp = new List(); + tmp.Add(last_level [j++ % last_level.Count]); + tmp.Add(last_level [j++ % last_level.Count]); + l2.Add(tmp); + } + last_level = l2; + } + Bridge c = new Bridge(); + c.Links.Add(last_level); + } + + // Simulates a graph with two nested cycles that is produces by + // the async state machine when `async Task M()` method gets its + // continuation rooted by an Action held by RunnableImplementor + // (ie. the task continuation is hooked through the SynchronizationContext + // implentation and rooted only by Android bridge objects). + static void NestedCycles() + { + Bridge runnableImplementor = new Bridge (); + Bridge byteArrayOutputStream = new Bridge (); + NonBridge2 action = new NonBridge2 (); + NonBridge displayClass = new NonBridge (); + NonBridge2 asyncStateMachineBox = new NonBridge2 (); + NonBridge2 asyncStreamWriter = new NonBridge2 (); + + runnableImplementor.Links.Add(action); + action.Link = displayClass; + action.Link2 = asyncStateMachineBox; + displayClass.Link = action; + asyncStateMachineBox.Link = asyncStreamWriter; + asyncStateMachineBox.Link2 = action; + asyncStreamWriter.Link = byteArrayOutputStream; + asyncStreamWriter.Link2 = asyncStateMachineBox; + } + + // Simulates a graph where a heavy node has its fanout components + // represented by cycles with back-references to the heavy node and + // references to the same bridge objects. + // This enters a pathological path in the SCC contraction where the + // links to the bridge objects need to be correctly deduplicated. The + // deduplication causes the heavy node to no longer be heavy. + static void FauxHeavyNodeWithCycles() + { + Bridge fanout = new Bridge(); + + // Need enough edges for the node to be considered heavy by bridgeless_color_is_heavy + NonBridge[] fauxHeavyNode = new NonBridge[100]; + for (int i = 0; i < fauxHeavyNode.Length; i++) + { + NonBridge2 cycle = new NonBridge2(); + cycle.Link = fanout; + cycle.Link2 = fauxHeavyNode; + fauxHeavyNode[i] = cycle; + } + + // Need at least HEAVY_REFS_MIN + 1 fan-in nodes + Bridge[] faninNodes = new Bridge[3]; + for (int i = 0; i < faninNodes.Length; i++) + { + faninNodes[i] = new Bridge(); + faninNodes[i].Links.Add(fauxHeavyNode); + } + } + + static void RunGraphTest(Action test) + { + Console.WriteLine("Start test {0}", test.Method.Name); + FinalizerHelpers.PerformNoPinAction(test); + Console.WriteLine("-graph built-"); + for (int i = 0; i < 5; i++) + { + Console.WriteLine("-GC {0}/5-", i); + GC.Collect (); + GC.WaitForPendingFinalizers(); + } + + Console.WriteLine("Finished test {0}, finalized {1}", test.Method.Name, Bridge.fin_count); + } + + static void TestLinkedList() + { + int count = Environment.ProcessorCount + 2; + var th = new Thread [count]; + for (int i = 0; i < count; ++i) + { + th [i] = new Thread( _ => + { + var lst = new ArrayList(); + for (var j = 0; j < 500 * 1000; j++) + { + lst.Add (new object()); + if ((j % 999) == 0) + lst.Add (new BridgeBase()); + if ((j % 1000) == 0) + new BridgeBase(); + if ((j % 50000) == 0) + lst = new ArrayList(); + } + }); + + th [i].Start(); + } + + for (int i = 0; i < count; ++i) + th [i].Join(); + + GC.Collect(2); + Console.WriteLine("Finished test LinkedTest, finalized {0}", BridgeBase.fin_count); + } + + //we fill 16Mb worth of stuff, eg, 256k objects + const int major_fill = 1024 * 256; + + //4mb nursery with 64 bytes objects -> alloc half + const int nursery_obj_count = 16 * 1024; + + static void SetupFragmentation() + where TBridge : new() + where TNonBridge : new() + { + const int loops = 4; + for (int k = 0; k < loops; k++) + { + Console.WriteLine("[{0}] CrashLoop {1}/{2}", DateTime.Now, k + 1, loops); + var arr = new object[major_fill]; + for (int i = 0; i < major_fill; i++) + arr[i] = new TNonBridge(); + GC.Collect(1); + Console.WriteLine("[{0}] major fill done", DateTime.Now); + + //induce massive fragmentation + for (int i = 0; i < major_fill; i += 4) + { + arr[i + 1] = null; + arr[i + 2] = null; + arr[i + 3] = null; + } + GC.Collect (1); + Console.WriteLine("[{0}] fragmentation done", DateTime.Now); + + //since 50% is garbage, do 2 fill passes + for (int j = 0; j < 2; ++j) + { + for (int i = 0; i < major_fill; i++) + { + if ((i % 1000) == 0) + new TBridge(); + else + arr[i] = new TBridge(); + } + } + Console.WriteLine("[{0}] done spewing bridges", DateTime.Now); + + for (int i = 0; i < major_fill; i++) + arr[i] = null; + GC.Collect (); + } + } + + public static int Main(string[] args) + { +// TestLinkedList(); // Crashes, but only in this multithreaded variant + RunGraphTest(SetupFragmentation); // This passes but the following crashes ?? +// RunGraphTest(SetupFragmentation); + RunGraphTest(SetupLinks); + RunGraphTest(SetupLinkedFan); + RunGraphTest(SetupInverseFan); + + RunGraphTest(SetupDeadList); + RunGraphTest(SetupSelfLinks); + RunGraphTest(NestedCycles); + RunGraphTest(FauxHeavyNodeWithCycles); +// RunGraphTest(Spider); // Crashes + return 100; + } +} diff --git a/src/tests/GC/Features/Bridge/Bridge.csproj b/src/tests/GC/Features/Bridge/Bridge.csproj new file mode 100644 index 00000000000000..29b8c0f5fd3a2d --- /dev/null +++ b/src/tests/GC/Features/Bridge/Bridge.csproj @@ -0,0 +1,11 @@ + + + + true + false + BuildOnly + + + + + diff --git a/src/tests/GC/Features/Bridge/BridgeTester.cs b/src/tests/GC/Features/Bridge/BridgeTester.cs new file mode 100644 index 00000000000000..960e39a5e6eb0d --- /dev/null +++ b/src/tests/GC/Features/Bridge/BridgeTester.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime; +using System.Text; + +using Xunit; + +public class BridgeTester +{ + [Fact] + public static void RunTests() + { + string corerun = TestLibrary.Utilities.IsWindows ? "corerun.exe" : "corerun"; + string coreRoot = Environment.GetEnvironmentVariable("CORE_ROOT"); + string bridgeTestApp = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Bridge.dll"); + + var startInfo = new ProcessStartInfo(Path.Combine(coreRoot, corerun), bridgeTestApp); + startInfo.EnvironmentVariables["MONO_GC_DEBUG"] = "bridge=BridgeBase,bridge-compare-to=new"; + startInfo.EnvironmentVariables["MONO_GC_PARAMS"] = "bridge-implementation=tarjan,bridge-require-precise-merge"; + + using (Process p = Process.Start(startInfo)) + { + p.WaitForExit(); + Console.WriteLine ("Bridge Test App returned {0}", p.ExitCode); + if (p.ExitCode != 100) + throw new Exception("Bridge Test App failed execution"); + } + } +} diff --git a/src/tests/GC/Features/Bridge/BridgeTester.csproj b/src/tests/GC/Features/Bridge/BridgeTester.csproj new file mode 100644 index 00000000000000..5045d91e4c89a8 --- /dev/null +++ b/src/tests/GC/Features/Bridge/BridgeTester.csproj @@ -0,0 +1,17 @@ + + + true + true + + + + + + + + false + Content + Always + + +