From 3f6b572942f6a3c6297ded77de3d3c0634c9bfdc Mon Sep 17 00:00:00 2001 From: noahfalk Date: Tue, 15 Aug 2017 22:13:01 -0700 Subject: [PATCH] Make unit tests run clean with tiered compilation A handful of tests are optimization sensitive and needed to be disabled because tiered jitting doesn't optimize right away. There was also a shutdown timing issue where the tiered jitting background compilation thread would continue calling into the JIT after the JIT was shutdown. This manifested as an error writing to the jit log after the stream had been closed. --- src/vm/appdomain.cpp | 2 +- src/vm/ceemain.cpp | 4 ++ src/vm/tieredcompilation.cpp | 87 ++++++++++++++++++++++++--------- src/vm/tieredcompilation.h | 7 ++- tests/runtest.cmd | 2 + tests/src/CLRTest.Execute.Bash.targets | 6 +-- tests/src/CLRTest.Execute.Batch.targets | 6 +-- 7 files changed, 83 insertions(+), 31 deletions(-) diff --git a/src/vm/appdomain.cpp b/src/vm/appdomain.cpp index 78ebe50..a569b90 100644 --- a/src/vm/appdomain.cpp +++ b/src/vm/appdomain.cpp @@ -8103,7 +8103,7 @@ void AppDomain::Exit(BOOL fRunFinalizers, BOOL fAsyncExit) // have exited the domain. // #ifdef FEATURE_TIERED_COMPILATION - m_tieredCompilationManager.OnAppDomainShutdown(); + m_tieredCompilationManager.Shutdown(FALSE); #endif // diff --git a/src/vm/ceemain.cpp b/src/vm/ceemain.cpp index 0bc7f49..a9cbc8f 100644 --- a/src/vm/ceemain.cpp +++ b/src/vm/ceemain.cpp @@ -1623,6 +1623,10 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading) // Indicate the EE is the shut down phase. g_fEEShutDown |= ShutDown_Start; +#ifdef FEATURE_TIERED_COMPILATION + TieredCompilationManager::ShutdownAllDomains(); +#endif + fFinalizeOK = TRUE; // Terminate the BBSweep thread diff --git a/src/vm/tieredcompilation.cpp b/src/vm/tieredcompilation.cpp index 8486bf2..a4a8c90 100644 --- a/src/vm/tieredcompilation.cpp +++ b/src/vm/tieredcompilation.cpp @@ -48,14 +48,16 @@ // # Important entrypoints in this code: // // -// a) .ctor and Init(...) - called once during AppDomain initialization -// b) OnMethodCalled(...) - called when a method is being invoked. When a method -// has been called enough times this is currently the only -// trigger that initiates re-compilation. -// c) OnAppDomainShutdown() - called during AppDomain::Exit() to begin the process -// of stopping tiered compilation. After this point no more -// background optimization work will be initiated but in-progress -// work still needs to complete. +// a) .ctor and Init(...) - called once during AppDomain initialization +// b) OnMethodCalled(...) - called when a method is being invoked. When a method +// has been called enough times this is currently the only +// trigger that initiates re-compilation. +// c) Shutdown() - called during AppDomain::Exit() to begin the process +// of stopping tiered compilation. After this point no more +// background optimization work will be initiated but in-progress +// work still needs to complete. +// d) ShutdownAllDomains() - Called from EEShutdownHelper to block until all async work is +// complete. We must do this before we shutdown the JIT. // // # Overall workflow // @@ -108,6 +110,8 @@ void TieredCompilationManager::Init(ADID appDomainId) SpinLockHolder holder(&m_lock); m_domainId = appDomainId; + m_pAsyncWorkDoneEvent = new CLREvent(); + m_pAsyncWorkDoneEvent->CreateManualEvent(TRUE); } // Called each time code in this AppDomain has been run. This is our sole entrypoint to begin @@ -190,7 +194,7 @@ void TieredCompilationManager::AsyncPromoteMethodToTier1(MethodDesc* pMethodDesc { // Our current policy throttles at 1 thread, but in the future we // could experiment with more parallelism. - m_countOptimizationThreadsRunning++; + IncrementWorkerThreadCount(); } else { @@ -203,7 +207,7 @@ void TieredCompilationManager::AsyncPromoteMethodToTier1(MethodDesc* pMethodDesc if (!ThreadpoolMgr::QueueUserWorkItem(StaticOptimizeMethodsCallback, this, QUEUE_ONLY, TRUE)) { SpinLockHolder holder(&m_lock); - m_countOptimizationThreadsRunning--; + DecrementWorkerThreadCount(); STRESS_LOG1(LF_TIEREDCOMPILATION, LL_WARNING, "TieredCompilationManager::OnMethodCalled: " "ThreadpoolMgr::QueueUserWorkItem returned FALSE (no thread will run), method=%pM\n", pMethodDesc); @@ -212,7 +216,7 @@ void TieredCompilationManager::AsyncPromoteMethodToTier1(MethodDesc* pMethodDesc EX_CATCH { SpinLockHolder holder(&m_lock); - m_countOptimizationThreadsRunning--; + DecrementWorkerThreadCount(); STRESS_LOG2(LF_TIEREDCOMPILATION, LL_WARNING, "TieredCompilationManager::OnMethodCalled: " "Exception queuing work item to threadpool, hr=0x%x, method=%pM\n", GET_EXCEPTION()->GetHR(), pMethodDesc); @@ -222,19 +226,35 @@ void TieredCompilationManager::AsyncPromoteMethodToTier1(MethodDesc* pMethodDesc return; } -void TieredCompilationManager::OnAppDomainShutdown() +// static +// called from EEShutDownHelper +void TieredCompilationManager::ShutdownAllDomains() { - CONTRACTL + STANDARD_VM_CONTRACT; + + AppDomainIterator domain(TRUE); + while (domain.Next()) { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - CAN_TAKE_LOCK; + AppDomain * pDomain = domain.GetDomain(); + if (pDomain != NULL) + { + pDomain->GetTieredCompilationManager()->Shutdown(TRUE); + } } - CONTRACTL_END +} - SpinLockHolder holder(&m_lock); - m_isAppDomainShuttingDown = TRUE; +void TieredCompilationManager::Shutdown(BOOL fBlockUntilAsyncWorkIsComplete) +{ + STANDARD_VM_CONTRACT; + + { + SpinLockHolder holder(&m_lock); + m_isAppDomainShuttingDown = TRUE; + } + if (fBlockUntilAsyncWorkIsComplete) + { + m_pAsyncWorkDoneEvent->Wait(INFINITE, FALSE); + } } // This is the initial entrypoint for the background thread, called by @@ -268,7 +288,7 @@ void TieredCompilationManager::OptimizeMethodsCallback() SpinLockHolder holder(&m_lock); if (m_isAppDomainShuttingDown) { - m_countOptimizationThreadsRunning--; + DecrementWorkerThreadCount(); return; } } @@ -289,7 +309,7 @@ void TieredCompilationManager::OptimizeMethodsCallback() if (nativeCodeVersion.IsNull() || m_isAppDomainShuttingDown) { - m_countOptimizationThreadsRunning--; + DecrementWorkerThreadCount(); break; } @@ -305,7 +325,7 @@ void TieredCompilationManager::OptimizeMethodsCallback() if (!ThreadpoolMgr::QueueUserWorkItem(StaticOptimizeMethodsCallback, this, QUEUE_ONLY, TRUE)) { SpinLockHolder holder(&m_lock); - m_countOptimizationThreadsRunning--; + DecrementWorkerThreadCount(); STRESS_LOG0(LF_TIEREDCOMPILATION, LL_WARNING, "TieredCompilationManager::OptimizeMethodsCallback: " "ThreadpoolMgr::QueueUserWorkItem returned FALSE (no thread will run)\n"); } @@ -421,6 +441,27 @@ NativeCodeVersion TieredCompilationManager::GetNextMethodToOptimize() return NativeCodeVersion(); } +void TieredCompilationManager::IncrementWorkerThreadCount() +{ + STANDARD_VM_CONTRACT; + //m_lock should be held + + m_countOptimizationThreadsRunning++; + m_pAsyncWorkDoneEvent->Reset(); +} + +void TieredCompilationManager::DecrementWorkerThreadCount() +{ + STANDARD_VM_CONTRACT; + //m_lock should be held + + m_countOptimizationThreadsRunning--; + if (m_countOptimizationThreadsRunning == 0) + { + m_pAsyncWorkDoneEvent->Set(); + } +} + //static CORJIT_FLAGS TieredCompilationManager::GetJitFlags(NativeCodeVersion nativeCodeVersion) { diff --git a/src/vm/tieredcompilation.h b/src/vm/tieredcompilation.h index 6e155fe..1e28f38 100644 --- a/src/vm/tieredcompilation.h +++ b/src/vm/tieredcompilation.h @@ -27,7 +27,8 @@ public: void Init(ADID appDomainId); BOOL OnMethodCalled(MethodDesc* pMethodDesc, DWORD currentCallCount); void AsyncPromoteMethodToTier1(MethodDesc* pMethodDesc); - void OnAppDomainShutdown(); + static void ShutdownAllDomains(); + void Shutdown(BOOL fBlockUntilAsyncWorkIsComplete); static CORJIT_FLAGS GetJitFlags(NativeCodeVersion nativeCodeVersion); private: @@ -39,6 +40,9 @@ private: BOOL CompileCodeVersion(NativeCodeVersion nativeCodeVersion); void ActivateCodeVersion(NativeCodeVersion nativeCodeVersion); + void IncrementWorkerThreadCount(); + void DecrementWorkerThreadCount(); + SpinLock m_lock; SList> m_methodsToOptimize; ADID m_domainId; @@ -46,6 +50,7 @@ private: DWORD m_countOptimizationThreadsRunning; DWORD m_callCountOptimizationThreshhold; DWORD m_optimizationQuantumMs; + CLREvent* m_pAsyncWorkDoneEvent; }; #endif // FEATURE_TIERED_COMPILATION diff --git a/tests/runtest.cmd b/tests/runtest.cmd index 675f1f4..936383e 100644 --- a/tests/runtest.cmd +++ b/tests/runtest.cmd @@ -85,6 +85,7 @@ if /i "%1" == "GenerateLayoutOnly" (set __GenerateLayoutOnly=1&shift&goto Arg if /i "%1" == "PerfTests" (set __PerfTests=true&shift&goto Arg_Loop) if /i "%1" == "runcrossgentests" (set RunCrossGen=true&shift&goto Arg_Loop) if /i "%1" == "link" (set DoLink=true&set ILLINK=%2&shift&shift&goto Arg_Loop) +if /i "%1" == "tieredcompilation" (set COMPLUS_EXPERIMENTAL_TieredCompilation=1&shift&goto Arg_Loop) REM change it to COMPlus_GCStress when we stop using xunit harness if /i "%1" == "gcstresslevel" (set __GCSTRESSLEVEL=%2&set __TestTimeout=1800000&shift&shift&goto Arg_Loop) @@ -464,6 +465,7 @@ echo 2: GC on transitions to preemptive GC echo 4: GC on every allowable JITed instruction echo 8: GC on every allowable NGEN instruction echo 16: GC only on a unique stack trace +echo tieredcompilation - Run the tests with COMPlus_EXPERIMENTAL_TieredCompilation=1 echo msbuildargs ^ - Pass all subsequent args directly to msbuild invocations. echo ^ - Path to the runtime to test (if specified). echo. diff --git a/tests/src/CLRTest.Execute.Bash.targets b/tests/src/CLRTest.Execute.Bash.targets index 10f4aa6..73f7031 100644 --- a/tests/src/CLRTest.Execute.Bash.targets +++ b/tests/src/CLRTest.Execute.Bash.targets @@ -83,9 +83,9 @@ fi ]]> @@ -396,4 +396,4 @@ $(BashCLRTestExitCodeCheck) Overwrite="true" /> - \ No newline at end of file + diff --git a/tests/src/CLRTest.Execute.Batch.targets b/tests/src/CLRTest.Execute.Batch.targets index dfd87ba..7abf16a 100644 --- a/tests/src/CLRTest.Execute.Batch.targets +++ b/tests/src/CLRTest.Execute.Batch.targets @@ -78,8 +78,8 @@ IF NOT "%COMPlus_GCStress%"=="" ( ]]> - \ No newline at end of file + -- 2.7.4