perf: make methods synchronous and prevent closure creation

[TimothyMakkison]

Largest memory saving, I reckon I'll be able to make by eliding state machines and removing redundant closures.

• Directly return a task and remove the async keyword to avoid allocating a state machine

  • Note that the stack trace from invoked methods which throw will no longer include this function. IMO this won't affect debugging as these methods are pretty linear 🤷

• Methods like CreateStaticHookDelegateAsync were creating a Func<Task> via CreateTimeoutHookAction before immediately invoking and awaiting it. I changed it to directly return a Task as the defered execution was never used.

  • This would allocate an unneeded state machine and there was no reason to create a delegate if you are immediately going to invoke it

• Get int timoutMs before passing it into CreateTimeoutHookActionAsync, this makes the resulting state machine 8 bytes smaller because it can pack the 4 byte integer with the state machine state integer.

• Move HookTimeoutHelper.ExecuteHookWithPotentialCustomExecutor to a different method to prevent closure creation.

  • Even though 99% of method calls end up using the default hook executor via hook.ExecuteAsync, the compiler will still eagerly generate and allocate the closures even when they aren't needed.

This saves around 180MB on TUnit.TestProject around 10% of total memory usage.

Before

After

Note that TUnit.Engine.Helpers.HookTimeoutHelper+<<CreateTimeoutHookAction>g__CreateTimeoutHookActionAsync|0_0>d<TestContext> is large because it replaces a Func<Task>, a closure for CreateTimeoutHookAction and an async state machine costing around 150MB total.

[TimothyMakkison]

A lot of these techniques can be applied to other areas.
For instance: AsyncConvert, I'm not sure what ValueTask Convert(Func<Task> func) is doing. Couldn't it just return new ValueTask(func()); I have no idea whether the if statement is doing anything.

[thomhurst]

This sounds like a good saving!

AsyncConverter probably can be improved. Initially wanted to move it to the source generator to reduce as much as possible, but the logic is just way more complicated than runtime checks

[thomhurst]

Summary

Performance optimization that eliminates redundant closures and state machines in hook execution, saving ~180MB (10% of total) on TUnit.TestProject.

Critical Issues

None found ✅

Suggestions

1. Missing using directive in new code

The diff shows using System.Diagnostics.CodeAnalysis; is added to HookTimeoutHelper.cs, but I don't see [DisallowNull] in the final diff for the StaticHookMethod overload. However, the new helper methods ExecuteBeforeTestHook and ExecuteAfterTestHook do use [DisallowNull] T context. This is good for AOT compatibility (TUnit Rule #5), but verify the annotation is needed given that:

• The context is already constrained by the calling code
• The methods are private helpers

If the annotation isn't necessary for the AOT scenario, consider removing it to keep the change minimal.

2. Consider ValueTask optimization for no-timeout path

In HookTimeoutHelper.cs:28, the no-timeout case now does:

return ExecuteHookWithPotentialCustomExecutor(hook, context, cancellationToken).AsTask();

Since ExecuteHookWithPotentialCustomExecutor returns ValueTask, and the caller expects Task, you're forced to convert. However, the calling code in HookCollectionService could potentially be refactored to return ValueTask instead of Task if the lambda return type was ValueTask instead of Task. This would save the .AsTask() allocation when the ValueTask completes synchronously.

This is a minor point - the current change is already excellent. Just noting for future optimization.

Observations

✅ Excellent alignment with TUnit Rule #4 (Performance First): This change demonstrates deep understanding of async state machine allocations and closure capture
✅ Correctness preserved: The semantic change from Func<Task> to Task is correctly handled by keeping deferred execution in the caller lambdas
✅ Local static async method pattern: Smart use of local static async method in the timeout path to avoid captures while keeping the code organized
✅ Benchmark data provided: The PR includes clear before/after memory profiles showing the impact

Verdict

✅ APPROVE - Excellent performance optimization with no correctness issues