[PERF] Simple workflow with nested ForEach taking a while to run

[badsyntax]

Performance Improvement Request

Performance Issue Overview

Hi there, thanks for the great library. We've noticed some perf issues that we'd like clarification on. We're using Elsa to process API responses that typically involve something like this:

In cases where we have a massive data set, eg 1300+ items in a list, then the workflow takes a long time to complete (longer than 30 seconds). Most of the time is spent in the ForEach activity.

Benchmarks and Metrics

I've created an example repo that demostrates the perf issue: https://github.com/badsyntax/ElsaRepo

For this example, it processes 1300 items in a list, and takes 10 seconds to run on my MacBook Pro M1.

We've done some basic profiling using Rider, and it looks like it's making more than a billion calls to GetDescendants

9.8% < GetDescendants>b_0 • 27,141 ms / 27,191 ms • 1 081 734 156 calls • Elsa.Extensions.ActivityExecutionContextExtensions+<>c_DisplayClass22_0.<GetDescendants>b_0(ActivityExecutionCon

Looking at a much simpler example, it processes 1300 items and takes +-170ms to run on my MacBook Pro M1.

It calls GetDescendents 5 million times.

We're not very experienced with profiling, so we might be reading this all wrong, but it certainly is taking longer than we expected and hoped, and it does seem excessive to be making so many calls.

Here's the rider profiling dumps

ElsaProfiling.zip

Additional Context

Happy to provide any additional context or information you require. Is there any way this can be optimised at all?

[badsyntax]

The problem is within the MapAsync method of DefaultActivityExecutionMapper:

elsa-core/src/modules/Elsa.Workflows.Runtime/Services/DefaultActivityExecutionMapper.cs

Line 54 in 6022df1

public async Task<ActivityExecutionRecord> MapAsync(ActivityExecutionContext source)

MapAsync is called for each activity context:

elsa-core/src/modules/Elsa.Workflows.Runtime/Services/ActivityExecutionRecordExtractor.cs

Lines 13 to 14 in 6022df1

	var activityExecutionContexts = context.ActivityExecutionContexts;
	var tasks = activityExecutionContexts.Select(activityExecutionMapper.MapAsync).ToList();

For each activity context, we find the descendants, and this is what's slowing everything down:

elsa-core/src/modules/Elsa.Workflows.Runtime/Services/DefaultActivityExecutionMapper.cs

Lines 223 to 232 in 6022df1

	private static ActivityStatus GetAggregateStatus(ActivityExecutionContext context)
	{
	// If any child activity is faulted, the aggregate status is faulted.
	var descendantContexts = context.GetDescendants().ToList();
	if (descendantContexts.Any(x => x.Status == ActivityStatus.Faulted))
	return ActivityStatus.Faulted;
	return context.Status;
	}

I attempted to improve the perf here but wasn't able to come up with anything effective. I do reckon this can optimised somehow. The problem is that there's just so many activity contexts and calling GetDescendants recursively to determine the actvity status is an expensive operation, which is only evident when working large data sets in a foreach. The perf problem is exacerbated what seems to be exponntially by adding more activies or by adding more items to the ForEach activity.

I've been able to find a workaround that improves the perf rather significantly by not enabling ActivityExecutionLogPersistence && WorkflowExecutionLogPersistence middlewares. By doing so, the example goes from 10s to 2s for me, and in other example it goes from 16s to 5.5s, which is great, as we don't require log persistence.

Here's an example:

services.AddElsa(elsa =>
{
    elsa.UseWorkflows(workflows =>
    {
        workflows.WithWorkflowExecutionPipeline(pipeline => pipeline
            .Reset()
            .UseEngineExceptionHandling()
            .UseExceptionHandling()
            .UseDefaultActivityScheduler());
    });
});