Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to using Android Background Platform Channels #719

Closed
gaaclarke opened this issue Oct 28, 2021 · 4 comments · Fixed by #830
Closed

Migrate to using Android Background Platform Channels #719

gaaclarke opened this issue Oct 28, 2021 · 4 comments · Fixed by #830

Comments

@gaaclarke
Copy link
Contributor

As of flutter/engine#29346 platform channels support the ability to execute on background threads instead of the platform thread. This means all the work in sqflite managing its own thread can be removed once that hits stable.

In the meantime I've migrated a few plugins and used reflection to take advantage of the new API if it is available.

Here is an example where I migrated path_provider which also was managing its own thread: flutter/plugins#4443

This should make sqflite perform much better on Android by removing the time on the platform thread, which is often contested, and by making each call 1 thread hop instead of 2.

@alextekartik
Copy link
Contributor

Thanks for the heads up. Interesting indeed. Do you know at which priority is this thread running? (Current sqlite thread uses a low background priority THREAD_PRIORITY_BACKGROUND)? That will likely simplify the implementation on this. The example on path_provider will help. I will likely wait for it hitting beta before doing the migration (maintaining both solution using reflection will be a pain I think, I'd rather rely on a given flutter version and move to it once available).

Also what about other non-android platforms?

@gaaclarke
Copy link
Contributor Author

hey @alextekartik

Do you know at which priority is this thread running?

I didn't specify it so whatever the default is. The API is documented and setup to not guarantee that you actually get your own thread. I plan on moving the implementation to a thread pool so if you have 20 different plugins you won't end up with 20 different threads. Right now there is only one method for creating TaskQueues, I was thinking that we can overload it to take an options object that could specify things like wether you use the thread pool and what priority the thread is. We were being very cautious about what power we released to plugin developers and I was waiting to get feedback from plugin developers about what more we have to open up.

I will likely wait for it hitting beta before doing the migration (maintaining both solution using reflection will be a pain I think

Yea, I figured. I'm evaluating which plugins are going to have the biggest impact for a google customer and which ones are the easiest to migrate and this one came up. Depending on how that work goes maybe I'll start a PR for sqflite and we can evaluate what we'd feel comfortable doing for now.

Also what about other non-android platforms?

I'd like to implement it for other platforms but it isn't as high priority. This change was informed by some traces from a google customer and they saw the biggest problem with android and the platform thread there.

@alextekartik
Copy link
Contributor

The API is documented and setup to not guarantee that you actually get your own thread. I plan on moving the implementation to a thread pool

One think I think about is that SQLite locking mechanism is thread based. For example all statements in a transaction must be executed in the same thread. Since this could be done in multiple calls there need to be a garantee that the same thread is used in this case so I'm not sure that having a pull of thread (or simply having different threads between 2 calls) will work here.

@gaaclarke
Copy link
Contributor Author

It will work as long as the calls are serialized. Sqlite only has a problem if you have concurrent calls into it... well actually it depends on how you compile sqlite but if the calls are serialized it doesn't matter. The only thing that can get tripped up is thread_local storage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants