From 335922e1704c581a286a761e9772ac91ac9bdda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 24 Aug 2026 13:15:48 +0200 Subject: [PATCH] AVRO-3893: [csharp] Avoid per-call closure allocation in ObjectCreator.FindType ObjectCreator.FindType passed an inline lambda to ConcurrentDictionary.GetOrAdd that discarded the key parameter ((_)) and captured the local `name` instead. Capturing a local forces the compiler to allocate a new display-class closure and delegate on every call, even on cache hits, which showed up as a significant share of allocations on the deserialization hot path when using a PreresolvingDatumReader. Extract the value factory into a FindTypeUncached(string) method and store a single cached Func delegate (findTypeFactory), created once in the constructor. The factory now uses its `name` parameter (the cache key supplied by GetOrAdd) instead of a captured local, so no closure is allocated per lookup. Behaviour is unchanged. The CA1031 suppression is retargeted to the extracted method. --- .../src/apache/main/GlobalSuppressions.cs | 2 +- .../src/apache/main/Specific/ObjectCreator.cs | 98 +++++++++++-------- 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/lang/csharp/src/apache/main/GlobalSuppressions.cs b/lang/csharp/src/apache/main/GlobalSuppressions.cs index 64b5fb8b774..16e1576dbea 100644 --- a/lang/csharp/src/apache/main/GlobalSuppressions.cs +++ b/lang/csharp/src/apache/main/GlobalSuppressions.cs @@ -38,7 +38,7 @@ [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1000:Do not declare static members on generic types", Justification = "Maintain public API", Scope = "member", Target = "~M:Avro.File.DataFileWriter`1.OpenAppendWriter(Avro.Generic.DatumWriter{`0},System.String)~Avro.File.IFileWriter{`0}")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1000:Do not declare static members on generic types", Justification = "Maintain public API", Scope = "member", Target = "~M:Avro.File.DataFileWriter`1.OpenAppendWriter(Avro.Generic.DatumWriter{`0},System.IO.Stream,System.IO.Stream)~Avro.File.IFileWriter{`0}")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Purposely catch any exception", Scope = "member", Target = "~M:Avro.File.DataFileReader`1.Sync(System.Int64)")] -[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Purposely catch any exception", Scope = "member", Target = "~M:Avro.Specific.ObjectCreator.FindType(System.String)~System.Type")] +[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Purposely catch any exception", Scope = "member", Target = "~M:Avro.Specific.ObjectCreator.FindTypeUncached(System.String)~System.Type")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Purposely catch any exception", Scope = "member", Target = "~M:Avro.Specific.ObjectCreator.GetType(Avro.Schema)~System.Type")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1051:Do not declare visible instance fields", Justification = "Maintain public API", Scope = "member", Target = "~F:Avro.CodeGen.namespaceLookup")] [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1051:Do not declare visible instance fields", Justification = "Maintain public API", Scope = "member", Target = "~F:Avro.Generic.GenericFixed.value")] diff --git a/lang/csharp/src/apache/main/Specific/ObjectCreator.cs b/lang/csharp/src/apache/main/Specific/ObjectCreator.cs index 073b107958a..f1831e21cd6 100644 --- a/lang/csharp/src/apache/main/Specific/ObjectCreator.cs +++ b/lang/csharp/src/apache/main/Specific/ObjectCreator.cs @@ -58,6 +58,10 @@ public sealed class ObjectCreator private readonly Assembly entryAssembly; private readonly bool diffAssembly; + // Cached value factory for typeCacheByName.GetOrAdd. Stored once so that + // FindType does not allocate a new closure on every call (AVRO-3893). + private readonly Func findTypeFactory; + /// /// Initializes a new instance of the class. /// @@ -69,6 +73,8 @@ public ObjectCreator() // entryAssembly returns null when running from NUnit diffAssembly = entryAssembly != null && execAssembly != entryAssembly; + + findTypeFactory = FindTypeUncached; } /// @@ -81,62 +87,70 @@ public ObjectCreator() /// private Type FindType(string name) { - return typeCacheByName.GetOrAdd(name, (_) => - { - Type type = null; + return typeCacheByName.GetOrAdd(name, findTypeFactory); + } - if (TryGetIListItemTypeName(name, out var itemTypeName)) - { - return GenericIListType.MakeGenericType(FindType(itemTypeName)); - } + /// + /// Value factory used by . Kept as a separate method (referenced + /// through the cached findTypeFactory delegate) so the lookup does not capture a + /// per-call closure. The argument is the cache key supplied by + /// . + /// + private Type FindTypeUncached(string name) + { + Type type = null; - if (TryGetNullableItemTypeName(name, out itemTypeName)) - { - return GenericNullableType.MakeGenericType(FindType(itemTypeName)); - } + if (TryGetIListItemTypeName(name, out var itemTypeName)) + { + return GenericIListType.MakeGenericType(FindType(itemTypeName)); + } - // if entry assembly different from current assembly, try entry assembly first - if (diffAssembly) - { - type = entryAssembly.GetType(name); - } + if (TryGetNullableItemTypeName(name, out itemTypeName)) + { + return GenericNullableType.MakeGenericType(FindType(itemTypeName)); + } - // try current assembly and mscorlib - if (type == null) - { - type = Type.GetType(name); - } + // if entry assembly different from current assembly, try entry assembly first + if (diffAssembly) + { + type = entryAssembly.GetType(name); + } + + // try current assembly and mscorlib + if (type == null) + { + type = Type.GetType(name); + } - // type is still not found, need to loop through all loaded assemblies - if (type == null) + // type is still not found, need to loop through all loaded assemblies + if (type == null) + { + foreach (Assembly assembly in AppDomain.CurrentDomain.GetAssemblies()) { - foreach (Assembly assembly in AppDomain.CurrentDomain.GetAssemblies()) + // Loading all types from all assemblies could fail for a variety of + // non -fatal reasons. If we fail to load types from an assembly, continue. + try { - // Loading all types from all assemblies could fail for a variety of - // non -fatal reasons. If we fail to load types from an assembly, continue. - try + // Change the search to look for Types by both NAME and FULLNAME + foreach (Type t in assembly.GetTypes()) { - // Change the search to look for Types by both NAME and FULLNAME - foreach (Type t in assembly.GetTypes()) + if (name == t.Name || name == t.FullName || CodeGenUtil.Instance.UnMangle(name) == t.FullName) { - if (name == t.Name || name == t.FullName || CodeGenUtil.Instance.UnMangle(name) == t.FullName) - { - type = t; - break; - } + type = t; + break; } } - catch - { - continue; - } + } + catch + { + continue; } } + } - return type - ?? throw new AvroException($"Unable to find type '{name}' in all loaded " + - $"assemblies"); - }); + return type + ?? throw new AvroException($"Unable to find type '{name}' in all loaded " + + $"assemblies"); } private bool TryGetIListItemTypeName(string name, out string itemTypeName)