`FutureOr<T>` are unnecessarily always awaited

[ueman]

trafficstars

The code from https://github.com/getsentry/sentry-dart/blob/954825b84f7b84f383bea614ba303b868595badb/dart/lib/src/sentry_client.dart#L290-L314 can be rewritten as follows

Future<SentryEvent?> _processEvent(
  SentryEvent event, {
  dynamic hint,
  required List<EventProcessor> eventProcessors,
}) async {
  SentryEvent? processedEvent = event;
  for (final processor in eventProcessors) {
    try {
      // the 6 following lines are the important bit
      final e = processor.apply(processedEvent!, hint: hint);
      if(e is Future) {
        processedEvent = await e;
      } else {
        processedEvent = e;
      }
    } catch (exception, stackTrace) {
      _options.logger(
        SentryLevel.error,
        'An exception occurred while processing event by a processor',
        exception: exception,
        stackTrace: stackTrace,
      );
    }
    if (processedEvent == null) {
      _recordLostEvent(event, DiscardReason.eventProcessor);
      _options.logger(SentryLevel.debug, 'Event was dropped by a processor');
      break;
    }
  }
  return processedEvent;
}

This should improve performance a little. There are a couple more places where the code can be improved similarly.

This comes out of a discussion at https://github.com/getsentry/sentry-dart/pull/858#discussion_r874812361

[marandaneto]

@ueman Does it really improve something? I guess it just makes the code a little bit more complicated, I believe either the compiler or the runtime is clever enough to understand that if it's FutureOr being a Future or not, the await is optimized to do waste resources if not necessary, this is just a hunch tho.

[ueman]

Yes, this actually does improve performance by not introducing an asynchronous gap.

For consistency and smaller code, an await e where e evaluated to a non-future would wrap that value in a future and await that. It means that there is only one quick and reusable check on a value (is it a future, if not wrap it), and then the remaining code is the same.

From an answer from one of the Dart language designer here https://stackoverflow.com/questions/59213196/what-is-the-purpose-of-futureor

So it's kinda the opposite of what you're saying, awaiting a non-future is degrading performance by introducing unnecessary asynchronous gaps.

[marandaneto]

We can likely wrap all the calls to FutureOr with an extension or something in order to be a good citizen in considering performance improvements in the SDK.