Closed orible closed 2 years ago
Seems to be related to StaggeredTile.fit(1) StaggeredTile.count(1, 1) only rebuilds its cells when they come into view.
I have also noticed that using StaggeredTile.fit(1)
degrades performance by alot.
Which is problematic for my project because I have a large list of items that all have variable size and need the fit
function.
I ran into this issue as well. I'll share the "hack" that fixed it for me, though I am by no means a Flutter expert - I'm a back-end dev who has a Flutter app as a side project. My hope is that the author, letsar, will be able to use this write-up to fix this in a more "officially supported" fashion.
It's important to keep in mind that while this fixed the issue for me, I can't guarantee it will also work for anyone else. Additionally, I'm using pagination within my app, and therefore I'm typically only loading around 20-30 items max. I haven't tested this with more elements in the list than that.
Summary
The primary bottleneck as it relates to efficiency while scrolling appears to be the sliver_variable_size_box_adaptor#_createOrObtainChild
function. This function is designed to either create the child element (the items supplied to the SliverStaggeredGrid's itemBuilder
), or to pull it from the cache (if it has already been created and kept alive). The cache itself in this case is a map named _keepAliveBucket
. The creation of the child element appears to be a somewhat expensive process, so populating it from the cache is much preferred.
Compounding this inefficiency are two things:
StaggeredTile.fit
items are a variable size), each and every item from element 0 to the last element displayed on the screen has to be maintained in the list of "rendered" elements. These elements must be created to calculate their size even though not all elements are displayed on the screen. A normal Sliver has items that are all the same size, and therefore only the elements that are displayed on screen have to be rendered.The Issue
From what I can tell, looking at all of the usages of this cache (_keepAliveBucket
), it is often checked to see if it contains an item, and items are removed from it, yet items are never added to it. Therefore the cache is effectively never used, and instead ALL elements are recreated every time you scroll.
While it only appears to take anywhere from 5ms to 30ms (during the heavy stuttering) to create a child element, because they're being recreated over and over, this begins to add up and effect the UI smoothness.
The Hack
We need to ensure the cache is actually populated when creating items in order to use it as much as possible. In order to do this, we have to set the keepAlive
value to true
for the parent of these child elements:
In the widgets/sliver.dart
file, in the updateChild
function just after line 172, add the following:
if (newParentData != null) {
(newParentData as KeepAliveParentDataMixin).keepAlive = true;
}
The full updateChild
function should look like this:
@override
Element updateChild(Element child, Widget newWidget, dynamic newSlot) {
final SliverVariableSizeBoxAdaptorParentData oldParentData =
child?.renderObject?.parentData;
final Element newChild = super.updateChild(child, newWidget, newSlot);
final SliverVariableSizeBoxAdaptorParentData newParentData =
newChild?.renderObject?.parentData;
// set keepAlive to true in order to populate the cache
if (newParentData != null) {
(newParentData as KeepAliveParentDataMixin).keepAlive = true;
}
// Preserve the old layoutOffset if the renderObject was swapped out.
if (oldParentData != newParentData &&
oldParentData != null &&
newParentData != null) {
newParentData.layoutOffset = oldParentData.layoutOffset;
}
return newChild;
}
This will populate the cache, but now we need to correctly remove items from the cache and drop them accordingly when the system tells us to:
In the rendering/sliver_variable_size_box_adapter.dart
file, in the remove
function just after line 207, add the following:
if (child == null) {
RenderBox cachedChild = _keepAliveBucket[index];
if (cachedChild != null) {
dropChild(cachedChild);
_keepAliveBucket.remove(index);
}
return;
}
The full remove
function should look like this:
@override
void remove(int index) {
final RenderBox child = this[index];
// if child is null, it means this element was cached - drop the cached element
if (child == null) {
RenderBox cachedChild = _keepAliveBucket[index];
if (cachedChild != null) {
dropChild(cachedChild);
_keepAliveBucket.remove(index);
}
return;
}
final SliverVariableSizeBoxAdaptorParentData childParentData =
child.parentData;
if (!childParentData._keptAlive) {
super.remove(index);
return;
}
assert(_keepAliveBucket[childParentData.index] == child);
_keepAliveBucket.remove(childParentData.index);
dropChild(child);
}
Optional I performed one final optimization tweak to further increase efficiency, but I wouldn't necessarily recommend it. From my timing, this would likely only save 1ms or so, but I was trying to perform every quick optimization I could while changing as little code as possible. Do this at your own risk, as I'm sure these assertions exist for a valid reason:
In the rendering/sliver_variable_size_box_adapter.dart
file, in the addChild
and addAndLayoutChild
functions (originally on lines 286 and 298 respectively, but if you've made the above changes they'll be a bit further down):
Remove the assertion: assert(indexOf(child) == index));
To me, this just seemed like a needless iteration while we were already within an expensive loop. I could very easily be wrong about the importance of this assertion though, so I wouldn't necessarily recommend removing this unless the few additional ms savings seem worth it.
Conclusion
Hopefully this will help letsar track down the root of the issue - the fact that the _keepAliveBucket
is not actually being used. I have no illusions that the fix above is anything more than a hack, and it could cause other unforeseen issues within your app. Please test it out thoroughly before assuming it will not cause an issue with your app. I hope we'll see an official fix for this soon, but in the meantime, please do let me know if it worked for you - I'm interested to hear.
I made a fork with the changes @erwolff suggested
https://github.com/BertrandBev/flutter_staggered_grid_view
It can be dropped in the pubspec.yaml
file
flutter_staggered_grid_view:
git:
url: https://github.com/BertrandBev/flutter_staggered_grid_view.git
The solution of @BertrandBev is worked for me! thank you.
I had the same problem
I don't know its a permanent solution or not but after roaming on the internet for this issue indeed the solution of @BertrandBev is worked for me too! thank you.
@BertrandBev thanks for the performance fix!
One little remark about the assert
statement though:
It's not necessary to worry about the performance of statements in asserts, as the are only evaluated in debug mode and ignored in production mode.
See https://dart.dev/guides/language/language-tour#assert
My app has to display a pinterest alike infinite scroll. When binding hundreds of images, it crashes the app. The test itself is simple. Just popup hundreds of dummy images. The more you generate, the more eaiser to crash the app. My tests show when it reaches about 300, it instantly crashes the app when scrolling in iPhone 11 Pro Max.
A fix mentioned above doesn't work. So I assume it's another performance issue.
My app has to display a pinterest alike infinite scroll. When binding hundreds of images, it crashes the app. The test itself is simple. Just popup hundreds of dummy images. The more you generate, the more eaiser to crash the app. My tests show when it reaches about 300, it instantly crashes the app when scrolling in iPhone 11 Pro Max.
A fix mentioned above doesn't work. So I assume it's another performance issue.
Have you found any solution?
Hi @setya7 its it working I mean better performance with this
but im not able to add this to my pubspec.yaml
Running "flutter pub get" in appName... Because appName depends on git from unknown source "url", version solving failed. pub get failed (1; Because appNamedepends on git from unknown source "url", version solving failed.) exit code 1
I wonder if this will be merged someday.
I wonder if this will be merged someday
@ingmferrer I wonder if my free time will be valuable to you someday. I'm not paid for this work, even if you or other make profit with it, so please, stop with these kind of comments. When I see comments like "any fix?" or the one you just post, it's just make me sick, and it doesn't make me want to put efforts and free time into this. If someday if I have enough sponsoring, I will be able to put enough time to make this package and others better, but for the moment, I can't. So again, please, stop with that.
Sorry if I was rude, I've been experiencing problems with this package and this possible fix has been unmerge since May 2020. I understand you don't have time to maintain, but maybe you should ask for help in the readme.md so others know what to do/expect. In the meantime I'll try to debug other breaking error to see if I'm able to fix and commit PR.
Thank you for your apology @ingmferrer. I've merged the fix of @BertrandBev in the master branch and published a new version on pub (0.3.4). Let me know if the performances issues are fixed and if I can close this.
Just tried 0.3.4, still the same problem. I loaded some 700+ images, crash when scrolling. Maybe my example is extreme. Does this update require code change on my own app?
Hi @PhantomRay, if the app crashes with a lot of images, I would say that's it's not the same issue and I'm not sure that it's because of this package. It can be slow, but make it crashes seems strange. Have you some logs that you could share on another issue?
I'm not very happy with the fit
feature because the layout need to be recalculate every time the user scrolls, so it will never be very well performant for large lists.
What you could do, if you have control over the web services, would be to provide the ratio of the images and using StaggeredTile.count
to create your tiles instead of fit
Thanks @letsar . I did use fit
. The server does pass down height/width info. Each card showing on the grid includes image, title and desc, not just image itself. So not sure how to make use of image ratio with other dynamic content in the card.
Anyway, I simply changed from fit
to count
, issue remains. When I have more time I may dig into the library. Hope the update fixes other people's problem.
Hi @letsar and Thanks The issues is gone now for me. no frame drop while scrolling.
It seems like this fix inadvertently causes another problem when using images.
Images are unpacked and cached in the global ImageCache, which has three "tiers" of images:
The problem here is not from CachedImage or PendingImage, both of which perform properly, but from LiveImage. Since the widgets are never disposed of, the ImageProviders are never cancelled, which means that they will linger in LiveImage forever. This leads to image caches bloating up until the LMK has no more memory to free, and force-kills the app.
To test this, build a view with stateful widgets that print in dispose()
, and scroll, you'll see that it never gets disposed of while scrolling in 0.3.4.
This behavior is not expressed in 0.3.3, as items are not cached there.
A very simple fix (for now) is to use 0.3.3 if you have many pictures, try to keep your widgets as simple as possible to make them build as fast as possible. A simple, but workable fix going forward would be a switch to disable keepalives, which would trade off performance for proper disposing. Beyond that, we need to get into more complex stuff, either find a way to dispose image listeners without destroying the whole widget, and vice versa when restoring it, or find a better way to store what is needed in the keepalive.
My app has to display a pinterest alike infinite scroll. When binding hundreds of images, it crashes the app. The test itself is simple. Just popup hundreds of dummy images. The more you generate, the more eaiser to crash the app. My tests show when it reaches about 300, it instantly crashes the app when scrolling in iPhone 11 Pro Max. A fix mentioned above doesn't work. So I assume it's another performance issue.
Have you found any solution?
I'm building something very similar. Did you find a solution?
If I could find the solution, I would post it here. As other said, it doesn’t look like a trivial job.
On Fri, 5 Mar 2021 at 3:22 pm, Sofianel5 notifications@github.com wrote:
My app has to display a pinterest alike infinite scroll. When binding hundreds of images, it crashes the app. The test itself is simple. Just popup hundreds of dummy images. The more you generate, the more eaiser to crash the app. My tests show when it reaches about 300, it instantly crashes the app when scrolling in iPhone 11 Pro Max. A fix mentioned above doesn't work. So I assume it's another performance issue.
Have you found any solution?
I'm building something very similar. Did you find a solution?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/letsar/flutter_staggered_grid_view/issues/113#issuecomment-791142113, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR7QSU3KCXWE7FNXRP66NDTCBMALANCNFSM4MGSWRIQ .
-- Regards Ray Wang
It seems like this fix inadvertently causes another problem when using images.
Images are unpacked and cached in the global ImageCache, which has three "tiers" of images:
- LiveImage, which are "Actively used", which means that their ImageProvider has not been disposed of yet
- CachedImage, which are not used, but ready in memory
- PendingImage, which are being retrieved
The problem here is not from CachedImage or PendingImage, both of which perform properly, but from LiveImage. Since the widgets are never disposed of, the ImageProviders are never cancelled, which means that they will linger in LiveImage forever. This leads to image caches bloating up until the LMK has no more memory to free, and force-kills the app.
To test this, build a view with stateful widgets that print in
dispose()
, and scroll, you'll see that it never gets disposed of while scrolling in 0.3.4. This behavior is not expressed in 0.3.3, as items are not cached there.A very simple fix (for now) is to use 0.3.3 if you have many pictures, try to keep your widgets as simple as possible to make them build as fast as possible. A simple, but workable fix going forward would be a switch to disable keepalives, which would trade off performance for proper disposing. Beyond that, we need to get into more complex stuff, either find a way to dispose image listeners without destroying the whole widget, and vice versa when restoring it, or find a better way to store what is needed in the keepalive.
This does not fix the issue with images for me. I made an example app so you can reproduce the issue: https://gist.github.com/Sofianel5/3b29e15024b902f6f04ce2f84598171c
Additionally, I posted more details in my issue here: https://github.com/flutter/flutter/issues/77324
It seems like this fix inadvertently causes another problem when using images. Images are unpacked and cached in the global ImageCache, which has three "tiers" of images:
- LiveImage, which are "Actively used", which means that their ImageProvider has not been disposed of yet
- CachedImage, which are not used, but ready in memory
- PendingImage, which are being retrieved
The problem here is not from CachedImage or PendingImage, both of which perform properly, but from LiveImage. Since the widgets are never disposed of, the ImageProviders are never cancelled, which means that they will linger in LiveImage forever. This leads to image caches bloating up until the LMK has no more memory to free, and force-kills the app. To test this, build a view with stateful widgets that print in
dispose()
, and scroll, you'll see that it never gets disposed of while scrolling in 0.3.4. This behavior is not expressed in 0.3.3, as items are not cached there. A very simple fix (for now) is to use 0.3.3 if you have many pictures, try to keep your widgets as simple as possible to make them build as fast as possible. A simple, but workable fix going forward would be a switch to disable keepalives, which would trade off performance for proper disposing. Beyond that, we need to get into more complex stuff, either find a way to dispose image listeners without destroying the whole widget, and vice versa when restoring it, or find a better way to store what is needed in the keepalive.This does not fix the issue with images for me. I made an example app so you can reproduce the issue: https://gist.github.com/Sofianel5/3b29e15024b902f6f04ce2f84598171c
Additionally, I posted more details in my issue here: flutter/flutter#77324
Look guys, I've struggled a lot for this problem (memory leak) I finally found a solution, but before I tell you the solution, I want to point out that the flutter_staggered_grid_view package also causes memory swelling. Namely, I was doing my job with gridview.builder before and I had an infinite list and then I decided to add banner ads to this list. I needed the flutter_staggered_grid_view package to be able to add the banner ad and used it. After 80-100 photos were uploaded, I clicked on each photo and when I came back, I saw that the widgets were rebuilt. I come back and opss all images are reloaded. I experienced what I explained above because of the flutter_staggered_grid_view package after solving the memory leak. Now let's come to the endless list and memory leak, one of the biggest problems of flutter. The data we pull from the internet or local needs a package that will not be cached by flutter. Most packages unfortunately do cache. The first best and the solution I used is https://pub.dev/packages/extended_image using this package and just ticking cache: false. The second solution that breaks the visual dimensions I dislike is to use FadeinImage in imageCacheWidth: and imageCacheHeight: properties. I hope everyone can help.
Strangely enough, after upgrading to Flutter 2.0.6, I don't see the performance issue anymore. At least, for the same amount of images, it doesn't crash the app any more.
Well I still have this issue with Flutter 2.0.6... any temporary fix until we have solution for this?
I tested with both debug and release build.....
Yes the issue is still there. It just seems the page scrolling is a lot faster with memory issue until it crashes.
On Thu, 6 May 2021 at 10:36 am, rmControls @.***> wrote:
Well I still have this issue with Flutter 2.0.6... any temporary fix until we have solution for this?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/letsar/flutter_staggered_grid_view/issues/113#issuecomment-833141873, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR7QSWNEEOWUQ4RAVJ4AQTTMHQB7ANCNFSM4MGSWRIQ .
-- Regards Ray Wang
So I am guessing that this issue does not have any solution when it comes to images?
Hi, @clragon try using this octo_image
when I was useingcached_network_image
i faced some frame drop
The issue with images is that the current grid caches all children
and widgets like cached_network_image
will be unable to release their images,
resulting in memory overflow.
I have made a PR #188 that allows disabling the cache which seems to fix this issue.
using this option addAutomaticKeepAlives: false,
in your grid constructor
with this fork in your pubspec.yaml
:
flutter_staggered_grid_view:
git:
url: git://github.com/clragon/flutter_staggered_grid_view.git
ref: master
Can you test with the latest 0.5.0 preview? https://pub.dev/packages/flutter_staggered_grid_view/versions/0.5.0-dev.1
Same problem, but solved. Thanks @letsar !
I replaced StaggeredGridView.countBuilder with MasonryGridView.count(0.6.0).
My app has a list of over 1000 elements with a 480x480 Image.network and some text and Image.assets.
Previously, the list showed low performance while scrolling and crashed due to memory leak when scrolling continuously. However, all these problems have been solved by MasonryGridView.count.
I load a very large number of images. Before it couldn't handle a few hundred. Now it can handle thousands with no issues.
@PhantomRay could you share your code please? After I upgraded scrolling down seems to be fine but when I scroll up I get this issue https://github.com/letsar/flutter_staggered_grid_view/issues/244
@PhantomRaycould you share your code please? After I upgraded scrolling down seems to be fine but when I scroll up I get this issue
I use this code
if (_isStaggeredLayout) {
return [
MasonryGridView.builder(
gridDelegate: const SliverSimpleGridDelegateWithFixedCrossAxisCount(crossAxisCount: 2),
shrinkWrap: true,
primary: false,
padding: EdgeInsets.only(top: Style.intervalSmall),
mainAxisSpacing: Style.intervalXSmall,
crossAxisSpacing: Style.intervalXSmall,
itemBuilder: (context, index) => PostStaggeredItem(detail: state.posts.posts[index], maxWidth: _width, key: GlobalKey()),
itemCount: state.posts.posts.length,
)
];
Do you know how you would you use that with SliverMasonryGrid.count
@PhantomRay ?
Also, @mattg1995 I suggest that you use CachedNetworkImage for rendering. From your video, it appears that when you scroll up, some images appears to be reloaded.
Do you think that's what the issue is? I've tried it with a very small number and it always seems to reload with the jumping issue? @PhantomRay . It seems the variable heights is causing the issue. When I force a width/height with CachedNetworkImage
the issue appears to dissapear
Try my code above, please. Should be good.
This code? https://github.com/letsar/flutter_staggered_grid_view/issues/113#issuecomment-1127165863. I cannot use this as my code is inside a sliver context
I used to use StaggeredGridView
which had the problem. MasonryGridView
works for me.
I used the same as you before, but with Sliver (which seems to make things a bit more complicated). I think to solve my issue I need to give CachedNetworkImage the width and height of my image so it keeps the variable height, but just need to figure this out
@PhantomRay this is my code SliverMasonryGrid( delegate: SliverChildBuilderDelegate( (context, index) { return Exposure( key: Key("quanshi_list"), onExpose: () { untils.behaviorRecord( itemId: pageData[index]['product']['productNo'], bhvType: 'expose', bhvValue: '', ); }, child: GestureDetector( child: Card( elevation: 0, child: Container( height: index.isEven ? 400 : 400 * 3 / 4, child: Column( mainAxisAlignment: MainAxisAlignment.spaceBetween, crossAxisAlignment: CrossAxisAlignment.start, children: [ Expanded( child: Container( child: ClipRRect( borderRadius: BorderRadius.only( topLeft: Radius.circular(5), topRight: Radius.circular(5)), child: Image( image: CachedNetworkImageProvider(pageData[ index] ['product'] ['abbreviateUrl'] .length > 0 && pageData[index]['product'] ['abbreviateUrl'] [0] != '' ? pageData[index]['product'] ['abbreviateUrl'][0] : 'http://image.douquan.com/static/product-default.png'), width: double.infinity, height: 150, fit: BoxFit.cover, ), )), ), Container( padding: EdgeInsets.all(10), child: Column( crossAxisAlignment: CrossAxisAlignment.start, mainAxisAlignment: MainAxisAlignment.start, children: [ Text( pageData[index]['product'] ['productName'] != null ? pageData[index]['product'] ['productName'] : '', maxLines: 1, style: TextStyle( color: Color(0xFF101010), ), ), SizedBox( height: 5, ), Text.rich(TextSpan(children: [ TextSpan( text: '¥', style: TextStyle( color: Color(0xFFC73E3A), fontSize: 14, fontWeight: FontWeight.bold)), TextSpan( text: '${pageData[index]['product']['finalPrice'] != null ? double.parse("${pageData[index]['product']['finalPrice']}").toStringAsFixed(0) : ''}', style: TextStyle( color: Color(0xFFC73E3A), fontWeight: FontWeight.bold, fontSize: 18), ) ])), SizedBox( height: 8, ), Row( children: [ Stack( alignment: Alignment.bottomRight, children: [ ClipOval( child: Image( image: CachedNetworkImageProvider( pageData[index]['product'] [ 'userAvatar'] == '' ? 'http://image.douquan.com/static/product-default.png' : pageData[index] ['product'] ['userAvatar'], ), width: 24, height: 24, fit: BoxFit.cover, ), ), Offstage( child: ClipOval( child: Container( width: 8, height: 8, color: Colors.green, ), ), offstage: "${pageData[index]['product']['onlineStatus']}" == "1" ? false : true, ) ], ), SizedBox( width: 8, ), Flexible( child: Text( pageData[index]['product'] ['userName'], style: TextStyle( fontSize: 12.0, color: Color(0xFF8B8B8B)), ), ) ], ) ], ), ) ], ), ), ), onTap: () { UmengCommonSdk.onEvent("product_tap", { "name": "泉市/${widget.item["customName"]}" }); Navigator.of(context).pushNamed( RouteName.productDetail, arguments: { "productNo": pageData[index]['product'] ['productNo'] }); }), ); }, childCount: pageData.length, ), gridDelegate: SliverSimpleGridDelegateWithFixedCrossAxisCount( crossAxisCount: 2), mainAxisSpacing: 10, crossAxisSpacing: 10, ) When I load too many pictures, the app will flash back?
@wsb1689 mate, I cannot look into the issue of yours plus ugly formatting. I can only tell what works on my side.
@PhantomRay thanks anymore
It seems CachedNetworkImage
solved my reloading issue @wsb1689 why don't you try that (specify only width or height to preserve differing heights)
Thanks for your help pal @PhantomRay
I've tried. If the width and height are set too large, they will crash. If the setting is too small, the picture looks very blurred and not very beautiful
Closing since the performance issues are no longer present
The solution of @BertrandBev is worked for me! thank you.
for 1000 items it will work smooth?
Environment information: Flutter version 1.12.13+hotfix.7 Dart version 2.7.0 Platform: android-29, build-tools 29.0.3
Testing devices: Android device on API level 29 Android emulator for API level 29
Runtime environments tested: Debug Profile
~~The issue: When scrolling scrolling down a list of items,
SliverStaggeredGrid
executesitemBuilder()
for every item in its data set whenever a single item becomes visable in the viewport, this appears to cause unacceptable performance degredation on both UI and logic threads, 10fps etc.~~A large number of cells are rebuilt each frame when scrolling.
It also seems to agressively call
staggeredTileBuilder()
though I'm not sure if that's by design.