DYN1805 Validate the performance tracking system by using validated improvements Part II (#9665)

reddyashish · web-flow · commit b859c7ba73fa · 2019-04-23T00:35:55.000-04:00
* Move Engine TraceUtils methods to DynamoServices with ThreadStatic backing

* Convert GetTrace and SetTrace to user ThreadStatic mechanism

* Remove Engine TraceUtils

* Clean up tests to remove thread local storage reference

* Clean up callsite Trace workflow

* Remove expression body syntax

* Remove inline out declaration

* Obseleting the class instead of removing it

* Sort usings.

* Removing the TODO comment that is not valid anymore
diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs
@@ -1,3 +1,11 @@
+using DynamoServices;
+using ProtoCore.DSASM;
+using ProtoCore.Exceptions;
+using ProtoCore.Lang;
+using ProtoCore.Lang.Replication;
+using ProtoCore.Properties;
+using ProtoCore.Runtime;
+using ProtoCore.Utils;
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
@@ -8,14 +16,8 @@
 using System.Runtime.Serialization;
 using System.Runtime.Serialization.Formatters.Soap;
 using System.Text;
-using ProtoCore.DSASM;
-using ProtoCore.Exceptions;
-using ProtoCore.Lang;
-using ProtoCore.Lang.Replication;
-using ProtoCore.Properties;
-using ProtoCore.Runtime;
-using ProtoCore.Utils;
 using StackFrame = ProtoCore.DSASM.StackFrame;
+using Validity = ProtoCore.Utils.Validity;
 using WarningID = ProtoCore.Runtime.WarningID;
 
 namespace ProtoCore
@@ -1894,11 +1896,7 @@ private StackValue ExecWithZeroRI(List<FunctionEndPoint> functionEndPoint, Conte
             if (traceD != null)
             {
                 //There was data associated with the previous execution, push this into the TLS
-
-                Dictionary<string, ISerializable> dataDict = new Dictionary<string, ISerializable>();
-                dataDict.Add(TRACE_KEY, traceD);
-
-                TraceUtils.SetObjectToTLS(dataDict);
+                TraceUtils.SetTraceData(TRACE_KEY, traceD);
             }
             else
             {
@@ -1912,16 +1910,18 @@ private StackValue ExecWithZeroRI(List<FunctionEndPoint> functionEndPoint, Conte
             if (ret.IsNull)
             {
                 //wipe the trace cache
-                TraceUtils.ClearTLSKey(TRACE_KEY);
+                TraceUtils.ClearAllKnownTLSKeys();
+                newTraceData.Data = null;
             }
-
-            //TLS -> TraceCache
-            Dictionary<string, ISerializable> traceRet = TraceUtils.GetObjectFromTLS();
-
-            if (traceRet.ContainsKey(TRACE_KEY))
+            else
             {
-                var val = traceRet[TRACE_KEY];
-                newTraceData.Data = val;
+                //TLS -> TraceCache
+                var traceRet = TraceUtils.GetTraceData(TRACE_KEY);
+
+                if (traceRet != null)
+                {
+                    newTraceData.Data = traceRet;
+                }
             }
 
             // An explicit call requires return coercion at the return instruction
diff --git a/src/Engine/ProtoCore/Lang/TraceUtils.cs b/src/Engine/ProtoCore/Lang/TraceUtils.cs
@@ -5,6 +5,7 @@
 
 namespace ProtoCore.Lang
 {
+    [Obsolete("This class is not used anymore, use the same class from Dynamo.Services namespace", false)]
     public static class TraceUtils
     {
 
@@ -93,3 +94,4 @@ public static void ClearAllKnownTLSKeys()
 
     }
 }
+﻿
diff --git a/src/Engine/ProtoCore/ProtoCore.csproj b/src/Engine/ProtoCore/ProtoCore.csproj
@@ -149,7 +149,6 @@
     <Compile Include="Lang\FunctionPointerEvaluator.cs" />
     <Compile Include="Lang\Replication\ReplicationInstruction.cs" />
     <Compile Include="Lang\Replication\Replicator.cs" />
-    <Compile Include="Lang\TraceUtils.cs" />
     <Compile Include="Namespace\ElementRewriter.cs" />
     <Compile Include="Namespace\ElementResolver.cs" />
     <Compile Include="Namespace\SymbolTable.cs" />
diff --git a/src/NodeServices/Properties/AssemblyInfo.cs b/src/NodeServices/Properties/AssemblyInfo.cs
@@ -17,4 +17,6 @@
 [assembly: InternalsVisibleTo("TestServices")]
 [assembly: InternalsVisibleTo("ProtoTestFx")]
 [assembly: InternalsVisibleTo("DynamoCoreTests")]
-[assembly: InternalsVisibleTo("DynamoWPFCLI")]
+[assembly: InternalsVisibleTo("DynamoWPFCLI")]
+[assembly: InternalsVisibleTo("ProtoTest")]
+[assembly: InternalsVisibleTo("IntegrationTests")]
diff --git a/src/NodeServices/TraceSupport.cs b/src/NodeServices/TraceSupport.cs
@@ -1,6 +1,6 @@
 ﻿using System;
+using System.Collections.Generic;
 using System.Runtime.Serialization;
-using System.Threading;
 
 namespace DynamoServices
 {
@@ -17,25 +17,68 @@ public class RegisterForTraceAttribute : Attribute
     /// </summary>
     public static class TraceUtils
     {
+        internal const string __TEMP_REVIT_TRACE_ID = "{0459D869-0C72-447F-96D8-08A7FB92214B}-REVIT";
+        // ReSharper restore InconsistentNaming
+
+        [ThreadStatic] private static Dictionary<String, ISerializable> _localStorageSlot; //= new Dictionary<string, ISerializable>();
+
+        internal static Dictionary<String, ISerializable> LocalStorageSlot
+        {
+            get
+            {
+                return _localStorageSlot ?? (_localStorageSlot = new Dictionary<string, ISerializable>());
+            }
+            set
+            {
+                _localStorageSlot = value;
+            }
+        }
+
+        /// <summary>
+        /// Returns a list of the keys bound to trace elements
+        /// This should be extracted from the attribute on the methods
+        /// </summary>
+        /// <returns></returns>
+        internal static List<String> TEMP_GetTraceKeys()
+        {
+            //TODO:Luke Extract this from RequiresTraceAttribute
+
+            return new List<string>() { __TEMP_REVIT_TRACE_ID };
+        }
+
+        /// <summary>
+        /// Clear a specific key
+        /// </summary>
+        /// <param name="key"></param>
+        internal static void ClearTLSKey(string key)
+        {
+            LocalStorageSlot.Remove(key);
+        }
+
+        /// <summary>
+        /// Clear the named slots for all the know keys
+        /// </summary>
+        internal static void ClearAllKnownTLSKeys()
+        {
+            LocalStorageSlot.Clear();
+        }
+
         /// <summary>
         /// Returns the data that is bound to a particular key
         /// </summary>
         /// <param name="key"></param>
         /// <returns></returns>
         public static ISerializable GetTraceData(string key)
         {
-            Object data = Thread.GetData(Thread.GetNamedDataSlot(key));
-
-            //Null is ok
-            if (data == null)
+            ISerializable data;  
+            if (!LocalStorageSlot.TryGetValue(key, out data))
+            {
                 return null;
-
-            var ret = data as ISerializable;
-            if (ret != null)
-                return ret;
-       
-            //Data, that was not serializable is not
-            throw new InvalidOperationException("Data in Named slot was not serializable");
+            }
+            else
+            {
+                return data;
+            }
         }
 
         /// <summary>
@@ -45,7 +88,14 @@ public static ISerializable GetTraceData(string key)
         /// <param name="value"></param>
         public static void SetTraceData(string key, ISerializable value)
         {
-            Thread.SetData(Thread.GetNamedDataSlot(key), value);
+            if (LocalStorageSlot.ContainsKey(key))
+            {
+                LocalStorageSlot[key] = value;
+            }
+            else
+            {
+                LocalStorageSlot.Add(key, value);
+            }
         }
     }
 }
diff --git a/test/Engine/ProtoTest/TD/TraceUtilsTests.cs b/test/Engine/ProtoTest/TD/TraceUtilsTests.cs
@@ -1,8 +1,7 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Runtime.Serialization;
+﻿using DynamoServices;
 using NUnit.Framework;
-using ProtoCore.Lang;
+using System;
+using System.Runtime.Serialization;
 
 namespace ProtoTest.TD
 {
@@ -36,24 +35,20 @@ public static void SetGetTest()
         {
 
 
-            List<String> keys = TraceUtils.TEMP_GetTraceKeys();
+            var key = TraceUtils.TEMP_GetTraceKeys()[0];
 
             SerializableString testStr1 = new 
                 SerializableString("{0955D962-2936-4FB2-AAB3-635C6FF6E0AD}");
 
             SerializableString testStr2 = new 
                 SerializableString("{2D7FE0ED-56F3-47A4-9BAA-8DF570170D97}");
 
-
-            Dictionary<String, ISerializable> data = new Dictionary<string, ISerializable>();
-            data.Add(keys[0], testStr1);
-
-            TraceUtils.SetObjectToTLS(data);
+            TraceUtils.SetTraceData(key, testStr1);
 
             //Set complete, readback test
 
-            Dictionary<String, ISerializable> readback = TraceUtils.GetObjectFromTLS();
-            Assert.IsTrue(((SerializableString)readback[keys[0]]).Payload == testStr1.Payload);
+            ISerializable readback = TraceUtils.GetTraceData(key);
+            Assert.IsTrue(readback == testStr1);
         }
 
 
@@ -62,37 +57,29 @@ public static void SetGetTest()
         public static void OverwriteTest()
         {
 
-            List<String> keys = TraceUtils.TEMP_GetTraceKeys();
+            var key = TraceUtils.TEMP_GetTraceKeys()[0];
 
             SerializableString testStr1 = new
                 SerializableString("{0955D962-2936-4FB2-AAB3-635C6FF6E0AD}");
 
             SerializableString testStr2 = new
                 SerializableString("{2D7FE0ED-56F3-47A4-9BAA-8DF570170D97}");
 
-
-            var data = new Dictionary<string, ISerializable>();
-            data.Add(keys[0], testStr1);
-
-            var data2 = new Dictionary<string, ISerializable>();
-            data2.Add(keys[0], testStr2);
-
-
-            TraceUtils.SetObjectToTLS(data);
+            TraceUtils.SetTraceData(key, testStr1);
 
             //Set complete, readback test
 
-            var readback = TraceUtils.GetObjectFromTLS();
-            Assert.IsTrue(((SerializableString)readback[keys[0]]).Payload == testStr1.Payload);
+            ISerializable readback = TraceUtils.GetTraceData(key);
+            Assert.IsTrue(readback == testStr1);
 
 
 
-            TraceUtils.SetObjectToTLS(data2);
+            TraceUtils.SetTraceData(key, testStr2);
 
             //Set complete, readback test
 
-            readback = TraceUtils.GetObjectFromTLS();
-            Assert.IsTrue(((SerializableString)readback[keys[0]]).Payload == testStr2.Payload);
+            readback = TraceUtils.GetTraceData(key);
+            Assert.IsTrue(readback == testStr2);
         }
 
 
@@ -101,32 +88,24 @@ public static void OverwriteTest()
         public static void OverwriteNullTest()
         {
 
-            List<String> keys = TraceUtils.TEMP_GetTraceKeys();
+            var key = TraceUtils.TEMP_GetTraceKeys()[0];
 
             SerializableString testStr1 = new
                 SerializableString("{0955D962-2936-4FB2-AAB3-635C6FF6E0AD}");
 
-
-            Dictionary<String, ISerializable> data = new Dictionary<string, ISerializable>();
-            data.Add(keys[0], testStr1);
-
-
-            TraceUtils.SetObjectToTLS(data);
+            TraceUtils.SetTraceData(key, testStr1);
 
             //Set complete, readback test
 
-            var readback = TraceUtils.GetObjectFromTLS();
-            Assert.IsTrue(((SerializableString)readback[keys[0]]).Payload == testStr1.Payload);
-
-
-            data[keys[0]] = null;
+            ISerializable readback = TraceUtils.GetTraceData(key);
+            Assert.IsTrue(readback == testStr1);
 
-            TraceUtils.SetObjectToTLS(data);
+            TraceUtils.SetTraceData(key, null);
 
             //Set complete, readback test
 
-            readback = TraceUtils.GetObjectFromTLS();
-            Assert.IsTrue((ISerializable)readback[keys[0]] == null);
+            readback = TraceUtils.GetTraceData(key);
+            Assert.IsTrue(readback == null);
         }
 
 
diff --git a/test/System/IntegrationTests/CallsiteRegen.cs b/test/System/IntegrationTests/CallsiteRegen.cs
@@ -1,10 +1,10 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Threading;
+﻿using DynamoServices;
 using NUnit.Framework;
 using ProtoScript.Runners;
 using ProtoTestFx.TD;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 
 namespace IntegrationTests
 {
@@ -31,7 +31,7 @@ public void Setup()
             [TearDown]
             public static void TLSCleanup()
             {
-                Thread.FreeNamedDataSlot(__TEMP_REVIT_TRACE_ID);
+                TraceUtils.ClearAllKnownTLSKeys();
             }
 
 
diff --git a/test/System/IntegrationTests/IncrementingTraceTests.cs b/test/System/IntegrationTests/IncrementingTraceTests.cs
@@ -1,10 +1,11 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Threading;
+﻿using DynamoServices;
 using NUnit.Framework;
 using ProtoScript.Runners;
 using ProtoTestFx.TD;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
 
 namespace IntegrationTests
 {
@@ -35,8 +36,8 @@ public void TestFixup()
         public static void TLSCleanup()
         {
 
-            Thread.FreeNamedDataSlot(__TEMP_REVIT_TRACE_ID);
-           
+            TraceUtils.ClearAllKnownTLSKeys();
+
         }
 
         [Test]
diff --git a/test/System/IntegrationTests/MinimalTraceTests.cs b/test/System/IntegrationTests/MinimalTraceTests.cs
@@ -1,4 +1,4 @@
-﻿using System.Threading;
+using DynamoServices;
 using FFITarget;
 using NUnit.Framework;
 using ProtoTestFx.TD;
@@ -20,7 +20,7 @@ public void Setup()
         [TearDown]
         public static void TLSCleanup()
         {
-            Thread.FreeNamedDataSlot(__TEMP_REVIT_TRACE_ID);    
+            TraceUtils.ClearAllKnownTLSKeys();
         }
 
         [Test]
diff --git a/test/System/IntegrationTests/TestWrapperCleanup.cs b/test/System/IntegrationTests/TestWrapperCleanup.cs

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`
`6`	`6`	`namespace ProtoCore.Lang`
`7`	`7`	`{`
	`8`	`+ [Obsolete("This class is not used anymore, use the same class from Dynamo.Services namespace", false)]`
`8`	`9`	`public static class TraceUtils`
`9`	`10`	`{`
`10`	`11`
`@@ -93,3 +94,4 @@ public static void ClearAllKnownTLSKeys()`
`93`	`94`
`94`	`95`	`}`
`95`	`96`	`}`
	`97`	`+`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using System.Threading;`
	`1`	`+using DynamoServices;`
`2`	`2`	`using FFITarget;`
`3`	`3`	`using NUnit.Framework;`
`4`	`4`	`using ProtoTestFx.TD;`
`@@ -20,7 +20,7 @@ public void Setup()`
`20`	`20`	`[TearDown]`
`21`	`21`	`public static void TLSCleanup()`
`22`	`22`	`{`
`23`		`- Thread.FreeNamedDataSlot(__TEMP_REVIT_TRACE_ID);`
	`23`	`+ TraceUtils.ClearAllKnownTLSKeys();`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`[Test]`