AVRO-3893: [csharp] Avoid per-call closure allocation in ObjectCreator.FindType

[iemejia]

What changes were proposed in this pull request?

ObjectCreator.FindType passed an inline lambda to ConcurrentDictionary.GetOrAdd:

return typeCacheByName.GetOrAdd(name, (_) =>
{
    ...
    if (TryGetIListItemTypeName(name, out var itemTypeName)) { ... }   // captures `name`
    ...
});

The lambda discards the key parameter ((_)) and instead captures the local name. Capturing a local forces the C# compiler to allocate a new display-class closure (and delegate) on every call — including cache hits, since the factory delegate is constructed as an argument regardless of whether GetOrAdd invokes it. As reported in AVRO-3893, this accounted for ~10% of allocations on a deserialization hot path using PreresolvingDatumReader.

How was this patch fixed?

• Extract the value factory into a private FindTypeUncached(string name) method.
• Store a single cached Func<string, Type> findTypeFactory delegate, created once in the constructor, and pass it to GetOrAdd.
• The factory uses its name parameter (the cache key supplied by GetOrAdd) instead of a captured local, so no closure is allocated per lookup.

ObjectCreator is a shared singleton (ObjectCreator.Instance), so the factory delegate is effectively allocated once for the process. Behaviour is unchanged; the CA1031 suppression is retargeted from FindType to the extracted FindTypeUncached.

How was this patch tested?

• dotnet build of Avro.main succeeds with 0 warnings (confirming the retargeted suppression).
• Avro.test Specific/ObjectCreator suites pass: 96/96 across net6.0, net7.0, and net8.0.

[zcsizmadia]

Thanks, @iemejia — this looks correct to me.

I checked the allocation claim locally (net8.0). Without this change, each cache hit through ObjectCreator.GetType(string, Schema.Type) allocates 96 bytes: the display-class closure plus the delegate. With this change it allocates 0. The refactor otherwise leaves behaviour unchanged. The recursive FindType calls for IList<>/Nullable<> item types still go through the cache as before, and the suppression retarget matches the new member signature.

Storing the delegate in a field is the right approach. Passing the method group FindTypeUncached directly would still allocate a new delegate on every call, because the method is an instance method. Assigning it in the constructor rather than in a field initializer is also necessary, since the initializer can't reference this.

One request: could you add a regression test so this doesn't quietly come back? Something like:

[Test]
public void TestGetTypeCacheHitDoesNotAllocate()
{
    var objectCreator = new ObjectCreator();
    string name = typeof(Foo).FullName;

    // Warm the cache and JIT.
    for (int i = 0; i < 100; i++)
    {
        objectCreator.GetType(name, Schema.Type.Record);
    }

    long before = GC.GetAllocatedBytesForCurrentThread();
    for (int i = 0; i < 10000; i++)
    {
        objectCreator.GetType(name, Schema.Type.Record);
    }
    long allocated = GC.GetAllocatedBytesForCurrentThread() - before;

    Assert.AreEqual(0, allocated, $"Cache hits allocated {allocated} bytes over 10000 calls");
}

It passes on this branch and fails on main with 960000 bytes. GC.GetAllocatedBytesForCurrentThread is available on every test target framework (net6.0/7.0/8.0).

LGTM with or without the test.