search

kolinkrewinkel / KKGridView

Deprecated: Grid view library for iOS.
https://github.com/kolinkrewinkel/KKGridView
Other
818 stars 152 forks source link

Potential Memory Issue with New Cell Recycling Implementation #128

Open kolinkrewinkel opened 12 years ago

kolinkrewinkel commented 12 years ago

If we're not removing them, and just hiding them, aren't we creating potential to just recreate new cells and create an infinite stack?

jonsterling commented 12 years ago

@zadr?

zadr commented 12 years ago

I haven't noticed any memory issues when using this code. The point of reuse is to keep cells around for the duration of the gridview, anyway. It's rare for a cell to go away before the gridview does.

If you like, I can add some logic on cell deletion (and low memory warnings, probably) to check to see if the total # of cells we have is less than the number of cells on screen, and remove unused cells as necessary. My thought for not doing this in the first place was that if we needed these cells in the first place, we will likely need them again.

kolinkrewinkel commented 12 years ago

I think before we move onto the removal/hidden thing, there's some sort of logic error with the cell recycling, as pointed out to me by several people (with cells lingering around after they were supposed to have been removed.)

zadr commented 12 years ago

Yeah, I made a comment on ticket #129, and saw Jon's comment on #127. Looking into it now. Wondering how I didn't notice this before, as well.

kolinkrewinkel commented 12 years ago

Yeah, it's visible with cells with variable content and translucent content. Not really sure why, as it was bulletproof from what I could tell, even through @Gi-lo's testing.

zadr commented 12 years ago

Could it have been a problem with my merge picking the wrong thing?

kolinkrewinkel commented 12 years ago

No, I got these reports before the merge.

zadr commented 12 years ago

This (memory) should be addressed by https://github.com/kolinkrewinkel/KKGridView/pull/130