Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Incorporate review feedback from
  • Loading branch information
eiriktsarpalis committed Feb 15, 2022
commit 6a4d1404e557f65136334a17e257c75b84272c8e
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ namespace System.Text.Json.Serialization
/// </summary>
internal sealed class ConverterList : IList<JsonConverter>
{
private readonly List<JsonConverter> _list = new List<JsonConverter>();
private readonly List<JsonConverter> _list;
private readonly JsonSerializerOptions _options;

public ConverterList(JsonSerializerOptions options)
{
_options = options;
_list = new();
}

public ConverterList(JsonSerializerOptions options, ConverterList source)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Runtime.CompilerServices;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
using System.Threading;

namespace System.Text.Json
{
Expand All @@ -20,8 +21,7 @@ public sealed partial class JsonSerializerOptions
private CachingContext? _cachingContext;

// Simple LRU cache for the public (de)serialize entry points that avoid some lookups in _cachingContext.
// Although this may be written by multiple threads, 'volatile' was not added since any local affinity is fine.
private JsonTypeInfo? _lastTypeInfo;
private volatile JsonTypeInfo? _lastTypeInfo;

internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type)
{
Expand Down Expand Up @@ -109,7 +109,7 @@ public void Clear()

/// <summary>
/// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse
/// this approach uses a regular dictionary pointing to weak references of <see cref="CachingContext"/>.
/// this approach uses a concurrent dictionary pointing to weak references of <see cref="CachingContext"/>.
/// Relevant caching contexts are looked up using the equality comparison defined by <see cref="EqualityComparer"/>.
/// </summary>
internal static class TrackedCachingContexts
Expand Down Expand Up @@ -188,6 +188,8 @@ private static bool TryEvictDanglingEntries()
// across multiple initializations. The backoff count is determined by the eviction
// rates of the most recent runs.

Debug.Assert(Monitor.IsEntered(s_cache));

if (s_evictionRunsToSkip > 0)
{
--s_evictionRunsToSkip;
Expand Down Expand Up @@ -252,7 +254,7 @@ static int EstimateEvictionRunsToSkip(int latestEvictionCount)
/// If two instances are equivalent, they should generate identical metadata caches;
/// the converse however does not necessarily hold.
/// </summary>
private class EqualityComparer : IEqualityComparer<JsonSerializerOptions>
private sealed class EqualityComparer : IEqualityComparer<JsonSerializerOptions>
{
public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
{
Expand All @@ -276,9 +278,9 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
left._writeIndented == right._writeIndented &&
left._serializerContext == right._serializerContext &&
CompareConverters(left.Converters, right.Converters);
CompareConverters(left._converters, right._converters);

static bool CompareConverters(IList<JsonConverter> left, IList<JsonConverter> right)
static bool CompareConverters(ConverterList left, ConverterList right)
{
int n;
if ((n = left.Count) != right.Count)
Expand Down Expand Up @@ -321,9 +323,9 @@ public int GetHashCode(JsonSerializerOptions options)
hc.Add(options._writeIndented);
hc.Add(options._serializerContext);

foreach (JsonConverter converter in options.Converters)
for (int i = 0; i < options._converters.Count; i++)
{
hc.Add(converter);
hc.Add(options._converters[i]);
}

return hc.ToHashCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ void Add(JsonConverter converter) =>
/// <remarks>
/// Once serialization or deserialization occurs, the list cannot be modified.
/// </remarks>
public IList<JsonConverter> Converters { get; }
public IList<JsonConverter> Converters => _converters;

internal JsonConverter GetConverterFromMember(Type? parentClassType, Type propertyType, MemberInfo? memberInfo)
{
Expand Down Expand Up @@ -183,7 +183,7 @@ private JsonConverter GetConverterFromType(Type typeToConvert)

// Priority 2: Attempt to get custom converter added at runtime.
// Currently there is not a way at runtime to override the [JsonConverter] when applied to a property.
foreach (JsonConverter item in Converters)
foreach (JsonConverter item in _converters)
{
if (item.CanConvert(typeToConvert))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public sealed partial class JsonSerializerOptions
private JsonCommentHandling _readCommentHandling;
private ReferenceHandler? _referenceHandler;
private JavaScriptEncoder? _encoder;
private ConverterList _converters;
private JsonIgnoreCondition _defaultIgnoreCondition;
private JsonNumberHandling _numberHandling;
private JsonUnknownTypeHandling _unknownTypeHandling;
Expand All @@ -65,7 +66,7 @@ public sealed partial class JsonSerializerOptions
/// </summary>
public JsonSerializerOptions()
{
Converters = new ConverterList(this);
_converters = new ConverterList(this);
TrackOptionsInstance(this);
}

Expand Down Expand Up @@ -98,7 +99,7 @@ public JsonSerializerOptions(JsonSerializerOptions options!!)
_propertyNameCaseInsensitive = options._propertyNameCaseInsensitive;
_writeIndented = options._writeIndented;

Converters = new ConverterList(this, (ConverterList)options.Converters);
_converters = new ConverterList(this, options._converters);
EffectiveMaxDepth = options.EffectiveMaxDepth;
ReferenceHandlingStrategy = options.ReferenceHandlingStrategy;

Expand Down