Fixes for code review feedback

dotnet · Mar 29, 2017 · 52ed5e6 · 52ed5e6
1 parent c756426
commit 52ed5e6
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 22 deletions.
diff --git a/src/vm/appdomain.cpp b/src/vm/appdomain.cpp
@@ -4334,7 +4334,7 @@ void AppDomain::Init()
 #endif //FEATURE_COMINTEROP
 
 #ifdef FEATURE_TIERED_COMPILATION
-    m_callCounter.SetTieredCompilationManager(&GetTieredCompilationManager());
+    m_callCounter.SetTieredCompilationManager(GetTieredCompilationManager());
     m_tieredCompilationManager.Init(GetId());
 #endif
 #endif // CROSSGEN_COMPILE

diff --git a/src/vm/appdomain.hpp b/src/vm/appdomain.hpp
@@ -3831,20 +3831,20 @@ class AppDomain : public BaseDomain
 #if defined(FEATURE_TIERED_COMPILATION)
 
 public:
-    TieredCompilationManager & GetTieredCompilationManager()
+    TieredCompilationManager * GetTieredCompilationManager()
     {
         LIMITED_METHOD_CONTRACT;
-        return m_tieredCompilationManager;
+        return &m_tieredCompilationManager;
     }
 
 private:
     TieredCompilationManager m_tieredCompilationManager;
 
 public:
-    CallCounter & GetCallCounter()
+    CallCounter * GetCallCounter()
     {
         LIMITED_METHOD_CONTRACT;
-        return m_callCounter;
+        return &m_callCounter;
     }
 
 private:

diff --git a/src/vm/callcounter.cpp b/src/vm/callcounter.cpp
@@ -37,8 +37,7 @@ void CallCounter::SetTieredCompilationManager(TieredCompilationManager* pTieredC
     }
     CONTRACTL_END;
 
-    SpinLockHolder holder(&m_lock);
-    m_pTieredCompilationManager = pTieredCompilationManager;
+    m_pTieredCompilationManager.Store(pTieredCompilationManager);
 }
 
 // This is called by the prestub each time the method is invoked in a particular
@@ -80,8 +79,6 @@ BOOL CallCounter::OnMethodCalled(MethodDesc* pMethodDesc)
         //each claiming to be exactly the threshhold call count needed to trigger
         //optimization.
         SpinLockHolder holder(&m_lock);
-        pCallCounterSink = m_pTieredCompilationManager;
-
         CallCounterEntry* pEntry = const_cast<CallCounterEntry*>(m_methodToCallCount.LookupPtr(pMethodDesc));
         if (pEntry == NULL)
         {
@@ -95,7 +92,7 @@ BOOL CallCounter::OnMethodCalled(MethodDesc* pMethodDesc)
         }
     }
 
-    return pCallCounterSink->OnMethodCalled(pMethodDesc, callCount);
+    return m_pTieredCompilationManager.Load()->OnMethodCalled(pMethodDesc, callCount);
 }
 
 #endif // FEATURE_TIERED_COMPILATION
diff --git a/src/vm/callcounter.h b/src/vm/callcounter.h
@@ -75,8 +75,10 @@ class CallCounter
 
 private:
 
+    VolatilePtr<TieredCompilationManager> m_pTieredCompilationManager;
+
+    // fields protected by lock
     SpinLock m_lock;
-    TieredCompilationManager* m_pTieredCompilationManager;
     CallCounterHash m_methodToCallCount;
 };
 

diff --git a/src/vm/method.hpp b/src/vm/method.hpp
@@ -1318,7 +1318,7 @@ class MethodDesc
     // pointer need to be very careful about if and when they cache it
     // if it is not stable.
     //
-    // The stability of the native code pointer is seperate from the
+    // The stability of the native code pointer is separate from the
     // stability of the entrypoint. A stable entrypoint can be a precode
     // which dispatches to an unstable native code pointer.
     BOOL IsNativeCodeStableAfterInit()

diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp
@@ -7034,12 +7034,12 @@ MethodTableBuilder::NeedsNativeCodeSlot(bmtMDMethod * pMDMethod)
 
 #ifdef FEATURE_TIERED_COMPILATION
     // Keep in-sync with MethodDesc::IsEligibleForTieredCompilation()
-	if (g_pConfig->TieredCompilation() &&
+    if (g_pConfig->TieredCompilation() &&
         !GetModule()->HasNativeOrReadyToRunImage() &&
-		(pMDMethod->GetMethodType() == METHOD_TYPE_NORMAL || pMDMethod->GetMethodType() == METHOD_TYPE_INSTANTIATED))
-	{
-		return TRUE;
-	}
+        (pMDMethod->GetMethodType() == METHOD_TYPE_NORMAL || pMDMethod->GetMethodType() == METHOD_TYPE_INSTANTIATED))
+    {
+        return TRUE;
+    }
 #endif
 
     return GetModule()->IsEditAndContinueEnabled();

diff --git a/src/vm/prestub.cpp b/src/vm/prestub.cpp
@@ -1311,8 +1311,8 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
         PCODE pNativeCode = GetNativeCode();
         if (pNativeCode && IsEligibleForTieredCompilation())
         {
-            CallCounter & callCounter = GetAppDomain()->GetCallCounter();
-            BOOL doBackPatch = callCounter.OnMethodCalled(this);
+            CallCounter * pCallCounter = GetAppDomain()->GetCallCounter();
+            BOOL doBackPatch = pCallCounter->OnMethodCalled(this);
             if (!doBackPatch)
             {
                 return pNativeCode;
@@ -1626,8 +1626,8 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
 #ifdef FEATURE_TIERED_COMPILATION
     if (pCode && IsEligibleForTieredCompilation())
     {
-        CallCounter & callCounter = GetAppDomain()->GetCallCounter();
-        BOOL doBackPatch = callCounter.OnMethodCalled(this);
+        CallCounter * pCallCounter = GetAppDomain()->GetCallCounter();
+        BOOL doBackPatch = pCallCounter->OnMethodCalled(this);
         if (!doBackPatch)
         {
             return pCode;

diff --git a/src/vm/tieredcompilation.cpp b/src/vm/tieredcompilation.cpp
@@ -141,8 +141,8 @@ BOOL TieredCompilationManager::OnMethodCalled(MethodDesc* pMethodDesc, DWORD cur
     // unserviced. Synchronous retries appear unlikely to offer any material improvement 
     // and complicating the code to narrow an already rare error case isn't desirable.
     {
-        SpinLockHolder holder(&m_lock);
         SListElem<MethodDesc*>* pMethodListItem = new (nothrow) SListElem<MethodDesc*>(pMethodDesc);
+        SpinLockHolder holder(&m_lock);
         if (pMethodListItem != NULL)
         {
             m_methodsToOptimize.InsertTail(pMethodListItem);