Parsing large graphql responses and normalizing them is a potentially expensive operation that can lead to skipped frames.
Users complained about this in issues like this: gql-dart/ferry#394 (comment)
Make it possible for users to run their graphql client in a separate isolate, considering these goals:
-
The original API should stay available for users who do not want to migrate and for users on the Web.
-
Staying as close to the original API as possible. In particular, the TypedLink interface should be preserved so that the isolate client is mostly a drop-in replacement for the majority of users. Since Ferry was already Stream based and followed a
Request
->Stream
of responses structure, I believed this should be easily achievable over Isolates. -
For use cases that are not possible to implement using the existing API (e.g. synchronous access to the cache), offer asynchronous alternatives
-
Initializing the client should be as easy as possible for users, even if they are not familiar with the Isolate API.
-
It must be possible for users to add custom communication between the ferry isolate and the main isolate, especially e.g. passing and storing authentication tokens.
This is how the original PR looked like: gql-dart/ferry#405
I believe the first three goals were achieved. I received feedback from users that it works for them, and that jank got noticeably better.
A few users struggled with the last two goals though.
Especially enabling custom communication between the isolates by doing the
"create a new ReceivePort
, send the SendPort
of the ReceivePort
over the other SendPort
"-handshake is hard to understand
and probably should be abstracted away in a future version of ferry.
The first challenge was to decide on a general Architecture. There are only a few resources on best practices on how to do non-trivial communication over Isolates.
Decisions I struggled with:
Should all communication be done over one ReceivePort/SendPort pair? Or should there be one "main" pair but every method call creates a new private ReceivePort for communication for this specific method call?
I did not know the performance impact on creating many ReceivePorts. But it seemed cheap enough that I went for the latter approach.
I went for the following architecture:
-
On the main isolate, an instance of
IsolateClient
holds theSendPort
to communicate with the ferry Isolate. -
The
IsolateClient
sends instances ofIsolateCommand
over theSendPort
. Each IsolateCommand corresponds to a method call of the original API. The IsolateClient listens to responses from the other isolate and maps them to a Future or Stream.Simple example: ReadQueryCommand reads the result of a Query from the cache.
-
The
IsolateClient
is created by calling the IsolateClient.create method. Similarly to the compute() function, the user will pass a top-level function and the parameters. This function will be called on the new isolate to create the "real" client that executes requests (using the existing API of ferry). It's not called directly though, the actual entry point is here, it performs a [SendPort] handshake before initializing the client.
-
I created an ad-hoc protocol for wiring Streams over SendPorts while preserving cancel/done events. (When the user stops listing to a stream of a request, the client on the other side of the isolate should stop watching the cache for updates). Not convinced that this implementation is correct in all corner cases.
In retrospect, this should probably have been extracted to be used and tested independently.
The main isolate portion is here: https://github.com/gql-dart/ferry/blob/master/packages/ferry/lib/ferry_isolate.dart#L111 The ferry isolate part here: https://github.com/gql-dart/ferry/blob/master/packages/ferry/lib/src/isolate/isolate_commands.dart#L38
-
I initially tried to implement handing IsolateCommands via an old-fashioned switch-case statement (https://github.com/gql-dart/ferry/blob/d5363b9681fd3a514e5422469bce1bfc3c6c9680/packages/ferry/lib/src/isolate/handle_command.dart#L6 ) instead of
handle()
method to perform the actual logic inside to command itself. Even though the switch case approach is not idiomatic OOP, it somehow felt right to me for this use case, since the sending objects with real logic in methods attached to them felt wrong (even though I knew this was supported).However, since some of the sent objects have fields with generic type parameters in a contravariant position, it is impossible to access them without knowing the right type (see this discussion on discord: https://discord.com/channels/608014603317936148/1022277801988063232/1022277801988063232 )
-
Ferry can use hive to persist the cache for offline usage. However, to initialize hive, a path is needed. For Flutter apps, people likely want to use
getApplicationDocumentsDirectory()
from path_provider. However, since this fuction calls Method channels, it's only available on the main isolate. So users have to call it in the main isolate and then pass it as a parameter to the ferry isolate. Example code: https://github.com/gql-dart/ferry/blob/439edc72eea6f5fc1b59a783759d285f83df6acf/examples/pokemon_explorer/lib/src/client_isolate.dart#L11 I frequently receive messages from users who don't understand why this is happening. -
Handling authentication is hard to understand for users since they usually need to manage the token on the main isolate but also need to include them on requests in the ferry isolate. I added an example project where the main isolate owns the token and the ferry isolate requests it for every request: https://github.com/gql-dart/ferry/blob/master/examples/auth_token_with_isolate/client/lib/src/client/client.dart#L35 .
But this is definitely a major pain point since you have to understand [SendPort] and |ReceivePort] and how to do the handshake by sending SendPorts to make this work.
This probably can be improved by ferry, to make it easier for users, but I'm not sure how at the moment.
- sending Network errors from the ferry isolate to the main isolate could fail because the thrown Exception would contain a reference the network request, which would have a reference to a MultiPartFile which is not serializable because it contains a NativeWrapper.
It was hard to pinpoint this issue because the exception thrown when trying to send the object is just
Illegal argument in isolate message: (object extends NativeWrapper - Library:'dart:io' Class: _RandomAccessFileOpsImpl@14069316)
If it would show the "path" in the object graph to the illegal field it would have made my life much easier!
Example error message that I wish would be thrown:
Illegal argument in isolate message: DioError -> request -> data -> (object extends NativeWrapper - Library:'dart:io' Class: _RandomAccessFileOpsImpl@14069316)
Thanks so much for your thoughtful response!
Thank you. I will consider switching to another implementation - We need to compare standard JSON-like HashMaps, so DeepCollectionEquality seems indeed like overkill. I was not aware of this considerable performance penalty for using it!
Maybe this will solve most of the user pain even without using Isolates...
Looks like it! Will definitely try this out on the master branch of flutter.
Thanks, upvoted.