From 9fe00836f741c68f09143cbc26ca2de1707d886f Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 5 Mar 2008 14:44:17 -0700 Subject: [PATCH] fix stack mapping code to do as many passes as necessary Previously, we had been doing exactly two passes over the event log to caculate the stack object reference map at each trace point. It turns out the correct number of passes depends on how many incorrect assumptions we make about what the stack looks like at instructions with multiple predecessors (i.e. targets of jumps and branches). Each time we detect we've made one or more incorrect assumptions during a pass, we must do another pass to correct those assumptions. That pass may in turn reveal further incorrect assumptions, and so on. --- src/compile.cpp | 106 ++++++++++++------------------------------------ test/GC.java | 20 +++++++++ 2 files changed, 47 insertions(+), 79 deletions(-) diff --git a/src/compile.cpp b/src/compile.cpp index bc4a910897..f7f1fbe490 100644 --- a/src/compile.cpp +++ b/src/compile.cpp @@ -496,6 +496,7 @@ class Context { TraceElement* traceLog; uint16_t* visitTable; uintptr_t* rootTable; + bool dirtyRoots; Vector eventLog; MyProtector protector; }; @@ -3537,7 +3538,8 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, // that position, or the contents of that position are as yet // unknown. - while (eventIndex < context->eventLog.length()) { + unsigned length = context->eventLog.length(); + while (eventIndex < length) { Event e = static_cast(context->eventLog.get(eventIndex++)); switch (e) { case PushEvent: { @@ -3549,6 +3551,7 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, case IpEvent: { ip = context->eventLog.get2(eventIndex); + eventIndex += 2; if (DebugFrameMaps) { fprintf(stderr, " roots at ip %3d: ", ip); @@ -3560,7 +3563,20 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, if (context->visitTable[ip] > 1) { for (unsigned wi = 0; wi < mapSize; ++wi) { - tableRoots[wi] &= roots[wi]; + uintptr_t newRoots = tableRoots[wi] & roots[wi]; + + if ((eventIndex == length + or context->eventLog.get(eventIndex) == PopEvent) + and newRoots != tableRoots[wi]) + { + if (DebugFrameMaps) { + fprintf(stderr, "dirty roots!\n"); + } + + context->dirtyRoots = true; + } + + tableRoots[wi] = newRoots; roots[wi] &= tableRoots[wi]; } @@ -3572,92 +3588,20 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, } else { memcpy(tableRoots, roots, mapSize * BytesPerWord); } - - eventIndex += 2; } break; case MarkEvent: { unsigned i = context->eventLog.get2(eventIndex); - markBit(roots, i); - eventIndex += 2; + + markBit(roots, i); } break; case ClearEvent: { unsigned i = context->eventLog.get2(eventIndex); + eventIndex += 2; + clearBit(roots, i); - - eventIndex += 2; - } break; - - case TraceEvent: { - eventIndex += BytesPerWord; - } break; - - default: abort(t); - } - } - - return eventIndex; -} - -unsigned -updateTraceElements(MyThread* t, Context* context, uintptr_t* originalRoots, - unsigned eventIndex) -{ - unsigned mapSize = frameMapSizeInWords(t, context->method); - - uintptr_t roots[mapSize]; - if (originalRoots) { - memcpy(roots, originalRoots, mapSize * BytesPerWord); - } else { - memset(roots, 0, mapSize * BytesPerWord); - } - - int32_t ip = -1; - - while (eventIndex < context->eventLog.length()) { - Event e = static_cast(context->eventLog.get(eventIndex++)); - switch (e) { - case PushEvent: { - eventIndex = updateTraceElements(t, context, roots, eventIndex); - } break; - - case PopEvent: - return eventIndex; - - case IpEvent: { - ip = context->eventLog.get2(eventIndex); - - if (DebugFrameMaps) { - fprintf(stderr, " map at ip %3d: ", ip); - printSet(*roots); - fprintf(stderr, "\n"); - } - - if (context->visitTable[ip] > 1) { - uintptr_t* tableRoots = context->rootTable + (ip * mapSize); - - for (unsigned wi = 0; wi < mapSize; ++wi) { - roots[wi] &= tableRoots[wi]; - } - } - - eventIndex += 2; - } break; - - case MarkEvent: { - unsigned i = context->eventLog.get2(eventIndex); - markBit(roots, i); - - eventIndex += 2; - } break; - - case ClearEvent: { - unsigned i = context->eventLog.get2(eventIndex); - clearBit(roots, i); - - eventIndex += 2; } break; case TraceEvent: { @@ -3811,6 +3755,7 @@ compile(MyThread* t, Context* context) compile(t, &frame, 0); if (UNLIKELY(t->exception)) return 0; + context->dirtyRoots = false; unsigned eventIndex = calculateFrameMaps(t, context, 0, 0); object eht = codeExceptionHandlerTable(t, methodCode(t, context->method)); @@ -3867,7 +3812,10 @@ compile(MyThread* t, Context* context) } } - updateTraceElements(t, context, 0, 0); + while (context->dirtyRoots) { + context->dirtyRoots = false; + calculateFrameMaps(t, context, 0, 0); + } return finish(t, context, 0); } diff --git a/test/GC.java b/test/GC.java index 2b9b46fc8d..eb657ef786 100644 --- a/test/GC.java +++ b/test/GC.java @@ -87,6 +87,23 @@ public class GC { System.gc(); } + private static void stackMap6(boolean predicate) { + if (predicate) { + int a = 42; + } else { + Object a = null; + } + + if (predicate) { + noop(); + } else { + Object a = null; + } + + noop(); + System.gc(); + } + public static void main(String[] args) { Object[] array = new Object[1024 * 1024]; array[0] = new Object(); @@ -119,6 +136,9 @@ public class GC { stackMap5(true); stackMap5(false); + + stackMap6(true); + stackMap6(false); } }