sentry-dart
sentry-dart copied to clipboard
Improve `FileSystemTransport` by using `compute()` or background isolates
We can try to use compute() here
https://github.com/getsentry/sentry-dart/blob/bca59abc7f5f257b4fcbb973090d9df719509ebc/flutter/lib/src/file_system_transport.dart#L14-L17
in order to offload the conversion to another isolate. This could improve the performance because we're not blocking the UI Isolate, especially if there are any attachments or big events.
There are some things which need to be considered.
class NewFileSystemTransport implements Transport {
NewFileSystemTransport(this._channel, this._options);
final MethodChannel _channel;
final SentryOptions _options;
@override
Future<SentryId?> send(SentryEnvelope envelope) async {
final args = await compute(_convert, envelope);
try {
await _channel.invokeMethod<void>('captureEnvelope', [args]);
} catch (exception, stackTrace) {
_options.logger(
SentryLevel.error,
'Failed to save envelope',
exception: exception,
stackTrace: stackTrace,
);
return SentryId.empty();
}
return envelope.header.eventId;
}
static Future<Uint8List> _convert(SentryEnvelope envelope) async {
final envelopeData = <int>[];
await envelope.envelopeStream().forEach(envelopeData.addAll);
// https://flutter.dev/docs/development/platform-integration/platform-channels#codec
return Uint8List.fromList(envelopeData);
}
}
makes sense, since Flutter Apps don't crash if not on the Native layer, we could offload the heavy work on a different Isolate using compute, worth saying that every capture method returns a Future, so they can always choose to not await the method anyway.
Even if it's not awaited it's still running on the UI isolate.
if we're not awaiting, we need another back pressure mechanism. Otherwise we might degrade performance if the app is producing more events than the SDK can flush out (to disk or network)
Background isolates might help solving this issue, if we are able to offload to a background Isolate and this Isolate is able to call method channels, only possible on Flutter 3.7 still.
Edit: 3.7 is GA See https://medium.com/flutter/whats-new-in-flutter-3-7-38cbea71133c
Checked the documentation for background channel usage, and we have the same Issue that we are facing with #1801, as we need to access ServicesBinding.rootIsolateToken, which is static, so we can't use the approach where we cast to dynamic and call a method that is unknown in older Flutter versions which we need to support. Is there approach or a way around this limitation?
Short summary from the #1811 findings:
- We can't use compute to convert the envelope, as we capture objects that cannot be shared between isolates. Same issues as in #707
- We'd need to bump flutter to 3.7 in order to use method channels in a background isolate. Is passing the serialised structures to the Cocoa/Android SDKs on the main thread a bottleneck that is causing us issues?
Is passing the serialised structures to the Cocoa/Android SDKs on the main thread a bottleneck that is causing us issues?
Have you checked that?
Not sure if passing serialized data will cause bottleneck issues so I'd think this is just a nice to have in terms of improvement but alone probably doesn't warrant a min version bump to 3.7
Blocked by min version Flutter 3.7