commit 9d420d4ed7b9e5315bc4e4248cef42034ec47530
author Elliott Hughes <enh@google.com> Thu Nov 03 18:17:19 2011 -0700
committer Elliott Hughes <enh@google.com> Thu Nov 03 18:17:19 2011 -0700
tree 89dd19327c1e3fb14b6f73c87602a1ca564bb394
parent c4102d6647043591bd89bba75454cc04d23753bd

Don't pay for filename/line number lookup unless you need to.

This wasn't a regression; the code's always been like this. But this shows up
in profiles of anything doing a lot of synchronized stuff, even when there's
no contention.

There are two awkward cases. One is that the old code used to offer a variety
of special-case messages for failures to find the current frame, save area,
and Method*. I assume this was just to help in debugging and shouldn't happen
in practice, so I don't think we've lost anything there. The other case was
that on unlocking, we used to explicitly say "unlocked". But I'm not sure we
could get into a situation where we'd be reporting contention on a monitor
that wasn't locked when we tried to lock it. So I think that's okay too.

Change-Id: Ib4401c771f717e9c8cc9c4e5346ec7a5f46a1636

vm/Sync.cpp

diff --git a/vm/Sync.cpp b/vm/Sync.cpp
index 6ac83d4..80cc6af 100644
--- a/vm/Sync.cpp
+++ b/vm/Sync.cpp
@@ -82,10 +82,10 @@
 
     /*
      * Who last acquired this monitor, when lock sampling is enabled.
-     * Even when enabled, ownerFileName may be NULL.
+     * Even when enabled, ownerMethod may be NULL.
      */
-    const char* ownerFileName;
-    u4          ownerLineNumber;
+    const Method* ownerMethod;
+    u4 ownerPc;
 };
 
 
@@ -355,8 +355,9 @@
         if (waitThreshold) {
             waitStart = dvmGetRelativeTimeUsec();
         }
-        const char* currentOwnerFileName = mon->ownerFileName;
-        u4 currentOwnerLineNumber = mon->ownerLineNumber;
+
+        const Method* currentOwnerMethod = mon->ownerMethod;
+        u4 currentOwnerPc = mon->ownerPc;
 
         dvmLockMutex(&mon->lock);
         if (waitThreshold) {
@@ -371,6 +372,15 @@
                 samplePercent = 100 * waitMs / waitThreshold;
             }
             if (samplePercent != 0 && ((u4)rand() % 100 < samplePercent)) {
+                const char* currentOwnerFileName = "no_method";
+                u4 currentOwnerLineNumber = 0;
+                if (currentOwnerMethod != NULL) {
+                    currentOwnerFileName = dvmGetMethodSourceFile(currentOwnerMethod);
+                    if (currentOwnerFileName == NULL) {
+                        currentOwnerFileName = "no_method_file";
+                    }
+                    currentOwnerLineNumber = dvmLineNumFromPC(currentOwnerMethod, currentOwnerPc);
+                }
                 logContentionEvent(self, waitMs, samplePercent,
                                    currentOwnerFileName, currentOwnerLineNumber);
             }
@@ -382,25 +392,17 @@
     // When debugging, save the current monitor holder for future
     // acquisition failures to use in sampled logging.
     if (gDvm.lockProfThreshold > 0) {
-        const StackSaveArea *saveArea;
-        const Method *meth;
-        mon->ownerLineNumber = 0;
+        mon->ownerMethod = NULL;
+        mon->ownerPc = 0;
         if (self->interpSave.curFrame == NULL) {
-            mon->ownerFileName = "no_frame";
-        } else if ((saveArea =
-                   SAVEAREA_FROM_FP(self->interpSave.curFrame)) == NULL) {
-            mon->ownerFileName = "no_save_area";
-        } else if ((meth = saveArea->method) == NULL) {
-            mon->ownerFileName = "no_method";
-        } else {
-            u4 relativePc = saveArea->xtra.currentPc - saveArea->method->insns;
-            mon->ownerFileName = (char*) dvmGetMethodSourceFile(meth);
-            if (mon->ownerFileName == NULL) {
-                mon->ownerFileName = "no_method_file";
-            } else {
-                mon->ownerLineNumber = dvmLineNumFromPC(meth, relativePc);
-            }
+            return;
         }
+        const StackSaveArea* saveArea = SAVEAREA_FROM_FP(self->interpSave.curFrame);
+        if (saveArea == NULL) {
+            return;
+        }
+        mon->ownerMethod = saveArea->method;
+        mon->ownerPc = (saveArea->xtra.currentPc - saveArea->method->insns);
     }
 }
 
@@ -443,8 +445,8 @@
          */
         if (mon->lockCount == 0) {
             mon->owner = NULL;
-            mon->ownerFileName = "unlocked";
-            mon->ownerLineNumber = 0;
+            mon->ownerMethod = NULL;
+            mon->ownerPc = 0;
             dvmUnlockMutex(&mon->lock);
         } else {
             mon->lockCount--;
@@ -617,8 +619,6 @@
     bool wasInterrupted = false;
     bool timed;
     int ret;
-    const char *savedFileName;
-    u4 savedLineNumber;
 
     assert(self != NULL);
     assert(mon != NULL);
@@ -662,10 +662,11 @@
     int prevLockCount = mon->lockCount;
     mon->lockCount = 0;
     mon->owner = NULL;
-    savedFileName = mon->ownerFileName;
-    mon->ownerFileName = NULL;
-    savedLineNumber = mon->ownerLineNumber;
-    mon->ownerLineNumber = 0;
+
+    const Method* savedMethod = mon->ownerMethod;
+    u4 savedPc = mon->ownerPc;
+    mon->ownerMethod = NULL;
+    mon->ownerPc = 0;
 
     /*
      * Update thread status.  If the GC wakes up, it'll ignore us, knowing
@@ -736,8 +737,8 @@
      */
     mon->owner = self;
     mon->lockCount = prevLockCount;
-    mon->ownerFileName = savedFileName;
-    mon->ownerLineNumber = savedLineNumber;
+    mon->ownerMethod = savedMethod;
+    mon->ownerPc = savedPc;
     waitSetRemove(mon, self);
 
     /* set self->status back to THREAD_RUNNING, and self-suspend if needed */