Performance Issues When using GetOldRecord in Flow

[mitchspano]

Expected Behavior

I expect the GetOldRecord action to perform reasonably fast since it is not making any queries.

Actual Behavior

GetOldRecord performs pretty slowly.

Steps to Reproduce the Problem

Create a new scratch org
Push Trigger Actions Framework to the org
Enable Trigger Actions on the Case sObject
Create a new Trigger Action Flow on Case which calls GetOldRecord for each record:

Update 200 cases in the system.

Long before = System.currentTimeMillis();
update [Select ID FROM Case];
Long after = System.currentTimeMillis();
System.debug('MS to perform "GetOldRecord" on 200 cases : ' + (after - before));

10:34:48:863 USER_DEBUG [4]|DEBUG|MS to perform "GetOldRecord" on 200 cases : 15861

[benjaminloerincz]

I just took a closer look at the situation and was surprised that this Invocable Method consumes so much time. I tried to keep the transfer to the flow a bit simpler: Instead of the Current Record, you could also just pass the Current Record Id. Unfortunately, this small optimisation doesn't help much.

I suspect that quite a lot of performance is lost here due to serialisation and deserialisation between Apex and Flows and assigning variables.

Unfortunately you can't access Trigger.old within the Invocable Method, then you wouldn't have to pass oldList....

[mitchspano]

These performance issues are rather concerning. I am going to try a couple of things:

1. I wonder if there would be a meaning performance difference if we passed an individual record to the flow, then have the trigger kick off one flow per record. I doubt this would improve performance, but I'm going to try it out.

2. There may be a better way to fetch the old record without the use of an invocable method; perhaps with a subflow; perhaps just ensuring the newList and oldList share the same record ordering and finding the old record by the index of the current.

[surajp]

Looks like there might be a simple solve for this issue. You can create a static variable of type Map<Id,SObject> (say, localOldMap) and assign Trigger.oldMap to it in the static block in TriggerActionFlow. You then don't have to pass in the oldList to the flow and back, and can use the static variable localOldMap to get the old record.

[surajp]

Forgot to note in my previous comment that with this, even though we are seeing ~50% reduction in time taken, its still not great. One thing to try would be to directly pass in the record id instead of the FlowInput object as noted above.

[mitchspano]

FYI, I posted an Idea in the community asking for the ability to define an Apex data type which includes an sObject as an @AuraEnabled instance variable for use in flows. If we could do that, then we could use an Apex defined data type in the flow like this:

public class TriggerRecord {
    @AuraEnabled sObject newRecord;
    @AuraEnabled sObject oldRecord;
}

This would allow us to iterate over a collection of TriggerRecord objects instead of injecting the newList and oldList variables at flow instantiation time and solve this issue.

[surajp]

You could pass in a single newRecord and oldRecord into the flow, into separate input record variables currently. Is there a downside to that approach over using the Apex defined data type?

[mitchspano]

"You could pass in a single newRecord and oldRecord into the flow, into separate input record variables currently."

The issue with that approach would be that you then need to kick off up to 200 flow instances.

I did try this out and the code is present in this branch.
One thing which is nice about this approach is that it makes the approach for writing the flows a bit more approachable.

Here is an out of the box record triggered flow:

And here is the same flow written in this new version of TriggerActionFlow:

This appears to operate a little better, with a performance time of about 3.5 seconds per 200 records, which is still rather unusable in my opinion, but there are some serious downfalls:

1. Writing the flows in this way will take about 3.5 seconds to execute no matter if you want to check old record values or not.
2. The "GetRecords" flow element would immediately break if more than 99 records were processed, since the SOQL would be executed for all of the unique flow instances.

[surajp]

I see your point. Makes sense. The Apex defined type would help get around that for sure. However, I do see a bigger issue with the approach of invoking flows from triggers in general and that is the inability to bulkiify record operations and invocable actions within the flow. As we have already seen, even soql-less invocables consume a significant amount of CPU time. I think there should be a disclaimer with the framework in general that it is not recommended to use the flow-based approach if you have invocable actions or database operations in your flow.

Also, regarding "The "GetRecords" flow element would immediately break if more than 99 records were processed, since the SOQL would be executed for all of the unique flow instances." like i said above, if you store the value of Trigger.oldMap in a static variable within the invocable class, you can use an operation like staticOldMap.get(record.Id) and avoid SOQL for getting the old record if you are invoking the flow once for each record. So, there wouldn't be an issue processing more than 99 records.

[mitchspano]

Exploration

So I was playing around with this last night and I tried a few things:

1. Perform an n^2 loop over oldList and newList to find the old record.
   This actually works surprisingly well but would require a bit of repeated boilerplate flow elements:

2. Use the index in the newList to set values before insert/update instead of newListAfterFlow.
   The serialization of adding records to newListAfterFlow caused some performance issues, so I put the newList into a static variable made an invocable method which will set the values during before insert/update without the requirement of newListAfterFlow. This works a little better, and seems to provide a better experience for the admin.

@invocableMethod(
  category='Trigger Action Flow'
  label='Set Values Before Insert/Update'
  description='Sets the value in the newList before insert or update'
)
public static void setNewFieldValueBeforeContext(
  List<SetNewFieldValueBeforeContextRequest> request
) {
  if (request.isEmpty() || request.size() > 1) {
    throw new TriggerActionFlowException(INVALID_REQUEST);
  }
  TriggerActionFlow.setEditableFields(
    TriggerActionFlow.newList[request[0].newListIndex],
    request[0].currentRecord
  );
}

3. Keep a static variable for oldList to reduce deserialization overhead
   This helps to improve the performance of the normal getOldRecord and will allow us to access the old values one at a time. I changed the method to only require an Id to further reduce deserialization overhead.

public static List<SObject> oldList;

public void beforeUpdate(List<SObject> newList, List<SObject> oldList) {
  TriggerActionFlow.oldList = oldList;
  if (!thisFlowIsBypassed()) {
    List<sObject> recordsNotYetProcessed = new List<sObject>();
    for (sObject record : newList) {
      if (
        TriggerBase.idToNumberOfTimesSeenBeforeUpdate.get(record.id) == 1 ||
        (allowRecursion == true)
      ) {
        recordsNotYetProcessed.add(record);
      }
    }
    Flow.Interview myFlow = Flow.Interview.createInterview(
      flowName,
      getFlowInput(recordsNotYetProcessed, oldList)
    );
    myFlow.start();
  }
}

@invocableMethod(
  category='Trigger Action Flow'
  label='Get Old Record'
  description='Returns the old version of a record from oldList'
)
public static List<sObject> getOldRecord(List<Id> request) {
  return new List<sObject>{
    new Map<Id, SObject>(TriggerActionFlow.oldList).get(request[0])
  };
}

With a combination, we are able to get the time down to about 2.5-3.0 seconds.

Findings

• Moving the oldList to a static variable had some pretty serious performance gains when compared to deserializing the record.
• It appears that invoking apex within a flow loop has a tax of about 1.5 seconds per 200 records.
• Serializing the full records from newList or oldList has a heavy tax as well.
• Whenever possible, we should check the current record's field state before getting the old record to check if the field has changed.
• We still need to determine if "Set Values Before Update" instead of adding to newListAfterFlow is more effective.
• Flow's decision and assignment steps seem to take much longer than one would expect. Each one of these steps can add 100s of ms to total execution time when executed in a loop over 200 records.