[JIT] Fix for 3385583: Performance variance
Closes a window in which the "interpret-only" templace could get chained
to an existing trace while the intended translation was under construction.
Note that this CL also introduces some small, but fundamental changes in trace
formation:
1. Previouosly, when an exception or other trace terminating event
occurred during trace formation, the entire trace was abandoned. With this
change, we instead end the trace at the last successful instruction.
2. We previously allowed multiple attempts (perhaps by multiple threads)
to form a trace compilation request for a dalvik PC. This was done in an
attempt to allow recovery from compiler failures. Now we enforce a new rule:
only the thread that wins the race to allocate an entry in the JitTable will
form the trace request.
3. In a (probably misguided) attempt avoid unnecessary contention, we
previously allowed work order enqueue requests to be dropped if a requester
did not aquire TableLock on first attempt (assuming that if the trace were
hot, it would be requested again). Now we block on enqueue.
Change-Id: I40ea4f1b012250219ca37d5c40c5f22cae2092f1
diff --git a/vm/interp/Jit.c b/vm/interp/Jit.c
index 7abe728..6d7ea52 100644
--- a/vm/interp/Jit.c
+++ b/vm/interp/Jit.c
@@ -518,30 +518,11 @@
}
-static void setTraceConstruction(JitEntry *slot, bool value)
-{
-
- JitEntryInfoUnion oldValue;
- JitEntryInfoUnion newValue;
- do {
- oldValue = slot->u;
- newValue = oldValue;
- newValue.info.traceConstruction = value;
- } while (android_atomic_release_cas(oldValue.infoWord, newValue.infoWord,
- &slot->u.infoWord) != 0);
-}
-
-static void resetTracehead(InterpState* interpState, JitEntry *slot)
-{
- slot->codeAddress = dvmCompilerGetInterpretTemplate();
- setTraceConstruction(slot, false);
-}
-
-/* Clean up any pending trace builds */
-void dvmJitAbortTraceSelect(InterpState* interpState)
+/* End current trace after last successful instruction */
+void dvmJitEndTraceSelect(InterpState* interpState)
{
if (interpState->jitState == kJitTSelect)
- interpState->jitState = kJitDone;
+ interpState->jitState = kJitTSelectEnd;
}
/*
@@ -848,9 +829,12 @@
/* NOTE: intentional fallthrough for returns */
case kJitTSelectEnd:
{
- /* Bad trace */
+ /* Empty trace - set to bail to interpreter */
if (interpState->totalTraceLen == 0) {
- /* Bad trace - mark as untranslatable */
+ dvmJitSetCodeAddr(interpState->currTraceHead,
+ dvmCompilerGetInterpretTemplate(),
+ dvmCompilerGetInterpretTemplateSet(),
+ false /* Not method entry */, 0);
interpState->jitState = kJitDone;
switchInterp = true;
break;
@@ -902,17 +886,6 @@
*/
free(desc);
}
- /*
- * Reset "trace in progress" flag whether or not we
- * successfully entered a work order.
- */
- JitEntry *jitEntry =
- lookupAndAdd(interpState->currTraceHead,
- false /* lock */,
- false /* method entry */);
- if (jitEntry) {
- setTraceConstruction(jitEntry, false);
- }
interpState->jitState = kJitDone;
switchInterp = true;
}
@@ -973,18 +946,21 @@
!dvmJitStayInPortableInterpreter();
}
-JitEntry *dvmFindJitEntry(const u2* pc)
+JitEntry *dvmJitFindEntry(const u2* pc, bool isMethodEntry)
{
int idx = dvmJitHash(pc);
/* Expect a high hit rate on 1st shot */
- if (gDvmJit.pJitEntryTable[idx].dPC == pc)
+ if ((gDvmJit.pJitEntryTable[idx].dPC == pc) &&
+ (gDvmJit.pJitEntryTable[idx].u.info.isMethodEntry == isMethodEntry))
return &gDvmJit.pJitEntryTable[idx];
else {
int chainEndMarker = gDvmJit.jitTableSize;
while (gDvmJit.pJitEntryTable[idx].u.info.chain != chainEndMarker) {
idx = gDvmJit.pJitEntryTable[idx].u.info.chain;
- if (gDvmJit.pJitEntryTable[idx].dPC == pc)
+ if ((gDvmJit.pJitEntryTable[idx].dPC == pc) &&
+ (gDvmJit.pJitEntryTable[idx].u.info.isMethodEntry ==
+ isMethodEntry))
return &gDvmJit.pJitEntryTable[idx];
}
}
@@ -1074,7 +1050,7 @@
{
JitEntryInfoUnion oldValue;
JitEntryInfoUnion newValue;
- JitEntry *jitEntry = lookupAndAdd(dPC, false, isMethodEntry);
+ JitEntry *jitEntry = dvmJitFindEntry(dPC, isMethodEntry);
assert(jitEntry);
/* Note: order of update is important */
do {
@@ -1188,45 +1164,24 @@
*/
if (interpState->jitState == kJitTSelectRequest ||
interpState->jitState == kJitTSelectRequestHot) {
- JitEntry *slot = lookupAndAdd(interpState->pc,
- false /* lock */,
- false /* method entry */);
- if (slot == NULL) {
- /*
- * Table is full. This should have been
- * detected by the compiler thread and the table
- * resized before we run into it here. Assume bad things
- * are afoot and disable profiling.
- */
- interpState->jitState = kJitDone;
- LOGD("JIT: JitTable full, disabling profiling");
- dvmJitStopTranslationRequests();
- } else if (slot->u.info.traceConstruction) {
- /*
- * Trace request already in progress, but most likely it
- * aborted without cleaning up. Assume the worst and
- * mark trace head as untranslatable. If we're wrong,
- * the compiler thread will correct the entry when the
- * translation is completed. The downside here is that
- * some existing translation may chain to the interpret-only
- * template instead of the real translation during this
- * window. Performance, but not correctness, issue.
- */
- interpState->jitState = kJitDone;
- resetTracehead(interpState, slot);
- } else if (slot->codeAddress) {
- /* Nothing to do here - just return */
- interpState->jitState = kJitDone;
+ if (dvmJitFindEntry(interpState->pc, false)) {
+ /* In progress - nothing do do */
+ interpState->jitState = kJitDone;
} else {
- /*
- * Mark request. Note, we are not guaranteed exclusivity
- * here. A window exists for another thread to be
- * attempting to build this same trace. Rather than
- * bear the cost of locking, we'll just allow that to
- * happen. The compiler thread, if it chooses, can
- * discard redundant requests.
- */
- setTraceConstruction(slot, true);
+ JitEntry *slot = lookupAndAdd(interpState->pc,
+ false /* lock */,
+ false /* method entry */);
+ if (slot == NULL) {
+ /*
+ * Table is full. This should have been
+ * detected by the compiler thread and the table
+ * resized before we run into it here. Assume bad things
+ * are afoot and disable profiling.
+ */
+ interpState->jitState = kJitDone;
+ LOGD("JIT: JitTable full, disabling profiling");
+ dvmJitStopTranslationRequests();
+ }
}
}
@@ -1450,21 +1405,21 @@
void dvmJitTraceProfilingOn()
{
if (gDvmJit.profileMode == kTraceProfilingPeriodicOff)
- dvmCompilerWorkEnqueue(NULL, kWorkOrderProfileMode,
- (void*) kTraceProfilingPeriodicOn);
+ dvmCompilerForceWorkEnqueue(NULL, kWorkOrderProfileMode,
+ (void*) kTraceProfilingPeriodicOn);
else if (gDvmJit.profileMode == kTraceProfilingDisabled)
- dvmCompilerWorkEnqueue(NULL, kWorkOrderProfileMode,
- (void*) kTraceProfilingContinuous);
+ dvmCompilerForceWorkEnqueue(NULL, kWorkOrderProfileMode,
+ (void*) kTraceProfilingContinuous);
}
void dvmJitTraceProfilingOff()
{
if (gDvmJit.profileMode == kTraceProfilingPeriodicOn)
- dvmCompilerWorkEnqueue(NULL, kWorkOrderProfileMode,
- (void*) kTraceProfilingPeriodicOff);
+ dvmCompilerForceWorkEnqueue(NULL, kWorkOrderProfileMode,
+ (void*) kTraceProfilingPeriodicOff);
else if (gDvmJit.profileMode == kTraceProfilingContinuous)
- dvmCompilerWorkEnqueue(NULL, kWorkOrderProfileMode,
- (void*) kTraceProfilingDisabled);
+ dvmCompilerForceWorkEnqueue(NULL, kWorkOrderProfileMode,
+ (void*) kTraceProfilingDisabled);
}
#endif /* WITH_JIT */
diff --git a/vm/interp/Jit.h b/vm/interp/Jit.h
index e86d548..c49b365 100644
--- a/vm/interp/Jit.h
+++ b/vm/interp/Jit.h
@@ -112,12 +112,12 @@
*/
typedef struct JitEntryInfo {
- unsigned int traceConstruction:1; /* build underway? */
unsigned int isMethodEntry:1;
unsigned int inlineCandidate:1;
unsigned int profileEnabled:1;
- JitInstructionSetType instructionSet:4;
- unsigned int profileOffset:8;
+ JitInstructionSetType instructionSet:3;
+ unsigned int profileOffset:5;
+ unsigned int unused:5;
u2 chain; /* Index of next in chain */
} JitEntryInfo;
@@ -141,12 +141,12 @@
void dvmJitStats(void);
bool dvmJitResizeJitTable(unsigned int size);
void dvmJitResetTable(void);
-struct JitEntry *dvmFindJitEntry(const u2* pc);
+struct JitEntry *dvmJitFindEntry(const u2* pc, bool isMethodEntry);
s8 dvmJitd2l(double d);
s8 dvmJitf2l(float f);
void dvmJitSetCodeAddr(const u2* dPC, void *nPC, JitInstructionSetType set,
bool isMethodEntry, int profilePrefixSize);
-void dvmJitAbortTraceSelect(InterpState* interpState);
+void dvmJitEndTraceSelect(InterpState* interpState);
JitTraceCounter_t *dvmJitNextTraceCounter(void);
void dvmJitTraceProfilingOff(void);
void dvmJitTraceProfilingOn(void);