[camerax] Shorten interval for releasing weak references to Dart-wrapped native objects #6493
Conversation
There was a problem hiding this comment.
Is there a reason we can't set to 1000 all the time? The fact that we are switching seems to imply some sort of trade off to me (otherwise we could just stay at 1000 right?) but it isn't clear to me what that trade off is.
Is it just the potential of performance impact? But we are fine with that potential performance impact when we need the shorter interval because image streaming demands it?
| // collected weak references to native Android objects that it manages in order to | ||
| // account for the increased memory usage that comes from analyzing images with an | ||
| // ImageAnalysis.Analyzer. | ||
| instanceManager.setClearFinalizedWeakReferencesInterval(1000); |
There was a problem hiding this comment.
Can this also get a descriptively named constant
Yes, you hit the nail on the head--we just aren't sure that it wouldn't impact performance, but we know that to avoid a crash, we have to lower it for image streaming. We could lower the default below 3000, but it doesn't seem necessary for any other camera use cases so far. |
camsim99
added
the
autosubmit
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
…art-wrapped native objects (flutter/packages#6493)
flutter/packages@2c15d86...e98839a 2024-04-10 ekuleshov@gmail.com [tool] Check for `test` and `flutter_test` in non-dev dependencies (flutter/packages#6472) 2024-04-10 43054281+camsim99@users.noreply.github.com [camerax] Shorten interval for releasing weak references to Dart-wrapped native objects (flutter/packages#6493) 2024-04-10 engine-flutter-autoroll@skia.org Roll Flutter from 4967a94 to 97cd47a (11 revisions) (flutter/packages#6497) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@2c15d86...e98839a 2024-04-10 ekuleshov@gmail.com [tool] Check for `test` and `flutter_test` in non-dev dependencies (flutter/packages#6472) 2024-04-10 43054281+camsim99@users.noreply.github.com [camerax] Shorten interval for releasing weak references to Dart-wrapped native objects (flutter/packages#6493) 2024-04-10 engine-flutter-autoroll@skia.org Roll Flutter from 4967a94 to 97cd47a (11 revisions) (flutter/packages#6497) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…ped native objects (flutter#6493) This PR: 1. Shortens the `InstanceManager`'s default time interval that it waits to remove references to Dart-wrapped Android native objects. The shortened interval matches `webview_flutter_android`s `InstanceManager`; because it has been tested by that plugin, we expect it not to impact performance and this could help reduce overall memory usage by the plugin. 2. Dynamically shortens that same time interval whenever image streaming is started/stopped to account for the increased memory usage that this camera use case requires. Fixes flutter/flutter#145893.
This PR:
InstanceManager's default time interval that it waits to remove references to Dart-wrapped Android native objects. The shortened interval matcheswebview_flutter_androidsInstanceManager; because it has been tested by that plugin, we expect it not to impact performance and this could help reduce overall memory usage by the plugin.Fixes flutter/flutter#145893.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).