From 42882cb71b1b1afc58377d78a1e467bd7e505fa8 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 19 Aug 2024 12:05:51 +0100 Subject: [PATCH] Perf changes (#79) * Use Concurrent Dictionaries and ignore case on them to avoid ToLower * remove single array allocation * allocate GetClosures differently * Use JsonReader for closures * add comment * use isdefined * more readonly with less allocations * sorts * fmt * use element type when making an array --- .../Serialisation/BaseObjectDeserializerV2.cs | 46 ++-------- .../BaseObjectSerializationUtilities.cs | 82 ++++++++---------- .../SerializationUtilities/ClosureParser.cs | 83 +++++++++++++++++++ .../SerializationUtilities/ValueConverter.cs | 11 ++- 4 files changed, 134 insertions(+), 88 deletions(-) create mode 100644 src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs diff --git a/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs b/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs index bd3defb5..34ede6db 100644 --- a/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs +++ b/src/Speckle.Sdk/Serialisation/BaseObjectDeserializerV2.cs @@ -64,8 +64,7 @@ public Base Deserialize(string rootObjectJson) _workerThreads = new DeserializationWorkerThreads(this, WorkerThreadCount); _workerThreads.Start(); - List<(string, int)> closures = GetClosures(rootObjectJson); - closures.Sort((a, b) => b.Item2.CompareTo(a.Item2)); + var closures = ClosureParser.GetClosures(rootObjectJson); int i = 0; foreach (var closure in closures) { @@ -115,32 +114,6 @@ public Base Deserialize(string rootObjectJson) } } - private List<(string, int)> GetClosures(string rootObjectJson) - { - try - { - List<(string, int)> closureList = new(); - JObject doc1 = JObject.Parse(rootObjectJson); - - if (!doc1.ContainsKey("__closure")) - { - return new List<(string, int)>(); - } - - foreach (JToken prop in doc1["__closure"].NotNull()) - { - string childId = ((JProperty)prop).Name; - int childMinDepth = (int)((JProperty)prop).Value; - closureList.Add((childId, childMinDepth)); - } - return closureList; - } - catch (Exception ex) when (!ex.IsFatal()) - { - return new List<(string, int)>(); - } - } - private object? DeserializeTransportObjectProxy(string? objectJson, long? current, long? total) { if (objectJson is null) @@ -345,30 +318,26 @@ private Base Dict2Base(Dictionary dictObj) dictObj.Remove(TYPE_DISCRIMINATOR); dictObj.Remove("__closure"); - Dictionary staticProperties = BaseObjectSerializationUtilities.GetTypeProperties(typeName); - List onDeserializedCallbacks = BaseObjectSerializationUtilities.GetOnDeserializedCallbacks(typeName); - + var staticProperties = BaseObjectSerializationUtilities.GetTypeProperties(typeName); foreach (var entry in dictObj) { - string lowerPropertyName = entry.Key.ToLower(); - if (staticProperties.TryGetValue(lowerPropertyName, out PropertyInfo? value) && value.CanWrite) + if (staticProperties.TryGetValue(entry.Key, out PropertyInfo? value) && value.CanWrite) { - PropertyInfo property = staticProperties[lowerPropertyName]; if (entry.Value == null) { // Check for JsonProperty(NullValueHandling = NullValueHandling.Ignore) attribute - JsonPropertyAttribute attr = property.GetCustomAttribute(true); - if (attr != null && attr.NullValueHandling == NullValueHandling.Ignore) + JsonPropertyAttribute attr = value.GetCustomAttribute(true); + if (attr is { NullValueHandling: NullValueHandling.Ignore }) { continue; } } - Type targetValueType = property.PropertyType; + Type targetValueType = value.PropertyType; bool conversionOk = ValueConverter.ConvertValue(targetValueType, entry.Value, out object? convertedValue); if (conversionOk) { - property.SetValue(baseObj, convertedValue); + value.SetValue(baseObj, convertedValue); } else { @@ -390,6 +359,7 @@ private Base Dict2Base(Dictionary dictObj) bb.filePath = bb.GetLocalDestinationPath(BlobStorageFolder); } + var onDeserializedCallbacks = BaseObjectSerializationUtilities.GetOnDeserializedCallbacks(typeName); foreach (MethodInfo onDeserialized in onDeserializedCallbacks) { onDeserialized.Invoke(baseObj, new object?[] { null }); diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs b/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs index c64bd149..4e51ed93 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs +++ b/src/Speckle.Sdk/Serialisation/SerializationUtilities/BaseObjectSerializationUtilities.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Reflection; using System.Runtime.Serialization; using Speckle.Sdk.Host; @@ -7,62 +8,47 @@ namespace Speckle.Sdk.Serialisation.SerializationUtilities; internal static class BaseObjectSerializationUtilities { #region Getting Types - private static Dictionary> s_typeProperties = new(); - private static Dictionary> s_onDeserializedCallbacks = new(); + private static ConcurrentDictionary> s_typeProperties = new(); + private static ConcurrentDictionary> s_onDeserializedCallbacks = new(); - internal static Dictionary GetTypeProperties(string objFullType) - { - lock (s_typeProperties) - { - if (s_typeProperties.TryGetValue(objFullType, out Dictionary? value)) - { - return value; - } - - Dictionary ret = new(); - Type type = TypeLoader.GetType(objFullType); - PropertyInfo[] properties = type.GetProperties(); - foreach (PropertyInfo prop in properties) + internal static IReadOnlyDictionary GetTypeProperties(string objFullType) => + s_typeProperties.GetOrAdd( + objFullType, + s => { - ret[prop.Name.ToLower()] = prop; - } - - value = ret; - s_typeProperties[objFullType] = value; - return value; - } - } - - internal static List GetOnDeserializedCallbacks(string objFullType) - { - // return new List(); - lock (s_onDeserializedCallbacks) - { - // System.Runtime.Serialization.Ca - if (s_onDeserializedCallbacks.TryGetValue(objFullType, out List? value)) - { - return value; + Type type = TypeLoader.GetType(s); + PropertyInfo[] properties = type.GetProperties(); + Dictionary ret = new(properties.Length, StringComparer.OrdinalIgnoreCase); + foreach (PropertyInfo prop in properties) + { + ret[prop.Name] = prop; + } + return ret; } + ); - List ret = new(); - Type type = TypeLoader.GetType(objFullType); - MethodInfo[] methods = type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - foreach (MethodInfo method in methods) + internal static IReadOnlyList GetOnDeserializedCallbacks(string objFullType) => + s_onDeserializedCallbacks.GetOrAdd( + objFullType, + s => { - List onDeserializedAttributes = method - .GetCustomAttributes(true) - .ToList(); - if (onDeserializedAttributes.Count > 0) + List? ret = null; + Type type = TypeLoader.GetType(s); + MethodInfo[] methods = type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + foreach (MethodInfo method in methods) { - ret.Add(method); + if (method.IsDefined(typeof(OnDeserializedAttribute), true)) + { + if (ret == null) + { + ret = new List(); + } + ret.Add(method); + } } + return (ret as IReadOnlyList) ?? Array.Empty(); } - - value = ret; - s_onDeserializedCallbacks[objFullType] = value; - return value; - } - } + ); /// /// Flushes kit's (discriminator, type) cache. Useful if you're dynamically loading more kits at runtime, that provide better coverage of what you're deserialising, and it's now somehow poisoned because the higher level types were not originally available. diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs b/src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs new file mode 100644 index 00000000..1c5d0680 --- /dev/null +++ b/src/Speckle.Sdk/Serialisation/SerializationUtilities/ClosureParser.cs @@ -0,0 +1,83 @@ +using Speckle.Newtonsoft.Json; +using Speckle.Sdk.Common; + +namespace Speckle.Sdk.Serialisation.SerializationUtilities; + +public static class ClosureParser +{ + public static IReadOnlyList<(string, int)> GetClosures(string rootObjectJson) + { + try + { + using JsonTextReader reader = new(new StringReader(rootObjectJson)); + reader.Read(); + while (reader.TokenType != JsonToken.EndObject) + { + switch (reader.TokenType) + { + case JsonToken.StartObject: + { + var closureList = ReadObject(reader); + if (closureList?.Any() ?? false) + { + closureList.Sort((a, b) => b.Item2.CompareTo(a.Item2)); + return closureList; + } + return Array.Empty<(string, int)>(); + } + default: + reader.Read(); + reader.Skip(); + break; + } + } + } + catch (Exception ex) when (!ex.IsFatal()) { } + return Array.Empty<(string, int)>(); + } + + private static List<(string, int)>? ReadObject(JsonTextReader reader) + { + reader.Read(); + while (reader.TokenType != JsonToken.EndObject) + { + switch (reader.TokenType) + { + case JsonToken.PropertyName: + { + if (reader.Value as string == "__closure") + { + reader.Read(); //goes to prop vale + var closureList = ReadClosureList(reader); + return closureList; + } + reader.Read(); //goes to prop vale + reader.Skip(); + reader.Read(); //goes to next + } + break; + default: + reader.Read(); + reader.Skip(); + reader.Read(); + break; + } + } + return null; + } + + private static List<(string, int)> ReadClosureList(JsonTextReader reader) + { + List<(string, int)> closureList = new(); + reader.Read(); //startobject + while (reader.TokenType != JsonToken.EndObject) + { + var childId = (reader.Value as string).NotNull(); // propertyName + int childMinDepth = reader.ReadAsInt32().NotNull(); //propertyValue + reader.Read(); + closureList.Add((childId, childMinDepth)); + } + + return closureList; + } +} diff --git a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs b/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs index 87cb9d8e..0277ebde 100644 --- a/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs +++ b/src/Speckle.Sdk/Serialisation/SerializationUtilities/ValueConverter.cs @@ -1,3 +1,4 @@ +using System.Buffers; using System.Collections; using System.Diagnostics.Contracts; using System.Drawing; @@ -9,6 +10,8 @@ namespace Speckle.Sdk.Serialisation.SerializationUtilities; internal static class ValueConverter { + private static object[] _singleValue = new object[1]; + public static bool ConvertValue(Type type, object? value, out object? convertedValue) { // TODO: Document list of supported values in the SDK. (and grow it as needed) @@ -165,7 +168,11 @@ public static bool ConvertValue(Type type, object? value, out object? convertedV var targetType = typeof(List<>).MakeGenericType(type.GenericTypeArguments); Type listElementType = type.GenericTypeArguments[0]; - IList ret = (IList)Activator.CreateInstance(targetType, valueList.Count); + + _singleValue[0] = valueList.Count; + //reuse array to avoid params array allocation + IList ret = (IList)Activator.CreateInstance(targetType, _singleValue); + foreach (object inputListElement in valueList) { if (!ConvertValue(listElementType, inputListElement, out object? convertedListElement)) @@ -219,7 +226,7 @@ public static bool ConvertValue(Type type, object? value, out object? convertedV Type arrayElementType = type.GetElementType() ?? throw new ArgumentException("IsArray yet not valid element type", nameof(type)); - Array ret = (Array)Activator.CreateInstance(type, valueList.Count); + Array ret = Array.CreateInstance(arrayElementType, valueList.Count); for (int i = 0; i < valueList.Count; i++) { object inputListElement = valueList[i];