msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

Perf: Replace ItemExpander transforms with simple jump list

Open ccastanedaucf opened this issue 4 months ago • 1 comments

Fixes

Reduces millions of object allocations from ItemExpander intrinsic function path.

Context

As a high level, ItemExpander.ExpandExpressionCapture follows this flow:

  • Take in a set of strings and optional arguments that were parsed via regex.
  • Map them to a matching static function.
  • Run an iterative transform on the list of items.
  • Repeat until done.

Right now though, this actually looks more like this:

  • Create a string based on the item type and the function name to create a lookup into a ConcurrentDictionary cache...
    • If not there, use reflection to lookup the static method on IntrinsicItemFunctions
string qualifiedFunctionName = $"{itemType.FullName}::{functionName}";
if (!s_transformFunctionDelegateCache.TryGetValue(qualifiedFunctionName, out transformFunction))
{
	MethodInfo itemFunctionInfo = typeof(IntrinsicItemFunctions<S>).GetMethod(functionName, BindingFlags.IgnoreCase | BindingFlags.NonPublic | BindingFlags.Static);
	transformFunction = (ItemTransformFunction)itemFunctionInfo.CreateDelegate(typeof(ItemTransformFunction));
}
  • Build a stack of function calls + arguments...
Stack<TransformFunction<S>> transformFunctionStack = new Stack<TransformFunction<S>>(match.Captures.Count);
for (int n = match.Captures.Count - 1; n >= 0; n--)
{
	// ... get regex args
	IntrinsicItemFunctions<S>.ItemTransformFunction transformFunction = IntrinsicItemFunctions<S>.GetItemTransformFunction(elementLocation, functionName, typeof(S));
	transformFunctionStack.Push(new TransformFunction<S>(elementLocation, functionName, transformFunction, arguments));
}
  • Recursively build an enumeration of the stack, running each transform from the result of the parent.
// Transform(args)
	TransformFunction<S> function = transformFunctionStack.Pop();
	
	foreach (KeyValuePair<string, S> item in Transform(expander, includeNullEntries, transformFunctionStack, function.Execute(expander, includeNullEntries, itemsOfType)))
	{
		yield return item;
	}

Due to all the intermediate object types, enumerators, layers of indirection, and the frequency at which this is called - the wrapping code alone allocates more memory and nearly as much CPU as the actual function calls.

Changes Made

This change removes all of the reflection and intermediate objects in favor of essentially just doing the high-level flow described above.

Before: image

After, most of the allocations are coming from the intrinsic function implementations (vast majority being Regex.Replace... which I may also have a reduction for 😉).

image

Samples of some types and the number of objects that were being produced (before left, after right):

image

image

image

image

Testing

Tested builds on full repos (OrchardCore, CB) as well.

Notes

After this, ExpanderOptions.BreakOnNotEmpty will currently through a full transform first before checking for non-empty elements and breaking. If needed it would be easy to add a check to at least the initial set of items (aka before any transform runs), and otherwise just a fair amount of plumbing to pass it into every function if immediate return is necessary. I'm not familiar with how often that path gets hit.

ccastanedaucf avatar Jun 06 '25 23:06 ccastanedaucf