From eb43ff4c98157a75ade9abc6706f581d79a95fa3 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Sun, 11 Oct 2026 18:59:41 +0200 Subject: [PATCH] Drop the extra MruCache and resolve public fields lazily into the existing cache entry --- src/NLog/Internal/ObjectReflectionCache.cs | 48 +++++++++++-------- .../DefaultJsonSerializerClassTests.cs | 18 +++++++ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/NLog/Internal/ObjectReflectionCache.cs b/src/NLog/Internal/ObjectReflectionCache.cs index 59b170d..98aaba0 100644 --- a/src/NLog/Internal/ObjectReflectionCache.cs +++ b/src/NLog/Internal/ObjectReflectionCache.cs @@ -49,8 +49,6 @@ namespace NLog.Internal { private MruCache ObjectTypeCache => _objectTypeCache ?? System.Threading.Interlocked.CompareExchange(ref _objectTypeCache, new MruCache(10000), null) ?? _objectTypeCache; private MruCache? _objectTypeCache; - private MruCache ObjectTypeFieldCache => _objectTypeFieldCache ?? System.Threading.Interlocked.CompareExchange(ref _objectTypeFieldCache, new MruCache(10000), null) ?? _objectTypeFieldCache; - private MruCache? _objectTypeFieldCache; private readonly IServiceProvider _serviceProvider; private IObjectTypeTransformer ObjectTypeTransformation => _objectTypeTransformation ?? (_objectTypeTransformation = _serviceProvider?.GetService() ?? this); private IObjectTypeTransformer? _objectTypeTransformation; @@ -97,13 +95,14 @@ namespace NLog.Internal } var objectType = value.GetType(); - var typeCache = includeFields ? ObjectTypeFieldCache : ObjectTypeCache; - if (!typeCache.TryGetValue(objectType, out var propertyInfos)) + var propertyInfos = CheckForObjectProperties(value) ? BuildObjectPropertyInfos(value, includeFields) : ObjectPropertyInfos.SimpleToString; + if (includeFields && propertyInfos.FieldLookup is null) { - propertyInfos = CheckForObjectProperties(value) ? BuildObjectPropertyInfos(value, includeFields) : ObjectPropertyInfos.SimpleToString; - typeCache.TryAddValue(objectType, propertyInfos); + // Remember that the public fields were resolved, so the type is not reflected again + propertyInfos = new ObjectPropertyInfos(propertyInfos.Properties, propertyInfos.FastLookup, ArrayHelper.Empty()); } - return new ObjectPropertyList(value, propertyInfos.Properties, propertyInfos.FastLookup); + ObjectTypeCache.TryAddValue(objectType, propertyInfos); + return propertyInfos.CreatePropertyList(value, includeFields); } /// @@ -162,15 +161,15 @@ namespace NLog.Internal #endif Type objectType = value.GetType(); - if (!includeFields && ObjectTypeCache.TryGetValue(objectType, out var propertyInfos)) + if (ObjectTypeCache.TryGetValue(objectType, out var propertyInfos) && !(includeFields && propertyInfos.NeedsFieldLookup)) { if (!propertyInfos.HasFastLookup) { var fastLookup = BuildFastLookup(propertyInfos.Properties, false); - propertyInfos = new ObjectPropertyInfos(propertyInfos.Properties, fastLookup); + propertyInfos = new ObjectPropertyInfos(propertyInfos.Properties, fastLookup, propertyInfos.FieldLookup); ObjectTypeCache.TryAddValue(objectType, propertyInfos); } - objectPropertyList = new ObjectPropertyList(value, propertyInfos.Properties, propertyInfos.FastLookup); + objectPropertyList = propertyInfos.CreatePropertyList(value, includeFields); return true; } @@ -200,15 +199,12 @@ namespace NLog.Internal } else if (properties.Length == 0) { - // Nothing to show as properties, so fall back to the public fields when asked to. - var fields = includeFields ? GetPublicFields(objectType) : ArrayHelper.Empty(); - if (fields.Length > 0) - { - // Fields are only reachable through the fast-lookup, so it cannot be built lazily - return new ObjectPropertyInfos(ArrayHelper.Empty(), BuildFieldLookup(fields)); - } + if (!includeFields) + return ObjectPropertyInfos.SimpleToString; - return ObjectPropertyInfos.SimpleToString; + // Nothing to show as properties, so keep the public fields aside for the IncludePublicFields fallback + var fields = GetPublicFields(objectType); + return new ObjectPropertyInfos(ArrayHelper.Empty(), ArrayHelper.Empty(), BuildFieldLookup(fields)); } else { @@ -626,20 +622,32 @@ namespace NLog.Internal { public readonly PropertyInfo[] Properties; public readonly FastPropertyLookup[]? FastLookup; + /// Public fields of a type without public properties, null until resolved + public readonly FastPropertyLookup[]? FieldLookup; public static readonly ObjectPropertyInfos SimpleToString = new ObjectPropertyInfos(ArrayHelper.Empty(), ArrayHelper.Empty()); - public ObjectPropertyInfos(PropertyInfo[] properties, FastPropertyLookup[]? fastLookup) + public ObjectPropertyInfos(PropertyInfo[] properties, FastPropertyLookup[]? fastLookup, FastPropertyLookup[]? fieldLookup = null) { Properties = properties; FastLookup = fastLookup; + FieldLookup = fieldLookup; } public bool HasFastLookup => FastLookup != null; + public bool NeedsFieldLookup => FieldLookup is null && Properties.Length == 0 && FastLookup?.Length == 0; + + public ObjectPropertyList CreatePropertyList(object value, bool includeFields) + { + if (includeFields && FieldLookup?.Length > 0) + return new ObjectPropertyList(value, Properties, FieldLookup); + return new ObjectPropertyList(value, Properties, FastLookup); + } + public bool Equals(ObjectPropertyInfos other) { - return ReferenceEquals(Properties, other.Properties) && FastLookup?.Length == other.FastLookup?.Length; + return ReferenceEquals(Properties, other.Properties) && FastLookup?.Length == other.FastLookup?.Length && ReferenceEquals(FieldLookup, other.FieldLookup); } } diff --git a/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs b/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs index fd4cf7a..4ff5740 100644 --- a/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs +++ b/tests/NLog.UnitTests/Targets/DefaultJsonSerializerClassTests.cs @@ -246,6 +246,24 @@ namespace NLog.UnitTests.Targets Assert.Equal(@"{""Name"":""John"",""Age"":42}", withFieldsAgain.ToString()); } + [Fact] + public void IncludePublicFields_SameTypeWithoutOptionFirst_StillSerializesFieldsLater() + { + var testObject = new ClassWithOnlyFields { Name = "John", Age = 42 }; + var jsonSerializer = new DefaultJsonSerializer(null); + + var withoutFields = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withoutFields, new JsonSerializeOptions { IncludePublicFields = false }); + var withFields = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withFields, new JsonSerializeOptions { IncludePublicFields = true }); + var withoutFieldsAgain = new StringBuilder(); + jsonSerializer.SerializeObject(testObject, withoutFieldsAgain, new JsonSerializeOptions { IncludePublicFields = false }); + + Assert.DoesNotContain(@"""Age"":42", withoutFields.ToString()); + Assert.Equal(@"{""Name"":""John"",""Age"":42}", withFields.ToString()); + Assert.Equal(withoutFields.ToString(), withoutFieldsAgain.ToString()); + } + [Fact] public void IncludePublicFields_ShadowedField_IsWrittenOnce() {