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

[Feature] Optimization required: It is taking to long for the grid to show #116

Closed
yanivshaked opened this issue May 2, 2021 · 12 comments
Closed
Labels
b: flutter This is a flutter issue. e: goal This issue is a goal for development. e: performance This issue is related to performance. e: PR welcomed Can be improved by PRs. i: lack of information Further information is requested. r: photo_manager Related to the photo_manager package.

Comments

@yanivshaked
Copy link
Contributor

Version information

  • Device: Android OnePlus 8T
  • OS: Android 10
  • Package Version: 5.2.1
  • Flutter Version: 2.0.5

Is your feature request related to a problem?
Opening the picker, it is taking too long until the grid appears with the images.
If you compare the performance to other pickers in the market, this picker takes a long time to present the grid.

Describe the solution you'd like
I have tried to run some tests to focus on the performance bottle neck. I am not sure, but it appears that the generation of the thumbnail per image is taking quite some time:

      final List<int> _thumbSize = thumbSize!;
      final Stopwatch sw = Stopwatch()..start();
      data = await key.entity.thumbDataWithSize(_thumbSize[0], _thumbSize[1]);
      print('thumbDataWithSize ${sw.elapsedMilliseconds}');

output:

I/flutter (23576): thumbDataWithSize 250
I/flutter (23576): thumbDataWithSize 239
I/flutter (23576): thumbDataWithSize 236
I/flutter (23576): thumbDataWithSize 233
I/flutter (23576): thumbDataWithSize 230
I/flutter (23576): thumbDataWithSize 227
I/flutter (23576): thumbDataWithSize 224
I/flutter (23576): thumbDataWithSize 220
I/flutter (23576): thumbDataWithSize 217
I/flutter (23576): thumbDataWithSize 213
I/flutter (23576): thumbDataWithSize 210
I/flutter (23576): thumbDataWithSize 206
I/flutter (23576): thumbDataWithSize 193
I/flutter (23576): thumbDataWithSize 190
I/flutter (23576): thumbDataWithSize 200

Note:
There is a bug in the "Custom Image Preview thumb size". It is not actually working - the thumb size is lost in the way. I will submit a PR to fix it (the bug is NOT in the example code but in the plugin itself).

Describe alternatives you've considered

  • Tried reducing the image preview thumb size (after locally fixing the bug) - it helps, but not too much
  • Tried reducing the routeDuration and other durations in code - didn't help
  • Tried replacing default SortPathDelegate.common with SortPathDelegate.empty.empty (which does nothing), almost no change

Several solutions which should be inspected

  • Check for PhotoManager preload thumb feature: PhotoManager PreloadThumb
  • Don't wait for all images thumb to be ready, display each when ready (does it work like that now?)
  • Using the image itself instead of generating a thumbnail per image (probably not a good idea, but should be tested)
@yanivshaked yanivshaked added the await investigate The issue is waiting for further investigation. label May 2, 2021
@github-actions github-actions bot added the await triage The issue is waiting for triage. label May 2, 2021
@AlexV525
Copy link
Member

AlexV525 commented May 2, 2021

Seems no improvement for me. The method requires you to throw assets into it, then you'll back to the very start of the issue which is the duration when getting assets list.

  • Don't wait for all images thumb to be ready, display each when ready (does it work like that now?)

Yes this is how it works currently. Load assets list, layout the grid, and items take care of the thumbnail themselves.

  • Using the image itself instead of generating a thumbnail per image (probably not a good idea, but should be tested)

IMO there's no need for testing. You can directly see their difference between the grid and the preview.

@yanivshaked
Copy link
Contributor Author

Do you have any other suggestion on how to optimize the grid loading?

@AlexV525 AlexV525 added e: performance This issue is related to performance. r: photo_manager Related to the photo_manager package. i: postpone The issue cannot be solved right away. and removed await triage The issue is waiting for triage. labels May 3, 2021
@AlexV525
Copy link
Member

AlexV525 commented May 3, 2021

Not really. This might requires more enhancements from the photo_manager plugin. I'll discuss with @CaiJingLong offline later.

@yanivshaked
Copy link
Contributor Author

Thanks!

@AlexV525 AlexV525 added this to To do in Performance optimize May 6, 2021
@Letalus
Copy link
Contributor

Letalus commented Jul 2, 2021

i just wanted to also state, that some of my alpha tester, who have android devices, also complaint, that the loading time for the assets within the grid, is really slow. Apparently this problem is not really a big problem for iOS but it is was Android. If I can help somehow, please let me know and I can review the necessary android code to adjust it. Maybe one possibility would be to give the grid a stream of thumbnails instead of all images at once, to reduce the loading time.

Kind regards,
Chris

@AlexV525
Copy link
Member

AlexV525 commented Jul 2, 2021

Apparently this problem is not really a big problem for iOS but it is was Android.

Actually we heard more complaint about this issue on iOS. But no matter which platforms are suffering this problem, we should definitely discuss this in the photo_manager repo.

I'll give some detail about the laggy part which the picker mainly suffered.

Grid is always spinning

await getAssetPathList();
await getAssetList();

The reason why the grid spins too long is caused by these methods, and they must be called in proper sequence. Anyone who wants to test how long these methods been taken, can place a Stopwatch here. AFAIK, low performance devices, devices that have more than thousands of entities, could both suffered from this.

Thumbnails/Original files are slow with rendering

Thumbnails are provided by some native dependencies, e.g. Glide on Android, which requires a generate process when reaching them. Then they'll be copied in the Flutter's channel pipeline. This is how copies were transferred under the Android platform:

MediaStore -> Uri -> Bitmap -> ByteArray => Platform Channel => Uint8List -> resolve by ImageProvider.

Too many copies required to generate during this process, which means some laggy were produced by the architecture of channels.

However, it can be some solutions to particialy solve this issue, but more specific solutions are still under investigation by Flutter team. See https://discord.com/channels/608014603317936148/846507907876257822/850072476402450433 as we've discussed this few weeks ago.

@AlexV525 AlexV525 added b: flutter This is a flutter issue. e: goal This issue is a goal for development. labels Jul 2, 2021
@AlexV525 AlexV525 added the e: PR welcomed Can be improved by PRs. label Sep 16, 2021
@chriscdn
Copy link

chriscdn commented Nov 6, 2021

Has anyone found a solution? The asset picker is functionally great, but it takes almost 10s on my iPhone SE (in release mode) for photos to start showing.

@matthiasn
Copy link

Same here, like 10 seconds on an iPhone 12 Pro.

@AlexV525
Copy link
Member

AlexV525 commented Feb 23, 2022

If anyone still facing the same issue like this, or fluttercandies/flutter_photo_manager#711, please help us by running the latest example of photo_manager, then filtering log as fluttercandies/flutter_photo_manager#711 (comment) suggested. Since we can't reproduce this even on devices that contains thousands of assets, the root cause is invisible from our (maintainers) sight.

@AlexV525
Copy link
Member

AlexV525 commented Mar 7, 2022

We've shipped the 7.0.0 version which brought a bunch of improvements from the photo_manager 2.0.0 version.
If anyone is still facing the obvious performance/duration issues, contact me by submitting new issues in the photo_manager's repo.

Closing since we have few samples to solid reproduce this issue.

@AlexV525 AlexV525 closed this as completed Mar 7, 2022
Performance optimize automation moved this from To do to Done Mar 7, 2022
@AlexV525 AlexV525 added i: lack of information Further information is requested. and removed await investigate The issue is waiting for further investigation. i: postpone The issue cannot be solved right away. labels Mar 7, 2022
@kartik1225
Copy link

I reproduce this issue in google pixel 4a android 12.
When I filter only videos that time grid is taking too long (7s - 10s) to load initially.
Once everything is cached it works as intended.

So in summary if we clear the data and filter only videos that time I am able to reproduce this bug.

@AlexV525
Copy link
Member

So in summary if we clear the data and filter only videos that time I am able to reproduce this bug.

@kartik1225 If it's able to reproduce, please file issues against photo_manager and we'll take further investigations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b: flutter This is a flutter issue. e: goal This issue is a goal for development. e: performance This issue is related to performance. e: PR welcomed Can be improved by PRs. i: lack of information Further information is requested. r: photo_manager Related to the photo_manager package.
Development

No branches or pull requests

6 participants