Skip to content

Commit

Permalink
address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
eiriktsarpalis committed Feb 10, 2022
1 parent fea5593 commit 0e049e7
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,7 @@ internal void ClearCaches()

private void InitializeCachingContext()
{
#if NETCOREAPP
_cachingContext = TrackedCachingContexts.GetOrCreate(this);
#else
_cachingContext = new CachingContext(this);
#endif
}

/// <summary>
Expand Down Expand Up @@ -111,7 +107,6 @@ public void Clear()
}
}

#if NETCOREAPP
/// <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"/>.
Expand Down Expand Up @@ -226,7 +221,7 @@ static int EstimateEvictionRunsToSkip(int latestEvictionCount)

// - If avgEvictionsPerRun >= 1 we skip no subsequent eviction calls.
// - If avgEvictionsPerRun < 1 we skip ~ `1 / avgEvictionsPerRun` calls.
int runsPerEviction = (int)Math.Round(1 / Math.Clamp(avgEvictionsPerRun, 0.1, 1));
int runsPerEviction = (int)Math.Round(1 / Math.Min(Math.Max(avgEvictionsPerRun, 0.1), 1));
Debug.Assert(runsPerEviction >= 1 && runsPerEviction <= 10);
return runsPerEviction - 1;
}
Expand Down Expand Up @@ -315,7 +310,18 @@ public int GetHashCode(JsonSerializerOptions options)

return hc.ToHashCode();
}
}

#if !NETCOREAPP
/// <summary>
/// Polyfill for System.HashCode.
/// </summary>
private struct HashCode
{
private int _hashCode;
public void Add<T>(T? value) => _hashCode = (_hashCode, value).GetHashCode();
public int ToHashCode() => _hashCode;
}
#endif
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ void Add(JsonConverter converter) =>
/// </remarks>
public IList<JsonConverter> Converters { get; }

internal JsonConverter GetConverterForMember(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo)
internal JsonConverter GetConverterFromMember(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo)
{
JsonConverter converter = null!;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ private static JsonConverter GetConverter(
Debug.Assert(type != null);
ValidateType(type, parentClassType, memberInfo, options);

JsonConverter converter = options.GetConverterForMember(parentClassType, type, memberInfo);
JsonConverter converter = options.GetConverterFromMember(parentClassType, type, memberInfo);

// The runtimeType is the actual value being assigned to the property.
// There are three types to consider for the runtimeType:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor
{
private sealed class Cache<TKey> where TKey : notnull
{
private int _lock;
private long _lastEvictedTicks; // tracks total number of invocations to the cache; can be allowed to overflow.
private readonly long _evictionIntervalTicks; // number of cache invocations needed before triggering an eviction run.
private int _evictLock;
private long _lastEvictedTicks; // timestamp of latest eviction operation.
private readonly long _evictionIntervalTicks; // min timespan needed to trigger a new evict operation.
private readonly long _slidingExpirationTicks; // max timespan allowed for cache entries to remain inactive.
private readonly ConcurrentDictionary<TKey, CacheEntry> _cache = new();

Expand All @@ -28,7 +28,8 @@ public Cache(TimeSpan slidingExpiration, TimeSpan evictionInterval)

public TValue GetOrAdd<TValue>(TKey key, Func<TKey, TValue> valueFactory) where TValue : class?
{
CacheEntry entry = _cache.GetOrAdd(key,
CacheEntry entry = _cache.GetOrAdd(
key,
#if NETCOREAPP
static (TKey key, Func<TKey, TValue> valueFactory) => new(valueFactory(key)),
valueFactory);
Expand All @@ -40,14 +41,15 @@ public TValue GetOrAdd<TValue>(TKey key, Func<TKey, TValue> valueFactory) where

if (utcNowTicks - Volatile.Read(ref _lastEvictedTicks) >= _evictionIntervalTicks)
{
if (Interlocked.CompareExchange(ref _lock, 1, 0) == 0)
if (Interlocked.CompareExchange(ref _evictLock, 1, 0) == 0)
{
if (utcNowTicks - _lastEvictedTicks >= _evictionIntervalTicks)
{
EvictStaleCacheEntries(utcNowTicks);
Volatile.Write(ref _lastEvictedTicks, utcNowTicks);
Volatile.Write(ref _lock, 0);
}

Volatile.Write(ref _evictLock, 0);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor : MemberAccess
public override Action<TCollection, object?> CreateAddMethodDelegate<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TCollection>()
=> s_cache.GetOrAdd((nameof(CreateAddMethodDelegate), typeof(TCollection), null),
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2091:UnrecognizedReflectionPattern",
Justification = "DynamicallyAccessedMembersAttribute is not inherited by scoped generic parameter in lambda body.")]
Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")]
static (_) => s_sourceAccessor.CreateAddMethodDelegate<TCollection>());

public override JsonTypeInfo.ConstructorDelegate? CreateConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType)
Expand All @@ -38,14 +38,14 @@ public override Action<object, TProperty> CreateFieldSetter<TProperty>(FieldInfo
public override Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>()
=> s_cache.GetOrAdd((nameof(CreateImmutableDictionaryCreateRangeDelegate), typeof((TCollection, TKey, TValue)), null),
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "Factory method calls into chained RequiresUnreferencedCode method.")]
Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")]
static (_) => s_sourceAccessor.CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>());

[RequiresUnreferencedCode(IEnumerableConverterFactoryHelpers.ImmutableConvertersUnreferencedCodeMessage)]
public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>()
=> s_cache.GetOrAdd((nameof(CreateImmutableEnumerableCreateRangeDelegate), typeof((TCollection, TElement)), null),
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "Factory method calls into chained RequiresUnreferencedCode method.")]
Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")]
static (_) => s_sourceAccessor.CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>());

public override Func<object[], T>? CreateParameterizedConstructor<T>(ConstructorInfo constructor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresen
AssertFieldNull("s_defaultSimpleConverters", optionsInstance: null);
AssertFieldNull("s_defaultFactoryConverters", optionsInstance: null);

// Confirm type info dynamic creator not set.
AssertFieldNull("s_typeInfoCreationFunc", optionsInstance: null);

static void AssertFieldNull(string fieldName, JsonSerializerOptions? optionsInstance)
{
BindingFlags bindingFlags = BindingFlags.NonPublic | (optionsInstance == null ? BindingFlags.Static : BindingFlags.Instance);
Expand Down

0 comments on commit 0e049e7

Please sign in to comment.