Isolate the GraphClient in another thread

[Sewdn]

Is your feature request related to a problem? Please describe.
The Main thread of a flutter app, should be reserved for UI, to keep the rendering and interactions as smooth as possible. Currently all computation by the GraphQL client is run in the same thread, competing with the UI to get CPU time for fetching and parsing data, normalizing it, encoding it to json and storing it to cache. This results in jittered UI when lots is taking place

Describe the solution you'd like
Running the entire GraphlClient and Cache (and persistence backend) in a separate Isolate (or multiple Isolates), would result in more performant flutter apps, keeping more CPU time available for the main thread (UI). While still offering the same api for querying and mutating data, it would abstract away the complexity of sending and receiving messages from other thread.

Describe alternatives you've considered
When trying to persist cache more often then only during app shutdowns, we stumbled on rendering issues, because encoding the entire cache to json before persisting it, can be very CPU consuming. We tried to offload only cache persistence to a seperate Isolate, but since no memory is shared between 2 threads, and only primitives are allowed as arguments when sending messages to another thread, we didn't end up with a good solution. It makes more sense to shift the entire apollo client to another thread, because the graphql client's api is suited for sending these requests to another thread (sending operations and variables).

Additional context
#423

[mainawycliffe]

Yeah, this looks like a good idea. If we go down this path, we could make it configurable. I am just wondering if flutter for web supports isolates.

[Sewdn]

I can imagine Isolates could work for web, making use of webworkers. Don't know if Isolates are already been adapted to making use of them for web... I'll look into it.

[mainawycliffe]

That would be the ideal way, for now, it seems, they have not been adapted to be used by flutter for web and would require a platform check before using them to make sure we support flutter for web.

[msal4]

I'm interested in working on this one, any tips on where I should start?
@mainawycliffe @micimize

[micimize]

@msal4 if you aren't already have familiar with Isolates, take a look at Isolate 2-Way Communication and other articles. In terms of implementation guidance (from someone who knows very little about isolates), my first recommendation would be to not put the whole client in an isolate, and first create a link isolate.

Creating a link isolate:
Because gql_links are stream-based already, you'd just have to wrap a given the given link in an isolate, serializing requests into the isolates and parsing responses. You'll also need to provide request parsing and response serialization on the other side of the isolate.

Later if this is a successful approach you could contribute some optimizations to other links that avoid the need for the doubling of serialization/parsing.

You can't really create a cache isolate because the cache has a synchronous API, and this would probably not be worth all the heavy message passing anyhow.

Creating a client isolate:
The client itself has a lot of hard-to-serialize api surface area. I'd recommend ignoring watchQuery / watchMutation initially, and focusing on query, mutate, and subscribe. You'd need:

• serialization for options and results
• to create an "isolate wrapper" that:
  • reads from the isolate stream and calls the appropriate method+options combo
  • send the results back (you might need message ids or something for making sure the results get to the right place)
• to create an "isolate client" that proxies calls to the provided isolate
• for subscribe, you'll need to parse a message for the id, and then filter on the isolate stream for messages with that id, then return that filtered stream.

For watchQuery / watchMutation, you need to deal with the fact that they return an entire ObservableQuery, for which part of the API is setting functional callbacks, and which is fairly coupled to the QueryManager. You could have a proxy-ing version of ObservableQuery, but this part will be very tough, because for instance, Mutation's update takes in a cache proxy, so then you have to proxy through that to the isolated proxy. hive probably still can't be used in multiple isolates, so you probably can't have some trick where we have a partial client in the main thread.

And the last thing I'll say is that I'm actually fairly skeptical of this yielding performance gains. We have a much better caching solution than when this was initially opened, and isolating the client adds a fair amount of indirection for what is essentially a high-throughput, highly-coupled slice of functionality. I don't know much about dart isolates though, so I'd do some research into exactly how this will change the profiling of an app before embarking. This is what makes starting with a simple link such a good idea – minimum scenario for testing whether this is worth doing and what performance gains can be achieved.

[daksh-gargas]

It's so weird how the package still is performing all the tasks on the Main thread. I mean that's just basic practice to not do such computation-intensive tasks on the main thread.

[budde377]

It's so weird how the package still is performing all the tasks on the Main thread. I mean that's just basic practice to not do such computation-intensive tasks on the main thread.

@daksh-gargas no one is stopping you from submitting a pr

[daksh-gargas]

Working on it rn... will probably add one soon @budde377 :)

[vincenzopalazzo]

Thanks to resurrect this feature :)

[insidewhy]

We're working on a product where even a few requests causes the whole app to lag since the CPU of the device isn't very powerful.

[daksh-gargas]

@insidewhy I am almost done with a potential fix to this problem. Just testing it out. Will update soon!

[insidewhy]

@daksh-gargas Okay let me know if you need some help testing, I can try your branch against our product when it's ready if you want a second pair of eyes.

[aquadesk]

I am looking forward to testing this fix.
I think this could fix my issue here #1187

[daksh-gargas]

@aquadesk Wrap your whole GraphQL service inside IsolatedBloc or Isolates

I'll post the full working solution and reasoning for this approach real soon, but for the time being, please use that to unblock yourself! :)

[knaeckeKami]

@vincenzopalazzo If you're interested, I recently implemented support for running the client in an Isolate here:

https://github.com/gql-dart/ferry/blob/master/packages/ferry/lib/src/isolate/
https://github.com/gql-dart/ferry/blob/master/packages/ferry/lib/ferry_isolate.dart
Documentation:
https://ferrygraphql.com/docs/isolates

I'm not 100% satisfied with the solution yet, though, as custom communication between the isolates (for example when deleting the auth token on logout) still requires users to understand Isolates and SendPorts to some degree and I would like to improve that in future versions.

[insidewhy]

@daksh-gargas I don't suppose you had any success?

[orestesgaolin]

Alternatively, instead of keeping the whole client in a separate isolate, you can just delegate the json parsing via compute method. I've posted a sample implementation here and it's been working fine for the last month on production with several hundred users. One caveat is that if you have few queries being processed/deserialized and you do the hot restart in VS Code, it may lead to unhandled exceptions related to isolates, but it's not happening in release builds.

[daksh-gargas]

Seems like you didn't account for processing/parsing of objects inside GQL Cache
That also happens on the main thread unless moved to an isolate :)

Alternatively, instead of keeping the whole client in a separate isolate, you can just delegate the json parsing via compute method. I've posted a sample implementation here and it's been working fine for the last month on production with several hundred users. One caveat is that if you have few queries being processed/deserialized and you do the hot restart in VS Code, it may lead to unhandled exceptions related to isolates, but it's not happening in release builds.

[biklas7]

Was there any development on this topic? I'm also developing an app that relies heavily on GQL Cache and I still feel the performance hit, even though the app isn't that big. Should I look into other alternatives myself for now?

[insidewhy]

Was there any development on this topic? I'm also developing an app that relies heavily on GQL Cache and I still feel the performance hit, even though the app isn't that big. Should I look into other alternatives myself for now?

I think you'll need to write your own PR to fix this or find an alternative, with our simple app the performance is far too bad on anything but high-end phones. We don't consider this library usable in its current state.

[knaeckeKami]

FWIW a big performance boost can be achieved by replacing calls to DeepCollectionEquality.equals() with a custom json equals method like this:

/// compare two json-like objects for equality
/// parameters must be either null, num, bool, String or a List or Map of these types
/// this is an alternative too DeepCollectionEquality().equals, which is very slow and has
/// O(n^2) complexity:
bool jsonMapEquals(Object? a, Object? b) {
  if (identical(a, b)) {
    return true;
  }
  if (a == b) {
    return true;
  }
  if (a is Map) {
    if (b is! Map) {
      return false;
    }
    if (a.length != b.length) return false;
    for (var key in a.keys) {
      if (!b.containsKey(key)) return false;
      if (!jsonMapEquals(a[key], b[key])) return false;
    }
    return true;
  }
  if (a is List) {
    if (b is! List) {
      return false;
    }
    final length = a.length;
    if (length != b.length) return false;
    for (var i = 0; i < length; i++) {
      if (!jsonMapEquals(a[i], b[i])) return false;
    }
    return true;
  }
  return false;
}

Doing so would likely make this fast enough to this is not an issue any more for many apps.
And it should be much much easier do just switch the equals() implementation than to migrate to isolates.

See also:
#1196 (comment)
dart-lang/core#645

[vincenzopalazzo]

Yeah, this is the solution @knaeckeKami moving this in a new isolate will hide just the problem.

Like if my machine is too slow, I buy a bigger one instead to optimize the software that I use!

I will assing this to my self, and see we can include this inside the 5.2

[insidewhy]

moving this in a new isolate will hide just the problem.

All significant processing should be moved into an isolate though, other processing that graphql-flutter is doing is also fairly intensive and has the chance to lag the UI. So it's not only hiding the problem but also a part of best practice, ideally both of these solutions should be applied.

[vincenzopalazzo]

we start with the first one and then we see if the isolate is necessary! if you want make this test, feel free to open a PR

[ayyoubelamrani4]

we start with the first one and then we see if the isolate is necessary! if you want make this test, feel free to open a PR

There are options to avoid the first one, but even with that, having the main thread taking care of requests and communicating with the cache etc ... is very heavy and might create lots of jitters depending on the app.

Is anyone working on having isolates for this lib?

[mattsrobot]

Unfortunately my app is being impacted by this issue also.

[RobertBrunhage]

Just for transparency, I implemented the approach mentioned by @knaeckeKami, and here is the result #1315 (comment)

In short, I feel like the equality checks should be improved as a high priority