From 992cd930658328e511a4bb50fc7d15801a1abd3a Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Fri, 30 Apr 2021 04:52:13 +0200 Subject: [PATCH 1/5] More EventListener overhead reductions --- .../System/Diagnostics/Tracing/EventSource.cs | 304 +++++++++++------- 1 file changed, 179 insertions(+), 125 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 57f0a5bc92c57d..6a098dbfee2a1e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -1692,126 +1692,134 @@ private static Guid GenerateGuidFromName(string name) return new Guid(bytes); } - private static unsafe object? DecodeObject(Type dataType, ref EventSource.EventData* data) + private static unsafe object? DecodeObject(Type dataType, ref EventData* data) { // TODO FIX : We use reflection which in turn uses EventSource, right now we carefully avoid // the recursion, but can we do this in a robust way? IntPtr dataPointer = data->DataPointer; - // advance to next EventData in array - ++data; - Again: - if (dataType == typeof(IntPtr)) + switch (RuntimeTypeHandle.GetCorElementType((RuntimeType)dataType)) { - return *((IntPtr*)dataPointer); - } - else if (dataType == typeof(int)) - { - return *((int*)dataPointer); - } - else if (dataType == typeof(uint)) - { - return *((uint*)dataPointer); - } - else if (dataType == typeof(long)) - { - return *((long*)dataPointer); - } - else if (dataType == typeof(ulong)) - { - return *((ulong*)dataPointer); - } - else if (dataType == typeof(byte)) - { - return *((byte*)dataPointer); - } - else if (dataType == typeof(sbyte)) - { - return *((sbyte*)dataPointer); - } - else if (dataType == typeof(short)) - { - return *((short*)dataPointer); - } - else if (dataType == typeof(ushort)) - { - return *((ushort*)dataPointer); - } - else if (dataType == typeof(float)) - { - return *((float*)dataPointer); - } - else if (dataType == typeof(double)) - { - return *((double*)dataPointer); - } - else if (dataType == typeof(decimal)) - { - return *((decimal*)dataPointer); - } - else if (dataType == typeof(bool)) - { - // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. - return *((int*)dataPointer) == 1; - } - else if (dataType == typeof(Guid)) - { - return *((Guid*)dataPointer); - } - else if (dataType == typeof(char)) - { - return *((char*)dataPointer); - } - else if (dataType == typeof(DateTime)) - { - long dateTimeTicks = *((long*)dataPointer); - return DateTime.FromFileTimeUtc(dateTimeTicks); - } - else if (dataType == typeof(byte[])) - { - // byte[] are written to EventData* as an int followed by a blob - int cbSize = *((int*)dataPointer); - byte[] blob = new byte[cbSize]; - dataPointer = data->DataPointer; - data++; - for (int i = 0; i < cbSize; ++i) - blob[i] = *((byte*)(dataPointer + i)); - return blob; - } - else if (dataType == typeof(byte*)) - { - // TODO: how do we want to handle this? For now we ignore it... - return null; - } - else - { - if (dataType.IsEnum) - { - dataType = Enum.GetUnderlyingType(dataType); + case CorElementType.ELEMENT_TYPE_I4: + Debug.Assert(data->Size == 4); + return *(int*)dataPointer; - // Enums less than 4 bytes in size should be treated as int. - switch (Type.GetTypeCode(dataType)) + case CorElementType.ELEMENT_TYPE_I8: + Debug.Assert(data->Size == 8); + return *(long*)dataPointer; + + case CorElementType.ELEMENT_TYPE_STRING: + goto default; + + case CorElementType.ELEMENT_TYPE_I: + Debug.Assert(data->Size == IntPtr.Size); + return *(IntPtr*)dataPointer; + + case CorElementType.ELEMENT_TYPE_U1: + Debug.Assert(data->Size == 1); + return *(byte*)dataPointer; + + case CorElementType.ELEMENT_TYPE_VALUETYPE: + if (data->Size == 16) { - case TypeCode.Byte: - case TypeCode.SByte: - case TypeCode.Int16: - case TypeCode.UInt16: - dataType = typeof(int); - break; + if (dataType == typeof(Guid)) + { + return *(Guid*)dataPointer; + } + else if (dataType == typeof(decimal)) + { + return *(decimal*)dataPointer; + } } - goto Again; - } + else if (dataType == typeof(DateTime)) + { + Debug.Assert(data->Size == 8); + return *(DateTime*)dataPointer; + } + else if (dataType.IsEnum) + { + if (data->Size == 8) + { + return *(long*)dataPointer; + } + else + { + // Enums less than 4 bytes in size should be treated as int. + return *(int*)dataPointer; + } + } + Debug.Assert(dataType != typeof(Guid) && dataType != typeof(decimal), + $"Invalid size passed for {dataType.Name} ({data->Size})"); + goto default; - // Everything else is marshaled as a string. - // ETW strings are NULL-terminated, so marshal everything up to the first - // null in the string. - if (dataPointer == IntPtr.Zero) - { - return null; - } + case CorElementType.ELEMENT_TYPE_U4: + Debug.Assert(data->Size == 4); + return *(uint*)dataPointer; + + case CorElementType.ELEMENT_TYPE_U8: + Debug.Assert(data->Size == 8); + return *(ulong*)dataPointer; + + case CorElementType.ELEMENT_TYPE_I1: + Debug.Assert(data->Size == 1); + return *(sbyte*)dataPointer; + + case CorElementType.ELEMENT_TYPE_I2: + Debug.Assert(data->Size == 2); + return *(short*)dataPointer; + + case CorElementType.ELEMENT_TYPE_U2: + Debug.Assert(data->Size == 2); + return *(ushort*)dataPointer; + + case CorElementType.ELEMENT_TYPE_R4: + Debug.Assert(data->Size == 4); + return *(float*)dataPointer; + + case CorElementType.ELEMENT_TYPE_R8: + Debug.Assert(data->Size == 8); + return *(double*)dataPointer; + + case CorElementType.ELEMENT_TYPE_BOOLEAN: + Debug.Assert(data->Size == 4); + // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. + return *(int*)dataPointer == 1; + + case CorElementType.ELEMENT_TYPE_CHAR: + Debug.Assert(data->Size == 2); + return *(char*)dataPointer; + + case CorElementType.ELEMENT_TYPE_PTR: + if (dataType == typeof(byte*)) + { + // TODO: how do we want to handle this? For now we ignore it... + return null; + } + goto default; + + case CorElementType.ELEMENT_TYPE_SZARRAY: + if (dataType == typeof(byte[])) + { + // byte[] are written to EventData* as an int followed by a blob + int size = *(int*)dataPointer; + byte[] blob = new byte[size]; + data++; + dataPointer = data->DataPointer; + for (int i = 0; i < blob.Length; i++) + { + blob[i] = *(byte*)(dataPointer + i); + } + return blob; + } + goto default; - return new string((char*)dataPointer); + default: + // Everything else is marshaled as a string. + // ETW strings are NULL-terminated, so marshal everything up to the first + // null in the string. + Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); + return dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); } } @@ -2029,8 +2037,7 @@ private unsafe void WriteToAllListeners(int eventId, Guid* activityID, Guid* chi ReportOutOfBandMessage(SR.Format(SR.EventSource_EventParametersMismatch, eventId, eventDataCount, metadata.Parameters.Length)); } - object?[]? args; - + object?[] args; if (eventDataCount == 0) { args = Array.Empty(); @@ -2038,11 +2045,31 @@ private unsafe void WriteToAllListeners(int eventId, Guid* activityID, Guid* chi else { ParameterInfo[] parameters = metadata.Parameters; - args = new object[Math.Min(eventDataCount, parameters.Length)]; - EventSource.EventData* dataPtr = data; - for (int i = 0; i < args.Length; i++) + args = new object?[Math.Min(eventDataCount, parameters.Length)]; + + if (metadata.AllParametersAreString) { - args[i] = DecodeObject(parameters[i].ParameterType, ref dataPtr); + for (int i = 0; i < args.Length; i++, data++) + { + IntPtr dataPointer = data->DataPointer; + Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); + args[i] = dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); + } + } + else if (metadata.AllParametersAreInt32) + { + for (int i = 0; i < args.Length; i++, data++) + { + Debug.Assert(data->Size == 4); + args[i] = *(int*)data->DataPointer; + } + } + else + { + for (int i = 0; i < args.Length; i++, data++) + { + args[i] = DecodeObject(parameters[i].ParameterType, ref data); + } } } @@ -2428,6 +2455,8 @@ internal partial struct EventMetadata public string? Message; // If the event has a message associated with it, this is it. public ParameterInfo[] Parameters; // TODO can we remove? public int EventListenerParameterCount; + public bool AllParametersAreString; + public bool AllParametersAreInt32; public TraceLoggingEventTypes? TraceLoggingEventTypes; public EventActivityOptions ActivityOptions; @@ -3390,7 +3419,9 @@ private static void AddEventDescriptor( eventData = newValues; } - eventData[eventAttribute.EventId].Descriptor = new EventDescriptor( + ref EventMetadata metadata = ref eventData[eventAttribute.EventId]; + + metadata.Descriptor = new EventDescriptor( eventAttribute.EventId, eventAttribute.Version, #if FEATURE_MANAGED_ETW_CHANNELS @@ -3403,26 +3434,49 @@ private static void AddEventDescriptor( (int)eventAttribute.Task, unchecked((long)((ulong)eventAttribute.Keywords | SessionMask.All.ToEventKeywords()))); - eventData[eventAttribute.EventId].Tags = eventAttribute.Tags; - eventData[eventAttribute.EventId].Name = eventName; - eventData[eventAttribute.EventId].Parameters = eventParameters; - eventData[eventAttribute.EventId].Message = eventAttribute.Message; - eventData[eventAttribute.EventId].ActivityOptions = eventAttribute.ActivityOptions; - eventData[eventAttribute.EventId].HasRelatedActivityID = hasRelatedActivityID; - eventData[eventAttribute.EventId].EventHandle = IntPtr.Zero; + metadata.Tags = eventAttribute.Tags; + metadata.Name = eventName; + metadata.Parameters = eventParameters; + metadata.Message = eventAttribute.Message; + metadata.ActivityOptions = eventAttribute.ActivityOptions; + metadata.HasRelatedActivityID = hasRelatedActivityID; + metadata.EventHandle = IntPtr.Zero; // We represent a byte[] with 2 EventData entries: an integer denoting the length and a blob of bytes in the data pointer. // This causes a spurious warning because eventDataCount is off by one for the byte[] case. // When writing to EventListeners, we want to check that the number of parameters is correct against the byte[] case. int eventListenerParameterCount = eventParameters.Length; + bool allParametersAreInt32 = true; + bool allParametersAreString = true; + foreach (ParameterInfo parameter in eventParameters) { - if (parameter.ParameterType == typeof(byte[])) + Type dataType = parameter.ParameterType; + if (dataType == typeof(string)) + { + allParametersAreInt32 = false; + } + else if (dataType == typeof(int) || + (dataType.IsEnum && Type.GetTypeCode(dataType.GetEnumUnderlyingType()) <= TypeCode.UInt32)) + { + // Int32 or an enum with a 1/2/4 byte backing type + allParametersAreString = false; + } + else { - eventListenerParameterCount++; + if (dataType == typeof(byte[])) + { + eventListenerParameterCount++; + } + + allParametersAreInt32 = false; + allParametersAreString = false; } } - eventData[eventAttribute.EventId].EventListenerParameterCount = eventListenerParameterCount; + + metadata.AllParametersAreInt32 = allParametersAreInt32; + metadata.AllParametersAreString = allParametersAreString; + metadata.EventListenerParameterCount = eventListenerParameterCount; } // Helper used by code:CreateManifestAndDescriptors that trims the m_eventData array to the correct From 0dadbd6af0e0c1d9e898ce11ca1fae4aa0251f77 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Fri, 30 Apr 2021 23:55:28 +0200 Subject: [PATCH 2/5] Use Type.GetTypeCode instead of RuntimeTypeHandle.GetCorElementType --- .../BasicEventSourceTest/TestsWriteEvent.cs | 3 + .../System/Diagnostics/Tracing/EventSource.cs | 226 +++++++++--------- 2 files changed, 119 insertions(+), 110 deletions(-) diff --git a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsWriteEvent.cs b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsWriteEvent.cs index f37106dd0e53ee..683ee1a67317b3 100644 --- a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsWriteEvent.cs +++ b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsWriteEvent.cs @@ -588,6 +588,9 @@ private void Test_WriteEvent_ByteArray(bool useSelfDescribingEvents, Listener li { Assert.Equal(3, evt.PayloadCount); byte[] retBlob = (byte[])evt.PayloadValue(1, "blob"); + Assert.Equal(4, retBlob.Length); + Assert.Equal(retBlob[0], blob[2]); + Assert.Equal(retBlob[3], blob[2 + 3]); Assert.Equal(1001, (int)evt.PayloadValue(2, "n")); } })); diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 6a098dbfee2a1e..5826051a539ca1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -1698,129 +1698,135 @@ private static Guid GenerateGuidFromName(string name) // the recursion, but can we do this in a robust way? IntPtr dataPointer = data->DataPointer; + Debug.Assert(dataPointer != IntPtr.Zero || data->Size == 0); - switch (RuntimeTypeHandle.GetCorElementType((RuntimeType)dataType)) + if (dataType == typeof(string)) { - case CorElementType.ELEMENT_TYPE_I4: - Debug.Assert(data->Size == 4); - return *(int*)dataPointer; - - case CorElementType.ELEMENT_TYPE_I8: - Debug.Assert(data->Size == 8); - return *(long*)dataPointer; + goto String; + } + else if (dataType == typeof(int)) + { + Debug.Assert(data->Size == 4); + return *(int*)dataPointer; + } - case CorElementType.ELEMENT_TYPE_STRING: - goto default; + TypeCode typeCode = Type.GetTypeCode(dataType); + int size = data->Size; - case CorElementType.ELEMENT_TYPE_I: - Debug.Assert(data->Size == IntPtr.Size); + if (size == 4) + { + if ((uint)(typeCode - TypeCode.SByte) <= TypeCode.UInt16 - TypeCode.SByte) + { + Debug.Assert(dataType.IsEnum); + // Enums less than 4 bytes in size should be treated as int. + return *(int*)dataPointer; + } + else if (typeCode == TypeCode.UInt32) + { + return *(uint*)dataPointer; + } + else if (typeCode == TypeCode.Single) + { + return *(float*)dataPointer; + } + else if (typeCode == TypeCode.Boolean) + { + // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. + return *(int*)dataPointer == 1; + } + else if (dataType == typeof(byte[])) + { + // byte[] are written to EventData* as an int followed by a blob + Debug.Assert(*(int*)dataPointer == (data + 1)->Size); + data++; + dataPointer = data->DataPointer; + goto BytePtr; + } + else if (IntPtr.Size == 4 && dataType == typeof(IntPtr)) + { return *(IntPtr*)dataPointer; - - case CorElementType.ELEMENT_TYPE_U1: - Debug.Assert(data->Size == 1); + } + } + else if (size <= 2) + { + Debug.Assert(!dataType.IsEnum); + if (typeCode == TypeCode.Byte) + { + Debug.Assert(size == 1); return *(byte*)dataPointer; - - case CorElementType.ELEMENT_TYPE_VALUETYPE: - if (data->Size == 16) - { - if (dataType == typeof(Guid)) - { - return *(Guid*)dataPointer; - } - else if (dataType == typeof(decimal)) - { - return *(decimal*)dataPointer; - } - } - else if (dataType == typeof(DateTime)) - { - Debug.Assert(data->Size == 8); - return *(DateTime*)dataPointer; - } - else if (dataType.IsEnum) - { - if (data->Size == 8) - { - return *(long*)dataPointer; - } - else - { - // Enums less than 4 bytes in size should be treated as int. - return *(int*)dataPointer; - } - } - Debug.Assert(dataType != typeof(Guid) && dataType != typeof(decimal), - $"Invalid size passed for {dataType.Name} ({data->Size})"); - goto default; - - case CorElementType.ELEMENT_TYPE_U4: - Debug.Assert(data->Size == 4); - return *(uint*)dataPointer; - - case CorElementType.ELEMENT_TYPE_U8: - Debug.Assert(data->Size == 8); - return *(ulong*)dataPointer; - - case CorElementType.ELEMENT_TYPE_I1: - Debug.Assert(data->Size == 1); + } + else if (typeCode == TypeCode.SByte) + { + Debug.Assert(size == 1); return *(sbyte*)dataPointer; - - case CorElementType.ELEMENT_TYPE_I2: - Debug.Assert(data->Size == 2); + } + else if (typeCode == TypeCode.Int16) + { + Debug.Assert(size == 2); return *(short*)dataPointer; - - case CorElementType.ELEMENT_TYPE_U2: - Debug.Assert(data->Size == 2); + } + else if (typeCode == TypeCode.UInt16) + { + Debug.Assert(size == 2); return *(ushort*)dataPointer; - - case CorElementType.ELEMENT_TYPE_R4: - Debug.Assert(data->Size == 4); - return *(float*)dataPointer; - - case CorElementType.ELEMENT_TYPE_R8: - Debug.Assert(data->Size == 8); - return *(double*)dataPointer; - - case CorElementType.ELEMENT_TYPE_BOOLEAN: - Debug.Assert(data->Size == 4); - // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. - return *(int*)dataPointer == 1; - - case CorElementType.ELEMENT_TYPE_CHAR: - Debug.Assert(data->Size == 2); + } + else if (typeCode == TypeCode.Char) + { + Debug.Assert(size == 2); return *(char*)dataPointer; + } + } + else if (size == 8) + { + if (typeCode == TypeCode.Int64) + { + return *(long*)dataPointer; + } + else if (typeCode == TypeCode.UInt64) + { + return *(ulong*)dataPointer; + } + else if (typeCode == TypeCode.Double) + { + return *(double*)dataPointer; + } + else if (typeCode == TypeCode.DateTime) + { + return *(DateTime*)dataPointer; + } + else if (IntPtr.Size == 8 && dataType == typeof(IntPtr)) + { + return *(IntPtr*)dataPointer; + } + } + else + { + if (typeCode == TypeCode.Decimal) + { + Debug.Assert(size == 16); + return *(decimal*)dataPointer; + } + else if (dataType == typeof(Guid)) + { + Debug.Assert(size == 16); + return *(Guid*)dataPointer; + } + } - case CorElementType.ELEMENT_TYPE_PTR: - if (dataType == typeof(byte*)) - { - // TODO: how do we want to handle this? For now we ignore it... - return null; - } - goto default; + if (dataType != typeof(byte*)) + { + goto String; + } - case CorElementType.ELEMENT_TYPE_SZARRAY: - if (dataType == typeof(byte[])) - { - // byte[] are written to EventData* as an int followed by a blob - int size = *(int*)dataPointer; - byte[] blob = new byte[size]; - data++; - dataPointer = data->DataPointer; - for (int i = 0; i < blob.Length; i++) - { - blob[i] = *(byte*)(dataPointer + i); - } - return blob; - } - goto default; + BytePtr: + return new Span((byte*)dataPointer, data->Size).ToArray(); - default: - // Everything else is marshaled as a string. - // ETW strings are NULL-terminated, so marshal everything up to the first - // null in the string. - Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); - return dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); - } + String: + // Everything else is marshaled as a string. + // ETW strings are NULL-terminated, so marshal everything up to the first + // null in the string. + Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); + return dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); } // Finds the Dispatcher (which holds the filtering state), for a given dispatcher for the current From 906431408d7f7cd71243627187f48c800344d529 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Sun, 2 May 2021 22:28:16 +0200 Subject: [PATCH 3/5] Fix int32 enum case --- .../src/System/Diagnostics/Tracing/EventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 5826051a539ca1..e4b6cc0b3bdf51 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -1715,7 +1715,7 @@ private static Guid GenerateGuidFromName(string name) if (size == 4) { - if ((uint)(typeCode - TypeCode.SByte) <= TypeCode.UInt16 - TypeCode.SByte) + if ((uint)(typeCode - TypeCode.SByte) <= TypeCode.Int32 - TypeCode.SByte) { Debug.Assert(dataType.IsEnum); // Enums less than 4 bytes in size should be treated as int. From 872f68d4dc66799bf13d6cbad67999d652f62bdb Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Mon, 3 May 2021 00:20:03 +0200 Subject: [PATCH 4/5] Revert back from span to array --- .../System/Diagnostics/Tracing/EventSource.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index e4b6cc0b3bdf51..5686cc856829e8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -1819,13 +1819,22 @@ private static Guid GenerateGuidFromName(string name) } BytePtr: - return new Span((byte*)dataPointer, data->Size).ToArray(); + byte[] blob = new byte[data->Size]; + for (int i = 0; i < blob.Length; i++) + { + blob[i] = *((byte*)dataPointer + i); + } + return blob; String: // Everything else is marshaled as a string. // ETW strings are NULL-terminated, so marshal everything up to the first // null in the string. - Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); +#if DEBUG + Debug.Assert(data->Size % 2 == 0, "String size should be even"); + for (int i = 0; i < data->Size / 2 - 1; i++) Debug.Assert(*((char*)dataPointer + i) != 0, "String may not contain null chars"); + Debug.Assert(*((char*)dataPointer + data->Size / 2 - 1) == 0, "String must be null terminated"); +#endif return dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); } @@ -2058,7 +2067,11 @@ private unsafe void WriteToAllListeners(int eventId, Guid* activityID, Guid* chi for (int i = 0; i < args.Length; i++, data++) { IntPtr dataPointer = data->DataPointer; - Debug.Assert(data->Size % 2 == 0 && new Span((char*)dataPointer, data->Size >> 1).IndexOf('\0') == (data->Size >> 1) - 1); +#if DEBUG + Debug.Assert(data->Size % 2 == 0, "String size should be even"); + for (int j = 0; j < data->Size / 2 - 1; j++) Debug.Assert(*((char*)dataPointer + j) != 0, "String may not contain null chars"); + Debug.Assert(*((char*)dataPointer + data->Size / 2 - 1) == 0, "String must be null terminated"); +#endif args[i] = dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); } } From eacdce0c0bc73119db97ba7c789ea3c6264c5e45 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Thu, 6 May 2021 17:55:51 +0200 Subject: [PATCH 5/5] DecodeObject => DecodeObjects --- .../System/Diagnostics/Tracing/EventSource.cs | 296 ++++++++++-------- 1 file changed, 159 insertions(+), 137 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 5686cc856829e8..32c8dc562efaab 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -172,6 +172,7 @@ using System.Numerics; using System.Reflection; using System.Resources; +using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -1692,150 +1693,178 @@ private static Guid GenerateGuidFromName(string name) return new Guid(bytes); } - private static unsafe object? DecodeObject(Type dataType, ref EventData* data) + private static unsafe void DecodeObjects(object?[] decodedObjects, ParameterInfo[] parameters, EventData* data) { - // TODO FIX : We use reflection which in turn uses EventSource, right now we carefully avoid - // the recursion, but can we do this in a robust way? - - IntPtr dataPointer = data->DataPointer; - Debug.Assert(dataPointer != IntPtr.Zero || data->Size == 0); - - if (dataType == typeof(string)) - { - goto String; - } - else if (dataType == typeof(int)) + for (int i = 0; i < decodedObjects.Length; i++, data++) { - Debug.Assert(data->Size == 4); - return *(int*)dataPointer; - } - - TypeCode typeCode = Type.GetTypeCode(dataType); - int size = data->Size; + IntPtr dataPointer = data->DataPointer; + Type dataType = parameters[i].ParameterType; + object? decoded; - if (size == 4) - { - if ((uint)(typeCode - TypeCode.SByte) <= TypeCode.Int32 - TypeCode.SByte) - { - Debug.Assert(dataType.IsEnum); - // Enums less than 4 bytes in size should be treated as int. - return *(int*)dataPointer; - } - else if (typeCode == TypeCode.UInt32) - { - return *(uint*)dataPointer; - } - else if (typeCode == TypeCode.Single) - { - return *(float*)dataPointer; - } - else if (typeCode == TypeCode.Boolean) - { - // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. - return *(int*)dataPointer == 1; - } - else if (dataType == typeof(byte[])) - { - // byte[] are written to EventData* as an int followed by a blob - Debug.Assert(*(int*)dataPointer == (data + 1)->Size); - data++; - dataPointer = data->DataPointer; - goto BytePtr; - } - else if (IntPtr.Size == 4 && dataType == typeof(IntPtr)) - { - return *(IntPtr*)dataPointer; - } - } - else if (size <= 2) - { - Debug.Assert(!dataType.IsEnum); - if (typeCode == TypeCode.Byte) - { - Debug.Assert(size == 1); - return *(byte*)dataPointer; - } - else if (typeCode == TypeCode.SByte) - { - Debug.Assert(size == 1); - return *(sbyte*)dataPointer; - } - else if (typeCode == TypeCode.Int16) - { - Debug.Assert(size == 2); - return *(short*)dataPointer; - } - else if (typeCode == TypeCode.UInt16) - { - Debug.Assert(size == 2); - return *(ushort*)dataPointer; - } - else if (typeCode == TypeCode.Char) - { - Debug.Assert(size == 2); - return *(char*)dataPointer; - } - } - else if (size == 8) - { - if (typeCode == TypeCode.Int64) - { - return *(long*)dataPointer; - } - else if (typeCode == TypeCode.UInt64) - { - return *(ulong*)dataPointer; - } - else if (typeCode == TypeCode.Double) - { - return *(double*)dataPointer; - } - else if (typeCode == TypeCode.DateTime) + if (dataType == typeof(string)) { - return *(DateTime*)dataPointer; + goto String; } - else if (IntPtr.Size == 8 && dataType == typeof(IntPtr)) + else if (dataType == typeof(int)) { - return *(IntPtr*)dataPointer; + Debug.Assert(data->Size == 4); + decoded = *(int*)dataPointer; } - } - else - { - if (typeCode == TypeCode.Decimal) + else { - Debug.Assert(size == 16); - return *(decimal*)dataPointer; + TypeCode typeCode = Type.GetTypeCode(dataType); + int size = data->Size; + + if (size == 4) + { + if ((uint)(typeCode - TypeCode.SByte) <= TypeCode.Int32 - TypeCode.SByte) + { + Debug.Assert(dataType.IsEnum); + // Enums less than 4 bytes in size should be treated as int. + decoded = *(int*)dataPointer; + } + else if (typeCode == TypeCode.UInt32) + { + decoded = *(uint*)dataPointer; + } + else if (typeCode == TypeCode.Single) + { + decoded = *(float*)dataPointer; + } + else if (typeCode == TypeCode.Boolean) + { + // The manifest defines a bool as a 32bit type (WIN32 BOOL), not 1 bit as CLR Does. + decoded = *(int*)dataPointer == 1; + } + else if (dataType == typeof(byte[])) + { + // byte[] are written to EventData* as an int followed by a blob + Debug.Assert(*(int*)dataPointer == (data + 1)->Size); + data++; + dataPointer = data->DataPointer; + goto BytePtr; + } + else if (IntPtr.Size == 4 && dataType == typeof(IntPtr)) + { + decoded = *(IntPtr*)dataPointer; + } + else + { + goto Unknown; + } + } + else if (size <= 2) + { + Debug.Assert(!dataType.IsEnum); + if (typeCode == TypeCode.Byte) + { + Debug.Assert(size == 1); + decoded = *(byte*)dataPointer; + } + else if (typeCode == TypeCode.SByte) + { + Debug.Assert(size == 1); + decoded = *(sbyte*)dataPointer; + } + else if (typeCode == TypeCode.Int16) + { + Debug.Assert(size == 2); + decoded = *(short*)dataPointer; + } + else if (typeCode == TypeCode.UInt16) + { + Debug.Assert(size == 2); + decoded = *(ushort*)dataPointer; + } + else if (typeCode == TypeCode.Char) + { + Debug.Assert(size == 2); + decoded = *(char*)dataPointer; + } + else + { + goto Unknown; + } + } + else if (size == 8) + { + if (typeCode == TypeCode.Int64) + { + decoded = *(long*)dataPointer; + } + else if (typeCode == TypeCode.UInt64) + { + decoded = *(ulong*)dataPointer; + } + else if (typeCode == TypeCode.Double) + { + decoded = *(double*)dataPointer; + } + else if (typeCode == TypeCode.DateTime) + { + decoded = *(DateTime*)dataPointer; + } + else if (IntPtr.Size == 8 && dataType == typeof(IntPtr)) + { + decoded = *(IntPtr*)dataPointer; + } + else + { + goto Unknown; + } + } + else if (typeCode == TypeCode.Decimal) + { + Debug.Assert(size == 16); + decoded = *(decimal*)dataPointer; + } + else if (dataType == typeof(Guid)) + { + Debug.Assert(size == 16); + decoded = *(Guid*)dataPointer; + } + else + { + goto Unknown; + } } - else if (dataType == typeof(Guid)) + + goto Store; + + Unknown: + if (dataType != typeof(byte*)) { - Debug.Assert(size == 16); - return *(Guid*)dataPointer; + // Everything else is marshaled as a string. + goto String; } - } - if (dataType != typeof(byte*)) - { - goto String; + BytePtr: + var blob = new byte[data->Size]; + Marshal.Copy(dataPointer, blob, 0, blob.Length); + decoded = blob; + goto Store; + + String: + // ETW strings are NULL-terminated, so marshal everything up to the first null in the string. + AssertValidString(data); + decoded = dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); + + Store: + decodedObjects[i] = decoded; } + } - BytePtr: - byte[] blob = new byte[data->Size]; - for (int i = 0; i < blob.Length; i++) + [Conditional("DEBUG")] + private static unsafe void AssertValidString(EventData* data) + { + Debug.Assert(data->Size >= 0 && data->Size % 2 == 0, "String size should be even"); + char* charPointer = (char*)data->DataPointer; + int charLength = data->Size / 2 - 1; + for (int i = 0; i < charLength; i++) { - blob[i] = *((byte*)dataPointer + i); + Debug.Assert(*(charPointer + i) != 0, "String may not contain null chars"); } - return blob; - - String: - // Everything else is marshaled as a string. - // ETW strings are NULL-terminated, so marshal everything up to the first - // null in the string. -#if DEBUG - Debug.Assert(data->Size % 2 == 0, "String size should be even"); - for (int i = 0; i < data->Size / 2 - 1; i++) Debug.Assert(*((char*)dataPointer + i) != 0, "String may not contain null chars"); - Debug.Assert(*((char*)dataPointer + data->Size / 2 - 1) == 0, "String must be null terminated"); -#endif - return dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); + Debug.Assert(*(charPointer + charLength) == 0, "String must be null terminated"); } // Finds the Dispatcher (which holds the filtering state), for a given dispatcher for the current @@ -2066,12 +2095,8 @@ private unsafe void WriteToAllListeners(int eventId, Guid* activityID, Guid* chi { for (int i = 0; i < args.Length; i++, data++) { + AssertValidString(data); IntPtr dataPointer = data->DataPointer; -#if DEBUG - Debug.Assert(data->Size % 2 == 0, "String size should be even"); - for (int j = 0; j < data->Size / 2 - 1; j++) Debug.Assert(*((char*)dataPointer + j) != 0, "String may not contain null chars"); - Debug.Assert(*((char*)dataPointer + data->Size / 2 - 1) == 0, "String must be null terminated"); -#endif args[i] = dataPointer == IntPtr.Zero ? null : new string((char*)dataPointer, 0, (data->Size >> 1) - 1); } } @@ -2085,10 +2110,7 @@ private unsafe void WriteToAllListeners(int eventId, Guid* activityID, Guid* chi } else { - for (int i = 0; i < args.Length; i++, data++) - { - args[i] = DecodeObject(parameters[i].ParameterType, ref data); - } + DecodeObjects(args, parameters, data); } }