PR feedback

* Removed Volatile<T> from globals
  since they are all accessed either via InterlockedExchange
  or VolatileLoad
* Added no tearing helper function for 32 bit systems
* updated tests to compare before and after
* added tests for AOT and Mono specific behaviors
* furether simplified NormalizedTimer class with asserts

src/coreclr/vm/jitinterface.cpp

@@ -102,35 +102,47 @@ GARY_IMPL(VMHELPDEF, hlpDynamicFuncTable, DYNAMIC_CORINFO_HELP_COUNT);
 
 #else // DACCESS_COMPILE
 
-Volatile<int64_t> g_cbILJitted = 0;
-Volatile<int64_t> g_cMethodsJitted = 0;
-Volatile<int64_t> g_c100nsTicksInJit = 0;
+int64_t g_cbILJitted = 0;
+int64_t g_cMethodsJitted = 0;
+int64_t g_c100nsTicksInJit = 0;
 thread_local int64_t t_cbILJittedForThread = 0;
 thread_local int64_t t_cMethodsJittedForThread = 0;
 thread_local int64_t t_c100nsTicksInJitForThread = 0;
 
+// This prevents tearing of 64 bit values on 32 bit systems
+static inline
+int64_t AtomicLoad64WithoutTearing(int64_t *valueRef)
+{
+    WRAPPER_NO_CONTRACT;
+#if TARGET_64BIT
+    return VolatileLoad(valueRef);
+#else
+    return InterlockedCompareExchangeT((int64_T*)valueRef, 0, 0);
+#endif // TARGET_64BIT
+}
+
 #ifndef CROSSGEN_COMPILE
 FCIMPL1(INT64, GetCompiledILBytes, CLR_BOOL currentThread)
 {
     FCALL_CONTRACT;
 
-    return currentThread ? t_cbILJittedForThread : g_cbILJitted.Load();
+    return currentThread ? t_cbILJittedForThread : AtomicLoad64WithoutTearing(&g_cbILJitted);
 }
 FCIMPLEND
 
 FCIMPL1(INT64, GetCompiledMethodCount, CLR_BOOL currentThread)
 {
     FCALL_CONTRACT;
 
-    return currentThread ? t_cMethodsJittedForThread : g_cMethodsJitted.Load();
+    return currentThread ? t_cMethodsJittedForThread : AtomicLoad64WithoutTearing(&g_cMethodsJitted);
 }
 FCIMPLEND
 
 FCIMPL1(INT64, GetCompilationTimeInTicks, CLR_BOOL currentThread)
 {
     FCALL_CONTRACT;
 
-    return currentThread ? t_c100nsTicksInJitForThread : g_c100nsTicksInJit.Load();
+    return currentThread ? t_c100nsTicksInJitForThread : AtomicLoad64WithoutTearing(&g_c100nsTicksInJit);
 }
 FCIMPLEND
 #endif

src/coreclr/vm/jitinterface.h

@@ -1158,9 +1158,9 @@ CORJIT_FLAGS GetDebuggerCompileFlags(Module* pModule, CORJIT_FLAGS flags);
 bool __stdcall TrackAllocationsEnabled();
 
 
-extern Volatile<int64_t> g_cbILJitted;
-extern Volatile<int64_t> g_cMethodsJitted;
-extern Volatile<int64_t> g_c100nsTicksInJit;
+extern int64_t g_cbILJitted;
+extern int64_t g_cMethodsJitted;
+extern int64_t g_c100nsTicksInJit;
 extern thread_local int64_t t_cbILJittedForThread;
 extern thread_local int64_t t_cMethodsJittedForThread;
 extern thread_local int64_t t_c100nsTicksInJitForThread;

src/coreclr/vm/util.hpp

@@ -929,11 +929,15 @@ class NormalizedTimer
 
     LARGE_INTEGER startTimestamp;
     LARGE_INTEGER stopTimestamp;
+
+#if _DEBUG
     bool isRunning = false;
+#endif // _DEBUG
 
 public:
     NormalizedTimer()
     {
+        LIMITED_METHOD_CONTRACT;
         if (s_frequency.Load() == -1)
         {
             double frequency;
@@ -953,22 +957,27 @@ class NormalizedTimer
     inline
     void Start()
     {
+        LIMITED_METHOD_CONTRACT;
+        _ASSERTE(!isRunning);
         QueryPerformanceCounter(&startTimestamp);
-        stopTimestamp.QuadPart = 0;
+
+#if _DEBUG
         isRunning = true;
+#endif // _DEBUG
     }
 
     // ======================================================================================
     // stop the timer. If called before starting, sets the start time to the same as the stop
     inline
     void Stop()
     {
+        LIMITED_METHOD_CONTRACT;
+        _ASSERTE(isRunning);
         QueryPerformanceCounter(&stopTimestamp);
-        // protect against stop before start
-        if (!isRunning)
-            startTimestamp = stopTimestamp;
 
+#if _DEBUG
         isRunning = false;
+#endif // _DEBUG
     }
 
     // ======================================================================================
@@ -978,9 +987,10 @@ class NormalizedTimer
     inline
     int64_t Elapsed100nsTicks()
     {
-        if (isRunning)
-            Stop();
-
+        LIMITED_METHOD_CONTRACT;
+        _ASSERTE(!isRunning);
+        _ASSERTE(startTimestamp.QuadPart > 0);
+        _ASSERTE(stopTimestamp.QuadPart > 0);
         return static_cast<int64_t>((stopTimestamp.QuadPart - startTimestamp.QuadPart) / s_frequency);
     }
 };

src/libraries/System.Runtime/tests/System/Runtime/JitInfoTests.cs

@@ -15,58 +15,123 @@ public class JitInfoTests
         [SkipOnPlatform(AotPlatforms, "JitInfo metrics will be 0 in AOT scenarios.")]
         public void JitInfoIsPopulated()
         {
+            TimeSpan beforeCompilationTime = System.Runtime.JitInfo.GetCompilationTime();
+            long beforeCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes();
+            long beforeCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();
+
+            Func<string> theFunc = () => "JIT compile this!";
+            Assert.True(theFunc().Equals("JIT compile this!"));
+
+            TimeSpan afterCompilationTime = System.Runtime.JitInfo.GetCompilationTime();
+            long afterCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes();
+            long afterCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();
+
+            Assert.True(beforeCompilationTime > TimeSpan.Zero, $"Compilation time not greater than 0! ({beforeCompilationTime})");
+            Assert.True(beforeCompiledILBytes > 0, $"Compiled IL bytes not greater than 0! ({beforeCompiledILBytes})");
+            Assert.True(beforeCompiledMethodCount > 0, $"Compiled method count not greater than 0! ({beforeCompiledMethodCount})");
+
+            Assert.True(afterCompilationTime > beforeCompilationTime, $"CompilationTime: after not greater than before! (after: {afterCompilationTime}, before: {beforeCompilationTime})");
+            Assert.True(afterCompiledILBytes > beforeCompiledILBytes, $"Compiled IL bytes: after not greater than before! (after: {afterCompiledILBytes}, before: {beforeCompiledILBytes})");
+            Assert.True(afterCompiledMethodCount > beforeCompiledMethodCount, $"Compiled method count: after not greater than before! (after: {afterCompiledMethodCount}, before: {beforeCompiledMethodCount})");
+        }
+
+        [Fact]
+        [SkipOnPlatform(~AotPlatforms, "JitInfo metrics will be 0 in AOT scenarios.")]
+        public void JitInfoIsNotPopulated()
+        {
+            TimeSpan beforeCompilationTime = System.Runtime.JitInfo.GetCompilationTime();
+            long beforeCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes();
+            long beforeCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();
+
             Func<string> theFunc = () => "JIT compile this!";
             Assert.True(theFunc().Equals("JIT compile this!"));
 
-            TimeSpan compilationTime = System.Runtime.JitInfo.GetCompilationTime();
-            long compiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes();
-            long compiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();
+            TimeSpan afterCompilationTime = System.Runtime.JitInfo.GetCompilationTime();
+            long afterCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes();
+            long afterCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount();
 
-            Assert.True(compilationTime > TimeSpan.Zero, $"Compilation time not greater than 0! ({compilationTime})");
-            Assert.True(compiledILBytes > 0, $"Compiled IL bytes not greater than 0! ({compiledILBytes})");
-            Assert.True(compiledMethodCount > 0, $"Compiled method count not greater than 0! ({compiledMethodCount})");
+            Assert.True(beforeCompilationTime == TimeSpan.Zero, $"Before Compilation time not eqeual to 0! ({beforeCompilationTime})");
+            Assert.True(beforeCompiledILBytes == 0, $"Before Compiled IL bytes not eqeual to 0! ({beforeCompiledILBytes})");
+            Assert.True(beforeCompiledMethodCount == 0, $"Before Compiled method count not eqeual to 0! ({beforeCompiledMethodCount})");
+
+            Assert.True(afterCompilationTime == TimeSpan.Zero, $"After Compilation time not eqeual to 0! ({afterCompilationTime})");
+            Assert.True(afterCompiledILBytes == 0, $"After Compiled IL bytes not eqeual to 0! ({afterCompiledILBytes})");
+            Assert.True(afterCompiledMethodCount == 0, $"After Compiled method count not eqeual to 0! ({afterCompiledMethodCount})");
         }
 
         [Fact]
         [SkipOnMono("Mono does not track thread specific JIT information")]
         public void JitInfoCurrentThreadIsPopulated()
         {
-            TimeSpan t1_compilationTime = TimeSpan.Zero;
-            long t1_compiledILBytes = 0;
-            long t1_compiledMethodCount = 0;
+            TimeSpan t1_beforeCompilationTime = TimeSpan.Zero;
+            long t1_beforeCompiledILBytes = 0;
+            long t1_beforeCompiledMethodCount = 0;
 
-            TimeSpan t2_compilationTime = TimeSpan.Zero;
-            long t2_compiledILBytes = 0;
-            long t2_compiledMethodCount = 0;
+            TimeSpan t1_afterCompilationTime = TimeSpan.Zero;
+            long t1_afterCompiledILBytes = 0;
+            long t1_afterCompiledMethodCount = 0;
 
-            var t1 = new Thread(() => {
-                Func<string> theFunc = () => "JIT compile this!";
-                Assert.True(theFunc().Equals("JIT compile this!"));
-                t1_compilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
-                t1_compiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
-                t1_compiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
-            });
+            TimeSpan t2_beforeCompilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
+            long t2_beforeCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
+            long t2_beforeCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
 
-            var t2 = new Thread(() => {
-                Func<string> theFunc2 = () => "Also JIT compile this!";
-                Assert.True(theFunc2().Equals("Also JIT compile this!"));
-                t2_compilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
-                t2_compiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
-                t2_compiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
+            var t1 = new Thread(() => {
+                t1_beforeCompilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
+                t1_beforeCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
+                t1_beforeCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
+                Func<string> theFunc2 = () => "JIT compile this!";
+                Assert.True(theFunc2().Equals("JIT compile this!"));
+                t1_afterCompilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
+                t1_afterCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
+                t1_afterCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
             });
 
             t1.Start();
-            t2.Start();
             t1.Join();
-            t2.Join();
 
-            Assert.True(t1_compilationTime > TimeSpan.Zero, $"Thread 1 compilation time not greater than 0! ({t1_compilationTime})");
-            Assert.True(t1_compiledILBytes > 0, $"Thread 1 compiled IL bytes not greater than 0! ({t1_compiledILBytes})");
-            Assert.True(t1_compiledMethodCount > 0, $"Thread 1 compiled method count not greater than 0! ({t1_compiledMethodCount})");
+            Func<string> theFunc = () => "JIT compile this!";
+            Assert.True(theFunc().Equals("JIT compile this!"));
+
+            TimeSpan t2_afterCompilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
+            long t2_afterCompiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
+            long t2_afterCompiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
+
+            Assert.True(t2_beforeCompilationTime > TimeSpan.Zero, $"Thread 2 Compilation time not greater than 0! ({t2_beforeCompilationTime})");
+            Assert.True(t2_beforeCompiledILBytes > 0, $"Thread 2 Compiled IL bytes not greater than 0! ({t2_beforeCompiledILBytes})");
+            Assert.True(t2_beforeCompiledMethodCount > 0, $"Thread 2 Compiled method count not greater than 0! ({t2_beforeCompiledMethodCount})");
+
+            Assert.True(t2_afterCompilationTime > t2_beforeCompilationTime, $"CompilationTime: after not greater than before! (after: {t2_afterCompilationTime}, before: {t2_beforeCompilationTime})");
+            Assert.True(t2_afterCompiledILBytes > t2_beforeCompiledILBytes, $"Compiled IL bytes: after not greater than before! (after: {t2_afterCompiledILBytes}, before: {t2_beforeCompiledILBytes})");
+            Assert.True(t2_afterCompiledMethodCount > t2_beforeCompiledMethodCount, $"Compiled method count: after not greater than before! (after: {t2_afterCompiledMethodCount}, before: {t2_beforeCompiledMethodCount})");
+
+            Assert.True(t1_beforeCompilationTime > TimeSpan.Zero, $"Thread 1 before compilation time not greater than 0! ({t1_beforeCompilationTime})");
+            Assert.True(t1_beforeCompiledILBytes > 0, $"Thread 1 before compiled IL bytes not greater than 0! ({t1_beforeCompiledILBytes})");
+            Assert.True(t1_beforeCompiledMethodCount > 0, $"Thread 1 before compiled method count not greater than 0! ({t1_beforeCompiledMethodCount})");
+
+            Assert.True(t1_afterCompilationTime > t1_beforeCompilationTime, $"Thread 1 compilation time: after not greater than before! (after: {t1_afterCompilationTime}, before: {t1_beforeCompilationTime})");
+            Assert.True(t1_afterCompiledILBytes > t1_beforeCompiledILBytes, $"Thread 1 compiled IL bytes: after not greater than before! (after: {t1_afterCompiledILBytes}, before: {t1_beforeCompiledILBytes})");
+            Assert.True(t1_afterCompiledMethodCount > t1_beforeCompiledMethodCount, $"Thread 1 compiled method count: after not greater than before! (after: {t1_afterCompiledMethodCount}, before: {t1_beforeCompiledMethodCount})");
+
+            Assert.True(t1_afterCompilationTime != t2_afterCompilationTime, $"Thread 1 compilation time: equal to other thread! (t1: {t1_afterCompilationTime}, t2: {t2_beforeCompilationTime})");
+            Assert.True(t1_afterCompiledILBytes != t2_afterCompiledILBytes, $"Thread 1 compiled IL bytes: equal to other thread! (t1: {t1_afterCompiledILBytes}, t2: {t2_beforeCompiledILBytes})");
+            Assert.True(t1_afterCompiledMethodCount != t2_afterCompiledMethodCount, $"Thread 1 compiled method count: equal to other thread! (t1: {t1_afterCompiledMethodCount}, t2: {t2_beforeCompiledMethodCount})");
+        }
+
+        [Fact]
+        [SkipOnCoreClr("CoreCLR does track thread specific JIT information")]
+        public void JitInfoCurrentThreadIsNotPopulated()
+        {
+            TimeSpan compilationTime = TimeSpan.Zero;
+            long compiledILBytes = 0;
+            long compiledMethodCount = 0;
+
+            compilationTime = System.Runtime.JitInfo.GetCompilationTime(currentThread: true);
+            compiledILBytes = System.Runtime.JitInfo.GetCompiledILBytes(currentThread: true);
+            compiledMethodCount = System.Runtime.JitInfo.GetCompiledMethodCount(currentThread: true);
 
-            Assert.True(t2_compilationTime > TimeSpan.Zero, $"Thread 2 compilation time not greater than 0! ({t2_compilationTime})");
-            Assert.True(t2_compiledILBytes > 0, $"Thread 2 compiled IL bytes not greater than 0! ({t2_compiledILBytes})");
-            Assert.True(t2_compiledMethodCount > 0, $"Thread 3 compiled method count not greater than 0! ({t2_compiledMethodCount}");
+            Assert.True(compilationTime == TimeSpan.Zero, $"compilation time not equal to 0! ({compilationTime})");
+            Assert.True(compiledILBytes == 0, $"compiled IL bytes not equal to 0! ({compiledILBytes})");
+            Assert.True(compiledMethodCount == 0, $"compiled method count not equal to 0! ({compiledMethodCount})");
         }
     }
 }