-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(messaging): use FlutterEngineGroup to improve performance of background handlers #9867
Conversation
} | ||
Log.i(TAG, "Creating background FlutterEngine instance."); | ||
backgroundFlutterEngine = | ||
new FlutterEngineGroup(ContextHolder.getApplicationContext()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is going to give us what we want. In order for you to avoid creating another isolate group and get the benefit of faster starting lower memory engines you'll have to share the FlutterEngineGroup between the plugin and the host app. I don't think there is a way for you to do that today since there is no guarantee the host app is using the FlutterEngineGroup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the flip side, isolate platform channels is about to land on stable. This allows sending platform channel messages from background isolates. I believe that will remove the need this plugin has for spawning an engine in the first place. It's probably better we just lean into that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the recommendation, I'll check that once it has land in stable :)
Description
Moving to FlutterEngineGroup is more performant and allows for a shared heap
Related Issues
#9816
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).This will ensure a smooth and quick review process. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?