Incorporate review feedback from #64646 (#65376)

* Incorporate review feedback from
#64646

* Prevent trimming of unused CachingContext.Count method

* use correct syntax for nested types

Author: eiriktsarpalis

src/libraries/System.Text.Json/src/ILLink/ILLink.Descriptors.LibraryBuild.xml

@@ -0,0 +1,8 @@
+<linker>
+  <assembly fullname="System.Text.Json">
+    <type fullname="System.Text.Json.JsonSerializerOptions/CachingContext">
+      <!-- Internal API used by tests only. -->
+      <method name="get_Count" />
+    </type>
+  </assembly>
+</linker>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConverterList.cs

@@ -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)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

@@ -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
 {
@@ -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)
         {
@@ -90,10 +90,11 @@ internal sealed class CachingContext
             public CachingContext(JsonSerializerOptions options)
             {
                 Options = options;
-                _ = Count;
             }
 
             public JsonSerializerOptions Options { get; }
+            // Property only accessed by reflection in testing -- do not remove.
+            // If changing please ensure that src/ILLink.Descriptors.LibraryBuild.xml is up-to-date.
             public int Count => _converterCache.Count + _jsonTypeInfoCache.Count;
             public JsonConverter GetOrAddConverter(Type type) => _converterCache.GetOrAdd(type, Options.GetConverterFromType);
             public JsonTypeInfo GetOrAddJsonTypeInfo(Type type) => _jsonTypeInfoCache.GetOrAdd(type, Options.GetJsonTypeInfoFromContextOrCreate);
@@ -109,7 +110,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
@@ -188,6 +189,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;
@@ -252,7 +255,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)
             {
@@ -276,9 +279,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)
@@ -321,9 +324,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();

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

@@ -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)
         {
@@ -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))
                 {

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

@@ -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;
@@ -65,7 +66,7 @@ public sealed partial class JsonSerializerOptions
         /// </summary>
         public JsonSerializerOptions()
         {
-            Converters = new ConverterList(this);
+            _converters = new ConverterList(this);
             TrackOptionsInstance(this);
         }
 
@@ -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;