Skip to content

Commit

Permalink
Add a test and other misc feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
steveharter committed Jun 18, 2020
1 parent 5b4d8b0 commit 37ef28f
Show file tree
Hide file tree
Showing 17 changed files with 101 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private static JsonEncodedText EncodeHelper(ReadOnlySpan<byte> utf8Value, JavaSc

if (idx != -1)
{
return new JsonEncodedText(JsonHelpers.GetEscapedString(utf8Value, idx, encoder));
return new JsonEncodedText(JsonHelpers.EscapeValue(utf8Value, idx, encoder));
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace System.Text.Json
internal static partial class JsonHelpers
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static byte[] GetEscapedPropertyNameSection(ReadOnlySpan<byte> utf8Value, JavaScriptEncoder? encoder)
public static byte[] GetEscapedPropertyNameSection(ReadOnlySpan<byte> utf8Value, JavaScriptEncoder? encoder)
{
int idx = JsonWriterHelper.NeedsEscaping(utf8Value, encoder);

Expand All @@ -26,7 +26,7 @@ internal static byte[] GetEscapedPropertyNameSection(ReadOnlySpan<byte> utf8Valu
}
}

internal static byte[] GetEscapedString(
public static byte[] EscapeValue(
ReadOnlySpan<byte> utf8Value,
int firstEscapeIndexVal,
JavaScriptEncoder? encoder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public static void ReadWithVerify(this ref Utf8JsonReader reader)
/// </summary>
/// <param name="bytes">The utf8 bytes to convert.</param>
/// <returns></returns>
internal static string Utf8GetString(ReadOnlySpan<byte> bytes)
public static string Utf8GetString(ReadOnlySpan<byte> bytes)
{
return Encoding.UTF8.GetString(bytes
#if NETSTANDARD2_0 || NETFRAMEWORK
Expand All @@ -103,7 +103,7 @@ internal static string Utf8GetString(ReadOnlySpan<byte> bytes)
/// <summary>
/// Emulates Dictionary.TryAdd on netstandard.
/// </summary>
internal static bool TryAdd<TKey, TValue>(Dictionary<TKey, TValue> dictionary, in TKey key, in TValue value) where TKey : notnull
public static bool TryAdd<TKey, TValue>(Dictionary<TKey, TValue> dictionary, in TKey key, in TValue value) where TKey : notnull
{
#if NETSTANDARD2_0 || NETFRAMEWORK
if (!dictionary.ContainsKey(key))
Expand All @@ -118,7 +118,7 @@ internal static bool TryAdd<TKey, TValue>(Dictionary<TKey, TValue> dictionary, i
#endif
}

internal static bool IsFinite(double value)
public static bool IsFinite(double value)
{
#if BUILDING_INBOX_LIBRARY
return double.IsFinite(value);
Expand All @@ -127,7 +127,7 @@ internal static bool IsFinite(double value)
#endif
}

internal static bool IsFinite(float value)
public static bool IsFinite(float value)
{
#if BUILDING_INBOX_LIBRARY
return float.IsFinite(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ internal static class IEnumerableConverterFactoryHelpers

Type? baseTypeToCheck = type;

while (baseTypeToCheck != null && baseTypeToCheck != typeof(object))
while (baseTypeToCheck != null && baseTypeToCheck != JsonClassInfo.ObjectType)
{
if (baseTypeToCheck.IsGenericType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer

if (parameterCount <= JsonConstants.UnboxedParameterCountThreshold)
{
Type placeHolderType = typeof(object);
Type placeHolderType = JsonClassInfo.ObjectType;
Type[] typeArguments = new Type[JsonConstants.UnboxedParameterCountThreshold + 1];

typeArguments[0] = typeToConvert;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ namespace System.Text.Json
[DebuggerDisplay("ClassType.{ClassType}, {Type.Name}")]
internal sealed partial class JsonClassInfo
{
/// <summary>
/// Cached typeof(object). It is faster to cache this than to call typeof(object) multiple times.
/// </summary>
public static readonly Type ObjectType = typeof(object);

// The length of the property name embedded in the key (in bytes).
// The key is a ulong (8 bytes) containing the first 7 bytes of the property name
// followed by a byte representing the length.
Expand Down Expand Up @@ -115,7 +120,7 @@ internal static JsonPropertyInfo CreatePropertyInfoForClassInfo(
declaredPropertyType: declaredPropertyType,
runtimePropertyType: runtimePropertyType,
propertyInfo: null, // Not a real property so this is null.
parentClassType: typeof(object), // a dummy value (not used)
parentClassType: JsonClassInfo.ObjectType, // a dummy value (not used)
converter: converter,
options);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protected internal JsonConverter()
{
// Today only typeof(object) can have polymorphic writes.
// In the future, this will be check for !IsSealed (and excluding value types).
CanBePolymorphic = TypeToConvert == typeof(object);
CanBePolymorphic = TypeToConvert == JsonClassInfo.ObjectType;
IsValueType = TypeToConvert.IsValueType;
HandleNull = IsValueType;
CanBeNull = !IsValueType || Nullable.GetUnderlyingType(TypeToConvert) != null;
Expand Down Expand Up @@ -268,7 +268,7 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions
}

Type type = value.GetType();
if (type == typeof(object))
if (type == JsonClassInfo.ObjectType)
{
writer.WriteStartObject();
writer.WriteEndObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public bool ReadJsonAndAddExtensionProperty(object obj, ref ReadStack state, ref
}
else
{
JsonConverter<object> converter = (JsonConverter<object>)Options.GetConverter(typeof(object));
JsonConverter<object> converter = (JsonConverter<object>)Options.GetConverter(JsonClassInfo.ObjectType);

if (!converter.TryRead(ref reader, typeof(JsonElement), Options, ref state, out object? value))
{
Expand Down Expand Up @@ -281,7 +281,7 @@ public bool ReadJsonExtensionDataValue(ref ReadStack state, ref Utf8JsonReader r
{
Debug.Assert(this == state.Current.JsonClassInfo.DataExtensionProperty);

if (RuntimeClassInfo.ElementType == typeof(object) && reader.TokenType == JsonTokenType.Null)
if (RuntimeClassInfo.ElementType == JsonClassInfo.ObjectType && reader.TokenType == JsonTokenType.Null)
{
value = null;
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal static void CreateDataExtensionProperty(
Debug.Assert(genericArgs.Length == 2);
Debug.Assert(genericArgs[0].UnderlyingSystemType == typeof(string));
Debug.Assert(
genericArgs[1].UnderlyingSystemType == typeof(object) ||
genericArgs[1].UnderlyingSystemType == JsonClassInfo.ObjectType ||
genericArgs[1].UnderlyingSystemType == typeof(JsonElement));
#endif
if (jsonPropertyInfo.RuntimeClassInfo.CreateObject == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private static void WriteCore<TValue>(
Debug.Assert(writer != null);

// We treat typeof(object) special and allow polymorphic behavior.
if (value != null && inputType == typeof(object))
if (value != null && inputType == JsonClassInfo.ObjectType)
{
inputType = value!.GetType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private static async Task WriteAsyncCore<TValue>(
using (var writer = new Utf8JsonWriter(bufferWriter, writerOptions))
{
// We treat typeof(object) special and allow polymorphic behavior.
if (inputType == typeof(object) && value != null)
if (inputType == JsonClassInfo.ObjectType && value != null)
{
inputType = value!.GetType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public sealed partial class JsonSerializerOptions

// Simple LRU cache for the public (de)serialize entry points that avoid some lookups in _classes.
// Although this may be written by multiple threads, 'volatile' was not added since any local affinity is fine.
internal JsonClassInfo? LastClass { get; set; }
private JsonClassInfo? _lastClass { get; set; }

// For any new option added, adding it to the options copied in the copy constructor below must be considered.

Expand Down Expand Up @@ -451,6 +451,22 @@ internal JsonClassInfo GetOrAddClass(Type type)
return result;
}

/// <summary>
/// Return the ClassInfo for root API calls.
/// This has a LRU cache that is intended only for public API calls that specify the root type.
/// </summary>
internal JsonClassInfo GetOrAddClassForRootType(Type type)
{
JsonClassInfo? jsonClassInfo = _lastClass;
if (jsonClassInfo?.Type != type)
{
jsonClassInfo = GetOrAddClass(type);
_lastClass = jsonClassInfo;
}

return jsonClassInfo;
}

internal bool TypeIsCached(Type type)
{
return _classes.ContainsKey(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,7 @@ private void AddCurrent()

public void Initialize(Type type, JsonSerializerOptions options, bool supportContinuation)
{
JsonClassInfo? jsonClassInfo = options.LastClass;
if (jsonClassInfo?.Type != type)
{
jsonClassInfo = options.GetOrAddClass(type);
options.LastClass = jsonClassInfo;
}

JsonClassInfo jsonClassInfo = options.GetOrAddClassForRootType(type);
Current.JsonClassInfo = jsonClassInfo;

// The initial JsonPropertyInfo will be used to obtain the converter.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor

var dynamicMethod = new DynamicMethod(
ConstructorInfo.ConstructorName,
typeof(object),
JsonClassInfo.ObjectType,
Type.EmptyTypes,
typeof(ReflectionEmitMemberAccessor).Module,
skipVisibility: true);
Expand Down Expand Up @@ -158,7 +158,7 @@ private static DynamicMethod CreateAddMethodDelegate(Type collectionType)
var dynamicMethod = new DynamicMethod(
realMethod.Name,
typeof(void),
new[] { collectionType, typeof(object) },
new[] { collectionType, JsonClassInfo.ObjectType },
typeof(ReflectionEmitMemberAccessor).Module,
skipVisibility: true);

Expand Down Expand Up @@ -226,7 +226,7 @@ private static DynamicMethod CreateImmutableDictionaryCreateRangeDelegate(Type c
private static DynamicMethod CreatePropertyGetter(PropertyInfo propertyInfo, Type classType, Type propertyType)
{
MethodInfo? realMethod = propertyInfo.GetMethod;
Type objectType = typeof(object);
Type objectType = JsonClassInfo.ObjectType;

Debug.Assert(realMethod != null);
var dynamicMethod = new DynamicMethod(
Expand Down Expand Up @@ -262,7 +262,7 @@ private static DynamicMethod CreatePropertyGetter(PropertyInfo propertyInfo, Typ
private static DynamicMethod CreatePropertySetter(PropertyInfo propertyInfo, Type classType, Type propertyType)
{
MethodInfo? realMethod = propertyInfo.SetMethod;
Type objectType = typeof(object);
Type objectType = JsonClassInfo.ObjectType;

Debug.Assert(realMethod != null);
var dynamicMethod = new DynamicMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public override JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1,
public override Action<TCollection, object?> CreateAddMethodDelegate<TCollection>()
{
Type collectionType = typeof(TCollection);
Type elementType = typeof(object);
Type elementType = JsonClassInfo.ObjectType;

// We verified this won't be null when we created the converter for the collection type.
MethodInfo addMethod = (collectionType.GetMethod("Push") ?? collectionType.GetMethod("Enqueue"))!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,7 @@ private void AddCurrent()
/// </summary>
public JsonConverter Initialize(Type type, JsonSerializerOptions options, bool supportContinuation)
{
JsonClassInfo? jsonClassInfo = options.LastClass;
if (jsonClassInfo?.Type != type)
{
jsonClassInfo = options.GetOrAddClass(type);
options.LastClass = jsonClassInfo;
}

JsonClassInfo jsonClassInfo = options.GetOrAddClassForRootType(type);
Current.JsonClassInfo = jsonClassInfo;

if ((jsonClassInfo.ClassType & (ClassType.Enumerable | ClassType.Dictionary)) == 0)
Expand Down
58 changes: 55 additions & 3 deletions src/libraries/System.Text.Json/tests/Serialization/CacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ namespace System.Text.Json.Serialization.Tests
public static class CacheTests
{
[Fact, OuterLoop]
public static void MultipleThreadsLooping()
public static void MultipleThreads_SameType_DifferentJson_Looping()
{
const int Iterations = 100;

for (int i = 0; i < Iterations; i++)
{
MultipleThreads();
MultipleThreads_SameType_DifferentJson();
}
}

[Fact]
public static void MultipleThreads()
public static void MultipleThreads_SameType_DifferentJson()
{
// Use local options to avoid obtaining already cached metadata from the default options.
var options = new JsonSerializerOptions();
Expand Down Expand Up @@ -73,6 +73,58 @@ void SerializeObject()
Task.WaitAll(tasks);
}

[Fact, OuterLoop]
public static void MultipleThreads_DifferentTypes_Looping()
{
const int Iterations = 100;

for (int i = 0; i < Iterations; i++)
{
MultipleThreads_DifferentTypes();
}
}

[Fact]
public static void MultipleThreads_DifferentTypes()
{
// Use local options to avoid obtaining already cached metadata from the default options.
var options = new JsonSerializerOptions();

const int TestClassCount = 2;

var testObjects = new ITestClass[TestClassCount]
{
new SimpleTestClassWithNulls(),
new SimpleTestClass(),
};

foreach (ITestClass obj in testObjects)
{
obj.Initialize();
}

void Test(int i)
{
Type testClassType = testObjects[i].GetType();

string json = JsonSerializer.Serialize(testObjects[i], testClassType, options);

ITestClass obj = (ITestClass)JsonSerializer.Deserialize(json, testClassType, options);
obj.Verify();
};

const int OuterCount = 12;
Task[] tasks = new Task[OuterCount * TestClassCount];

for (int i = 0; i < tasks.Length; i += TestClassCount)
{
tasks[i + 0] = Task.Run(() => Test(TestClassCount - 1));
tasks[i + 1] = Task.Run(() => Test(TestClassCount - 2));
}

Task.WaitAll(tasks);
}

[Fact]
public static void PropertyCacheWithMinInputsFirst()
{
Expand Down

0 comments on commit 37ef28f

Please sign in to comment.