cameron314 / PNGEncoder2

A better PNG encoder for Flash
97 stars 10 forks source link

Memory leak #3

Closed cameron314 closed 11 years ago

cameron314 commented 11 years ago

When encoding many images, the memory usage of Flash Player steadily increases and never declines.

I tried picking some low-hanging fruit (objects that were being created every frame), but it seems there's still a leak, particularly of Vector<*>, somehow (investigation of the code the HaXe compiler produces is probably in order).

This issue was brought to my attention by Kamel on my blog -- thanks for the debugging help!

Screenshot of profile (courtesy of Kamel): PNGEnocder2 memory leak

mattchan commented 11 years ago

I read somewhere about using the BitmapData.dispose() call that would fix the problem. Would this help at all? I haven't tried it myself, but this is what the AS3 docs for it say:

public function dispose():void

Frees memory that is used to store the BitmapData object.

When the dispose() method is called on an image, the width and height of the image are set to 0. All subsequent calls to methods or properties of this BitmapData instance fail, and an exception is thrown.

BitmapData.dispose() releases the memory occupied by the actual bitmap data, immediately (a bitmap can consume up to 64 MB of memory). After using BitmapData.dispose(), the BitmapData object is no longer usable and the Flash runtime throws an exception if you call functions on the BitmapData object. However, BitmapData.dispose() does not garbage collect the BitmapData object (approximately 128 bytes); the memory occupied by the actual BitmapData object is released at the time the BitmapData object is collected by the garbage collector.

cameron314 commented 11 years ago

I haven't set up any test conditions for this issue yet, so I can't say for sure, but I don't think this would fix the problem; theoretically, the GC would destroy the bitmaps after a while anyway (it looks like the GC can't collect the memory here).

Adding a call to dispose() could also break certain usage scenarios where the BitmapData object is used after being encoded.

But, I'll keep this in mind when I get around to working on this issue. Thanks!

sa6k0o0 commented 11 years ago

very strange, sometimes in the code the clearing of the memory is completely fine, other times however really weird stuff happen.

I did some testing in AdobeScout regular Png compression from adobe doesn't have memory leak. This one is really cool, but the weird thing is that at some places when I use it it has memory leaks, the weird thing is that it is not being constantly added.

Adobe Scout detects 16454 bytes in Othe ByteArrays and 1 byte in DomainMemory , this is the memory leak which I can't get rid of, switching to regular Png compression from adobe fixes it but it is slow as hell.

sa6k0o0 commented 11 years ago

It would be really cool if this issue can be fixed

cameron314 commented 11 years ago

Yep, this definitely needs to be fixed! I'm super busy with school at the moment, but I'll look at it as soon as I can. BTW, does anybody know if PNGEncoder2 still works with the new Flash (11+)? I haven't tested it in a while, but I know Adobe was killing off the old alchemy (which it relies on) in newer versions...

sa6k0o0 commented 11 years ago

well it does work in 11+ , it works on AIR for mobile too, except that memory leak.

cameron314 commented 11 years ago

I have some time to work on this issue now. I'm going to need a little help reproducing it though -- when I try encoding many images asynchronously (and throwing away the results after it's done), my memory usage goes up then falls like it should (leading to overall constant memory performance).

This is what I see in Scout (bottom section is memory): image

cameron314 commented 11 years ago

Unless someone can provide a test case that reproduces this problem, I'm going to have to close this as "unable to reproduce". I just wrote another sample application (in AS3 with FlashBuilder this time -- the one above is similar but in haXe 2), and the memory usage goes up and down as expected. Screenshot:

image

Here's the sample application I'm using:

package
{
    import flash.display.Sprite;
    import flash.net.FileReference;
    import flash.display.Loader;
    import flash.events.*;
    import flash.display.*;

    public class Main extends Sprite
    {
        public function Main()
        {
            addEventListener(Event.ADDED_TO_STAGE, init);               
        }

        private function init(e : Event)
        {
            stage.doubleClickEnabled = true;
            stage.addEventListener(MouseEvent.DOUBLE_CLICK, onDoubleClick);
        }

        private function onDoubleClick(e : MouseEvent)
        {
            var fileReference : FileReference = new FileReference();
            fileReference.addEventListener(Event.SELECT, function (e2) {
                fileReference.load();
            });

            fileReference.addEventListener(Event.COMPLETE, function (e2) {
                var loader : Loader = new Loader();
                loader.contentLoaderInfo.addEventListener(Event.COMPLETE, function (e3) {
                    var bmp : BitmapData = new BitmapData(int(loader.width), int(loader.height), true, 0x00FFFFFF);
                    bmp.draw(loader);

                    var encoder : PNGEncoder2 = PNGEncoder2.encodeAsync(bmp);
                    encoder.addEventListener(Event.COMPLETE, function (e) {
                        var percent : Number = 100 - encoder.png.length / (bmp.width * bmp.height * 4) * 100;
                        trace("Async complete (" + Math.floor(percent) + "%)");
                    });
                });

                loader.loadBytes(fileReference.data);
            });

            fileReference.browse();
        }
    }
}
cameron314 commented 11 years ago

And here's the memory usage (again, normal) if I use encode instead of encodeAsync:

image

This is with Flash Player 11.7 debug (it displays as 11.4), btw.

sa6k0o0 commented 11 years ago

well, after it goes down, are you looking specifically at the bytearays staying in the memory in scout ?

sa6k0o0 commented 11 years ago

the problem is that I did use it on about 3 places in the code of a game and only on one of the places that memory leak is very obvious , the only reason I can tell is from the bytearrays in Scout showing that that memory is still busy. I am a little busy with a project, but after that I will try to nail it down to show example of the memory not clearing

cameron314 commented 11 years ago

Excellent, thanks for the help. Take your time.

I am looking at the ByteArrays, however all seems normal -- a little bit is always leftover since there's a few bytes permanently reserved for a CRC lookup table and scratch memory for the compression (initialized the first time an image is encoded), 6600 bytes to be precise. I can't explain the "1 byte" always in domain memory, but I have a suspicion that's because the domain memory is a special byte array (part of Alchemy) and probably isn't really a normal ByteArray object at all internally.

Each time an image is encoded, a new ByteArray is created to store the result -- if references to these ByteArrays are persisted somewhere outside PNGEncoder2, then memory usage will go up -- but as far as I can tell so far, knowing the code and also from profiling, PNGEncoder2 itself doesn't hold on to any memory it shouldn't be.

There's one other interesting detail about the way memory is used -- there's one ByteArray (the domain memory) that is re-used for every image. Its length is reset for every image (several times per image, actually), but in between encoding images it just sits there with whatever data it had before -- so a potentially large number of bytes are always on reserve even though they're not needed; I could change that, but it's a constant amount of usage, not a leak like others are seeing.

It's also possible that the domain memory (or even just ByteArrays in general) are never actually shrunk in size when their length is reduced, as an optimization (this makes sense). That would mean that the re-used byte array would hold onto the maximum amount of memory that it ever peaked at. But again, this is constant and would not cause an ever-increasing leak.

So, to make a long story short, everything seems normal on my end, but it's difficult to tell exactly what's going on without a reliable test case :-)

cameron314 commented 11 years ago

Since I can't reproduce this and I haven't received any external test cases, I'm going to close this issue for now.

If anyone can provide a test case, please do and I'll be glad to re-open the issue.

francisvidal commented 9 years ago

Hi. On mobile there is a 24889 bytes in use in a BytesArray even after I clean everything. It seems to be in the static variable data. I don't think it's leaking, but I need every memory for my app, not for a one time operation. Can you .clear(); the data variable after the COMPLETE event? Or add a destroy method that we can call to do it?

Thanks.

sa6k0o0 commented 9 years ago

Well, I don't have the code with me right now, I have it on my desktop in Bulgaria. And now I am in Los Angeles , California.

But it is possible that the runtime from adobe at the time might have been buggy and might have created that memory issue. I actually switched away from flash and now I make video games with Unity. Flash had too many bugs.

Thanks Aleks

On Thu, Feb 19, 2015 at 11:07 AM, francisvidal notifications@github.com wrote:

Hi. On mobile there is a 24889 bytes in use in a BytesArray even after I clean everything. It seems to be in the static variable data. I don't think it's leaking, but I need every memory for my app, not for a one time operation. Can you .clear(); the data variable after the COMPLETE event? Or add a destroy method that we can call to do it?

Thanks.

— Reply to this email directly or view it on GitHub https://github.com/cameron314/PNGEncoder2/issues/3#issuecomment-75114900 .

cameron314 commented 9 years ago

@francisvidal: I'll have to look at the code, but I do recall having some reusable memory hanging around (particularly a CRC-32 table that's generated just the first time, if I'm not mistaken). I'll add a method to clear all cached data for resource-constrained users.

@sa6k0o0: Yep, too many bugs indeed!

francisvidal commented 9 years ago

@cameron314 & @sa6k0o0 Thank you both for your fast answer.

In the debugger I can see that this var: (private static var data : ByteArray;// The selected working memory) still hold some memory after the encoding. I don't know about the CRC-32 table. In Scout I also see some byteArray that is never released.

I have done some test with Haxe some years ago, but to setup myself and understand the code would really take my quite some time. So if anyone of you can take a look and solve this issue I would be very grateful.

BTW the speed of this library is amazing!

cameron314 commented 9 years ago

@francisvidal: I've pushed a commit to the master which adds a freeCachedMemory() method. You can call it after encoding an image to let go all the temporary memory that is normally hogged to make the next encoding faster. Depending on how you encode your images, and their size, this may save several kilobytes or even megabytes of memory :-)

The change will be in the next release (there's another small feature I'd like to add before then), but in the meantime up-to-date binaries are committed on the head of the master.

francisvidal commented 9 years ago

Thank you so much!

cameron314 commented 9 years ago

@francisvidal: I've published a new release which contains this feature. Cheers :-)