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

Invalidate SinglePointRenderer cache for recycled Bitmaps #101

Open
ComBatVision opened this issue Jul 24, 2022 · 6 comments
Open

Invalidate SinglePointRenderer cache for recycled Bitmaps #101

ComBatVision opened this issue Jul 24, 2022 · 6 comments

Comments

@ComBatVision
Copy link
Contributor

ComBatVision commented Jul 24, 2022

Hi!
Could you, please, make small improvement and release update - invalidate cache of single point images in case of Bitmap was recycled.

Class SinglePointRenderer.java line 843 - || ii.getImage().isRecycled()

            //see if it's in the cache
            ii = _tgCache.get(key);
            //if not, generate symbol
            if (ii == null || ii.getImage().isRecycled())//*/

And generally what about bitmap recycling? As I understand your library use LryCache and it does not recycle Bitmaps when it removes it from cache. Who and when should recycle Bitmaps?

I am developing NASA WorldWindAndroid, which is using your library to render tactical graphics.
In my code I have added recycling bitmaps after they are loaded to OpenGL texture.
Your built-in cache start returning me recycled bitmaps on subsequent requests of the same icon.
I can copy or do not recycle bitmaps received from your library, but I think it will be good to have "fool guard" and invalidate recycled bitmaps from cache is application recycled it for some reasons.
In other case image remains recycled and never recreates again.

Furthermore, I think library should recycle bitmaps when removing it from LruCache, or it will be native memory leak, istn't it?

Thanks.

@michael-spinelli
Copy link
Contributor

I can check if the Bitmap was recycled by someone else at some point and remove it from the cache.

The project is retired so I don't really want to make a lot of changes at this point but I can add that check.

My impression was after Honeycomb (API13), Bitmaps were stored in VM memory. So, if there's no references, it should get garbage collected at some point.

I'll be putting out a 0.1.50 release shortly. Let me know if that works for you.

@ComBatVision
Copy link
Contributor Author

Hi,

Probably you are right. Bitmap.recycle() documentation claims that "This is an advanced call, and normally need not be called, since the normal GC process will free up this memory when there are no more references to this bitmap."

So I really do not know if recycling of Bitmaps is necessary. It was my attempt to fix native memory leaks in my application.

But I think this IsRecycled() check will not harm anything, but I will handle situations, when somebody like me recycled Bitmap. Now cache begins to return recycled bitmaps for some symbol code till it will not be trimmed from the cache.

I will investigate more about recycling, but I read somewhere that they return storage of bitmap in native memory in some new Android versions, but I may be wrong.

@ComBatVision
Copy link
Contributor Author

ComBatVision commented Jul 25, 2022

I found why I added Recycling....
Android documentation says:
On Android 2.3.3 (API level 10) and lower, the backing pixel data for a bitmap is stored in native memory. It is separate from the bitmap itself, which is stored in the Dalvik heap. The pixel data in native memory is not released in a predictable manner, potentially causing an application to briefly exceed its memory limits and crash. From Android 3.0 (API level 11) through Android 7.1 (API level 25), the pixel data is stored on the Dalvik heap along with the associated bitmap. In Android 8.0 (API level 26), and higher, the bitmap pixel data is stored in the native heap.

So I think it will be good not only invalidate recycled bitmaps from cache, but also recycle bitmaps before removing them from cache, because it may create native memory leak, isn't it?

https://developer.android.com/topic/performance/graphics/manage-memory.html

@ComBatVision
Copy link
Contributor Author

ComBatVision commented Jul 25, 2022

Other article claims:
In Android 8.0 (API 26) and later, bitmap pixel data is stored in the native heap. Of course, although the bitmap pixel data is put back into the native heap, it will be released with the release of Java objects.
Whether the recycling implementation before or after API 26, the idea of releasing the bitmap object of the native layer is to monitor whether the bitmap of the Java layer is released. Once the bitmap object of the Java layer is released, the bitmap of the native layer is released immediately.

So, I do not really understand if Bitmap.recycle() is required in Android 8.0+, but isRecycled() check will be useful anyway, just to keep consistency of cache in case somebody like me recycle bitmap in application.

@michael-spinelli
Copy link
Contributor

michael-spinelli commented Jul 25, 2022

I could be wrong but it seems like if you needed the memory freed sooner than later (with eventual garbage collection), you'd call recycle. Otherwise it's not something you have to do.

A new release with the check is up. Let me know if that fixes your issue.

@ComBatVision
Copy link
Contributor Author

Thanks. I will make some experiments and let you know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants