From a3c8a26e549f9b6a71f5d188ca12ad1943487132 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 9 Sep 2019 14:10:49 -0500 Subject: [PATCH] Improve perf of property name lookup --- .../Text/Json/Serialization/JsonClassInfo.cs | 191 ++++++++++-------- .../JsonSerializer.Read.HandlePropertyName.cs | 7 +- .../tests/Serialization/PropertyNameTests.cs | 82 ++++++++ 3 files changed, 198 insertions(+), 82 deletions(-) diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index 2fa525bbf675..3e754b5bbcc2 100644 --- a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -16,7 +16,7 @@ namespace System.Text.Json internal sealed partial class JsonClassInfo { // The length of the property name embedded in the key (in bytes). - private const int PropertyNameKeyLength = 6; + private const int PropertyNameKeyLength = 7; // The limit to how many property names from the JSON are cached in _propertyRefsSorted before using PropertyCache. private const int PropertyNameCountCacheThreshold = 64; @@ -259,16 +259,14 @@ private JsonPropertyInfo GetPropertyWithUniqueAttribute(Type attributeType, Dict public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadStackFrame frame) { - JsonPropertyInfo info = null; - // Keep a local copy of the cache in case it changes by another thread. PropertyRef[] localPropertyRefsSorted = _propertyRefsSorted; + ulong key = GetKey(propertyName); + // If there is an existing cache, then use it. if (localPropertyRefsSorted != null) { - ulong key = GetKey(propertyName); - // Start with the current property index, and then go forwards\backwards. int propertyIndex = frame.PropertyIndex; @@ -276,66 +274,108 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta int iForward = Math.Min(propertyIndex, count); int iBackward = iForward - 1; - while (iForward < count || iBackward >= 0) + for (;;) { if (iForward < count) { - if (TryIsPropertyRefEqual(localPropertyRefsSorted[iForward], propertyName, key, ref info)) + PropertyRef propertyRef = localPropertyRefsSorted[iForward]; + if (key == propertyRef.Key) { - return info; + if (propertyName.Length <= PropertyNameKeyLength || + // We compare the whole name, although we could skip the first 7 bytes (but it's not any faster) + propertyName.SequenceEqual(propertyRef.Info.Name)) + { + return propertyRef.Info; + } } ++iForward; - } - if (iBackward >= 0) + if (iBackward >= 0) + { + propertyRef = localPropertyRefsSorted[iBackward]; + // This path is the same code as above; copied here instead of creating a method since this is a hot path. + if (key == propertyRef.Key) + { + if (propertyName.Length <= PropertyNameKeyLength || + propertyName.SequenceEqual(propertyRef.Info.Name)) + { + return propertyRef.Info; + } + } + + --iBackward; + } + } + else if (iBackward >= 0) { - if (TryIsPropertyRefEqual(localPropertyRefsSorted[iBackward], propertyName, key, ref info)) + // This path is the same code as above; copied here instead of creating a method since this is a hot path. + PropertyRef propertyRef = localPropertyRefsSorted[iBackward]; + if (key == propertyRef.Key) { - return info; + if (propertyName.Length <= PropertyNameKeyLength || + propertyName.SequenceEqual(propertyRef.Info.Name)) + { + return propertyRef.Info; + } } + --iBackward; } + else + { + // Property was not found. + break; + } } } // No cached item was found. Try the main list which has all of the properties. string stringPropertyName = JsonHelpers.Utf8GetString(propertyName); - if (PropertyCache.TryGetValue(stringPropertyName, out info)) + PropertyCache.TryGetValue(stringPropertyName, out JsonPropertyInfo info); + + // Three code paths to get here: + // 1) info == null. Property not found. + // 2) key == info.PropertyNameKey. Exact match found. + // 3) key != info.PropertyNameKey. Match found due to case insensitivity. + Debug.Assert(info == null || key == info.PropertyNameKey || Options.PropertyNameCaseInsensitive); + + if (info == null) { - // Check if we should add this to the cache. - // Only cache up to a threshold length and then just use the dictionary when an item is not found in the cache. - int count; - if (localPropertyRefsSorted != null) - { - count = localPropertyRefsSorted.Length; - } - else + info = JsonPropertyInfo.s_missingProperty; + } + + // Check if we should add this to the cache. + // Only cache up to a threshold length and then just use the dictionary when an item is not found in the cache. + int cacheCount; + if (localPropertyRefsSorted != null) + { + cacheCount = localPropertyRefsSorted.Length; + } + else + { + cacheCount = 0; + } + + // Do a quick check for the stable (after warm-up) case. + if (cacheCount < PropertyNameCountCacheThreshold) + { + // Do a slower check for the warm-up case. + if (frame.PropertyRefCache != null) { - count = 0; + cacheCount += frame.PropertyRefCache.Count; } - // Do a quick check for the stable (after warm-up) case. - if (count < PropertyNameCountCacheThreshold) + // Check again to append the cache up to the threshold. + if (cacheCount < PropertyNameCountCacheThreshold) { - // Do a slower check for the warm-up case. - if (frame.PropertyRefCache != null) + if (frame.PropertyRefCache == null) { - count += frame.PropertyRefCache.Count; + frame.PropertyRefCache = new List(); } - // Check again to append the cache up to the threshold. - if (count < PropertyNameCountCacheThreshold) - { - if (frame.PropertyRefCache == null) - { - frame.PropertyRefCache = new List(); - } - - ulong key = info.PropertyNameKey; - PropertyRef propertyRef = new PropertyRef(key, info); - frame.PropertyRefCache.Add(propertyRef); - } + PropertyRef propertyRef = new PropertyRef(key, info); + frame.PropertyRefCache.Add(propertyRef); } } @@ -360,60 +400,52 @@ private Dictionary CreatePropertyCache(int capacity) public JsonPropertyInfo PolicyProperty { get; private set; } - private static bool TryIsPropertyRefEqual(in PropertyRef propertyRef, ReadOnlySpan propertyName, ulong key, ref JsonPropertyInfo info) - { - if (key == propertyRef.Key) - { - if (propertyName.Length <= PropertyNameKeyLength || - // We compare the whole name, although we could skip the first 6 bytes (but it's likely not any faster) - propertyName.SequenceEqual(propertyRef.Info.Name)) - { - info = propertyRef.Info; - return true; - } - } - - return false; - } - - private static bool IsPropertyRefEqual(ref PropertyRef propertyRef, PropertyRef other) - { - if (propertyRef.Key == other.Key) - { - if (propertyRef.Info.Name.Length <= PropertyNameKeyLength || - propertyRef.Info.Name.AsSpan().SequenceEqual(other.Info.Name.AsSpan())) - { - return true; - } - } - - return false; - } - + /// + /// Get a key from the property name. + /// If property name length is 7 or less then this key can be used for equality since + /// the first 7 bytes are encoded along with the length. + /// public static ulong GetKey(ReadOnlySpan propertyName) { ulong key; int length = propertyName.Length; - // Embed the propertyName in the first 6 bytes of the key. - if (length > 3) + // Embed the propertyName in the first 7 bytes of the key. + if (length > 7) + { + key = MemoryMarshal.Read(propertyName); + + // Clear the last byte so it can hold the length. + // Be consistent by using shift operators instead of & to avoid endianness issues. + key = key << 0x8 >> 0x8; + } + else if (length > 3) { key = MemoryMarshal.Read(propertyName); - if (length > 4) + + if (length == 7) { - key |= (ulong)propertyName[4] << 32; + key |= (ulong)propertyName[4] << 0x20 + | (ulong)propertyName[5] << 0x28 + | (ulong)propertyName[6] << 0x30; } - if (length > 5) + else if (length == 6) { - key |= (ulong)propertyName[5] << 40; + key |= (ulong)propertyName[4] << 0x20 + | (ulong)propertyName[5] << 0x28; + } + else if (length == 5) + { + key |= (ulong)propertyName[4] << 0x20; } } else if (length > 1) { key = MemoryMarshal.Read(propertyName); - if (length > 2) + + if (length == 3) { - key |= (ulong)propertyName[2] << 16; + key |= (ulong)propertyName[2] << 0x10; } } else if (length == 1) @@ -426,8 +458,9 @@ public static ulong GetKey(ReadOnlySpan propertyName) key = 0; } - // Embed the propertyName length in the last two bytes. - key |= (ulong)propertyName.Length << 48; + // Embed the length in the last byte. + key |= (ulong)Math.Min(propertyName.Length, 0xFF) << 0x38; + return key; } diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index ced83b472836..3f42d0288a99 100644 --- a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -52,7 +52,7 @@ private static void HandlePropertyName( } JsonPropertyInfo jsonPropertyInfo = state.Current.JsonClassInfo.GetProperty(propertyName, ref state.Current); - if (jsonPropertyInfo == null) + if (jsonPropertyInfo == JsonPropertyInfo.s_missingProperty) { JsonPropertyInfo dataExtProperty = state.Current.JsonClassInfo.DataExtensionProperty; if (dataExtProperty == null) @@ -94,9 +94,10 @@ private static void HandlePropertyName( state.Current.JsonPropertyInfo.JsonPropertyName = propertyNameArray; } } - - state.Current.PropertyIndex++; } + + // Increment the PropertyIndex so JsonClassInfo.GetProperty() starts with the next property. + state.Current.PropertyIndex++; } } diff --git a/src/System.Text.Json/tests/Serialization/PropertyNameTests.cs b/src/System.Text.Json/tests/Serialization/PropertyNameTests.cs index e039ffdd7930..462c1512fc16 100644 --- a/src/System.Text.Json/tests/Serialization/PropertyNameTests.cs +++ b/src/System.Text.Json/tests/Serialization/PropertyNameTests.cs @@ -293,6 +293,87 @@ public static void ExtensionDataDictionarySerialize_DoesNotHonor() Assert.False(obj.MyOverflow.ContainsKey("key1")); Assert.Equal(1, obj.MyOverflow["Key1"].GetInt32()); } + + private class ClassWithPropertyNamePermutations + { + public int a { get; set; } + public int aa { get; set; } + public int aaa { get; set; } + public int aaaa { get; set; } + public int aaaaa { get; set; } + public int aaaaaa { get; set; } + + // 7 characters - caching code only keys up to 7. + public int aaaaaaa { get; set; } + public int aaaaaab { get; set; } + + // 8 characters. + public int aaaaaaaa { get; set; } + public int aaaaaaab { get; set; } + + // 9 characters - caching code doesn't differentiate past this. + public int aaaaaaaaa { get; set; } + public int aaaaaaaab { get; set; } + } + + [Fact] + public static void CachingKeys() + { + ClassWithPropertyNamePermutations obj = new ClassWithPropertyNamePermutations + { + a = 1, + aa = 2, + aaa = 3, + aaaa = 4, + aaaaa = 5, + aaaaaa = 6, + aaaaaaa = 7, + aaaaaab = 7, + aaaaaaaa = 8, + aaaaaaab = 8, + aaaaaaaaa = 9, + aaaaaaaab = 9, + }; + + string json = JsonSerializer.Serialize(obj); + obj = JsonSerializer.Deserialize(json); + + Assert.Equal(130, json.Length); + + Assert.Equal(1, obj.a); + Assert.Equal(2, obj.aa); + Assert.Equal(3, obj.aaa); + Assert.Equal(4, obj.aaaa); + Assert.Equal(5, obj.aaaaa); + Assert.Equal(6, obj.aaaaaa); + Assert.Equal(7, obj.aaaaaaa); + Assert.Equal(7, obj.aaaaaab); + Assert.Equal(8, obj.aaaaaaaa); + Assert.Equal(8, obj.aaaaaaab); + Assert.Equal(9, obj.aaaaaaaaa); + Assert.Equal(9, obj.aaaaaaaab); + } + + [Theory] + [InlineData(0x1)] + [InlineData(0x10)] + [InlineData(0x100)] + [InlineData(0x1000)] + [InlineData(0x10000)] + public static void LongPropertyNames(int propertyLength) + { + // Although the CLR may limit member length to 1023, the serializer doesn't have a hard limit. + + string val = new string('v', propertyLength); + string json = @"{""" + val + @""":1}"; + + EmptyClassWithExtensionProperty obj = JsonSerializer.Deserialize(json); + + Assert.True(obj.MyOverflow.ContainsKey(val)); + + json = JsonSerializer.Serialize(obj); + Assert.Contains(val, json); + } } public class OverridePropertyNameDesignTime_TestClass @@ -359,3 +440,4 @@ public class EmptyClassWithExtensionProperty public IDictionary MyOverflow { get; set; } } } +