commit 22f454c7d349c439c774c5d79b32514b190fd531
author Ian Rogers <irogers@google.com> Sat Sep 08 11:06:29 2012 -0700
committer Ian Rogers <irogers@google.com> Sat Sep 08 16:44:23 2012 -0700
tree b19271deb46c5313c72df18713d6b6be6798077b
parent d83bc36185c8b2a763c79fa364b5387faa26c669

Remove a lock from ScopedThreadStateChange.

Remove a lock used while inspecting the current thread state in
ScopedThreadStateChange. It is safe to do so as the code just checks the
state and the suspend count checks occur in the transition logic (with
the appropriate locks held).

Gives small but noticeable performance improvement to the JniField
microbenchmark.

Change-Id: Iadda8ae0b30355dc3d7e4daeed760c41b3fecfc2

src/scoped_thread_state_change.h

diff --git a/src/scoped_thread_state_change.h b/src/scoped_thread_state_change.h
index ed3c384..68fbf69 100644
--- a/src/scoped_thread_state_change.h
+++ b/src/scoped_thread_state_change.h
@@ -37,13 +37,14 @@
       CHECK(!Runtime::Current()->IsStarted() || Runtime::Current()->IsShuttingDown());
     } else {
       bool runnable_transition;
-      {
-        MutexLock mu(*Locks::thread_suspend_count_lock_);
-        old_thread_state_ = self->GetState();
-        runnable_transition = old_thread_state_ == kRunnable || new_thread_state == kRunnable;
-        if (!runnable_transition) {
-          self_->SetState(new_thread_state);
-        }
+      DCHECK_EQ(self, Thread::Current());
+      // Read state without locks, ok as state is effectively thread local and we're not interested
+      // in the suspend count (this will be handled in the runnable transitions).
+      old_thread_state_ = self->GetStateUnsafe();
+      runnable_transition = old_thread_state_ == kRunnable || new_thread_state == kRunnable;
+      if (!runnable_transition) {
+        // A suspended transition to another effectively suspended transition, ok to use Unsafe.
+        self_->SetStateUnsafe(new_thread_state);
       }
       if (runnable_transition && old_thread_state_ != new_thread_state) {
         if (new_thread_state == kRunnable) {
@@ -68,8 +69,8 @@
         } else if (thread_state_ == kRunnable) {
           self_->TransitionFromRunnableToSuspended(old_thread_state_);
         } else {
-          MutexLock mu(*Locks::thread_suspend_count_lock_);
-          self_->SetState(old_thread_state_);
+          // A suspended transition to another effectively suspended transition, ok to use Unsafe.
+          self_->SetStateUnsafe(old_thread_state_);
         }
       }
     }

src/thread.h

diff --git a/src/thread.h b/src/thread.h
index 9355dca..e37e17a 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -617,14 +617,14 @@
   friend class Runtime; // For CreatePeer.
 
   // TODO: remove, callers should use GetState and hold the appropriate locks. Used only by
-  //       ShortDump and TransitionFromSuspendedToRunnable.
+  //       ShortDump, TransitionFromSuspendedToRunnable and ScopedThreadStateChange.
   ThreadState GetStateUnsafe() const NO_THREAD_SAFETY_ANALYSIS {
     return state_;
   }
 
   // TODO: remove, callers should use SetState and hold the appropriate locks. Used only by
-  //       TransitionFromRunnableToSuspended that doesn't need to observe suspend counts as
-  //       it is by definition suspended anyway.
+  //       TransitionFromRunnableToSuspended and ScopedThreadStateChange that don't need to observe
+  //       suspend counts in situations where they know that the thread is already suspended.
   ThreadState SetStateUnsafe(ThreadState new_state) NO_THREAD_SAFETY_ANALYSIS {
     ThreadState old_state = state_;
     state_ = new_state;
@@ -796,6 +796,8 @@
   // How many times has our pthread key's destructor been called?
   uint32_t thread_exit_check_count_;
 
+  friend class ScopedThreadStateChange;
+
   DISALLOW_COPY_AND_ASSIGN(Thread);
 };