Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

@TimothyMakkison TimothyMakkison commented Jan 17, 2026

Use GetOrAdd overload and pass state in to avoid creating Func or closures

  • I think it's possible to do this in GetOrCreateClassContext, but the trimmer analyser doesn't like this. I'd assume it's safe to allow this, but I'm not familiar with AOT requirements

@thomhurst
Copy link
Owner

Summary

Performance optimization to eliminate closure allocations in GetOrAdd calls by using the state-passing overload.

Critical Issues

None found ✅

Suggestions

Consider applying the same optimization to GetOrCreateClassContext:

While I understand the trimmer analyzer concern mentioned in the PR description, the pattern should be safe to apply to GetOrCreateClassContext as well. The [DynamicallyAccessedMembers] attribute on classType is a compile-time annotation for the trimmer - it doesn't affect runtime behavior or closure capture.

The change would look like:

return _classContexts.GetOrAdd(classType, static (type, provider) =>
    new ClassHookContext(provider.GetOrCreateAssemblyContext(type.Assembly))
    {
        ClassType = type
    }, this);

The trimmer warning (if any) can likely be suppressed with [UnconditionalSuppressMessage] since the Type is already annotated at the method boundary. However, if the trimmer truly blocks this, the current state is acceptable.

Performance verification:

These changes touch hot paths (ContextProvider.cs:66, ObjectInitializer.cs:108). Have you benchmarked the allocation reduction? While the changes look correct, it would be valuable to quantify the improvement given TUnit's performance-first mandate.

Verdict

APPROVE - Clean performance optimization with no critical issues

@thomhurst thomhurst merged commit f19bb84 into thomhurst:main Jan 17, 2026
9 of 10 checks passed
This was referenced Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants