LeprovostQuentin / mt4j

Automatically exported from code.google.com/p/mt4j
GNU General Public License v2.0
0 stars 0 forks source link

setWidthLocal() and setHeightLocal() leak native memory #39

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create an MTRectangle.
2. Call .setWidthLocal(), or .setHeightLocal() on the rectangle repeatedly.
3. Memory is leaked.

What is the expected output? What do you see instead?
I would expect the dimensions of the rectangle to reflect the value passed in 
without leaking.

What version of the product are you using? On what operating system?
Tested against SVN trunk, branches/new (r501), and the v09 and v095 tags.  
Ubuntu 11.04 amd64, Java 1.6 64-bit jvm.  Although I didn't run this test code 
on Windows, I believe it leaks there, too.

Please provide any additional information below.
Here's a quick scene that will reproduce it: http://pastebin.com/zHEHVdX2
If let run, it will eat up a few gigs pretty rapidly, so adjust the end 
condition of the for loop accordingly.  If you run a heap profiler along side 
it (I've been using jvisualvm), you can see the heap doesn't grow out of 
control, but the java process does.  Which means a native leak, I think.

Original issue reported on code.google.com by _...@jpk.is on 2 Aug 2011 at 5:04

GoogleCodeExporter commented 8 years ago
I found that if you put a short sleep in the loop (like, 1ms), memory gets 
freed like I'd expect (albeit the loop takes much longer).  I also found that 
manually calling the garbage collector with System.gc() can free most of the 
memory, but letting it run automatically won't.

I notice that the objects that pile up in the heap are instances of 
java.nio.DirectByteBuffer, sun.misc.Cleaner, and 
java.nio.DirectByteBuffer$Deallocator.  Is that significant?

Original comment by _...@jpk.is on 2 Aug 2011 at 7:37

GoogleCodeExporter commented 8 years ago
Stepping through with the debugger, I found where direct buffers are being 
allocated and replaced.  One of them is the stack below:

Thread [Animation Thread] (Suspended)   
    GeometryInfo.setVertexBuffer(FloatBuffer) line: 444 
    GeometryInfo.generateDefaultVertexBuffer() line: 400    
    GeometryInfo.generateNewVertsColStrokeColTexBuffers(StyleInfo) line: 358    
    GeometryInfo.reconstruct(Vertex[], Vector3D[], int[], boolean, boolean, StyleInfo) line: 199    
    MTTextArea(AbstractShape).setVertices(Vertex[]) line: 418   
    MTTextArea(MTPolygon).setVertices(Vertex[]) line: 164   
    MTTextArea.setHeightLocal(float) line: 756  
    MTTextArea.<init>(PApplet, IFont) line: 168 
    MTTextArea.<init>(PApplet) line: 130    
    ATScrollingTextArea.<init>(float, float, float, int) line: 97   
    SplashScene.<init>(String, String, MTColor, ITransition, int, int, IFont) line: 101 
    StartMap.startUp() line: 27 
    StartMap(MTApplication).setup() line: 605   
    StartMap(PApplet).handleDraw() line: not available  
    StartMap(PApplet).run() line: not available 
    Thread.run() line: 679  

MT4j folk probably know this, but I'll talk it out to make sure I'm not crazy:  

When setVertexBuffer(), there, gets called, it replaces the current buffer with 
a new one.  So the buffer is eligible for garbage collection.  The thing about 
direct buffers is their content is stored natively, so their total memory 
footprint doesn't count against the total heap use.  The garbage collector only 
knows about what's in the heap.  When it runs automatically, it only does so to 
keep the heap in check, and doesn't always clean out every unreferenced 
instance.  When you run System.gc(), it tries to clear out as much as it can.

When you call something that ends up replacing a direct buffer in quick 
succession (like calling setHeightLocal() in an animation), new buffers are 
allocated and old ones become eligible for garbage collection.  The issue is 
that because the increase in heap use that this causes isn't all that much, the 
garbage collector doesn't really care, and either lazily clears on the direct 
buffer objects, or just grows the size of the heap assuming you're under the 
limit (-Xmx(blah)).  Meanwhile, the native memory being allocated for the 
buffers piles up.

The problem my application has is that the allocations for the direct buffers 
in native space build up until there isn't enough space to make threads (who's 
stacks live in native space), and the program dies with a 
"java.lang.OutOfMemoryError: unable to create new native thread".

MT4j probably has to use native buffers for performance reasons, and there's 
not a way (that I know of) to tell the buffers or the garbage collector to go 
ahead and free up the native allocation.  So, I don't think MT4j is doing 
anything wrong, here.  (So feel free to mark this issue invalid.)  I'll still 
have to figure out how to work around this, though.  A well timed System.gc() 
might be doable.  I could always just throw more RAM at it, too.  :-P

Original comment by _...@jpk.is on 2 Aug 2011 at 9:08

GoogleCodeExporter commented 8 years ago
Wath happend if you change the setWidthLocal with the same vertex array instead 
of create a new one, change the method from:
public void setSizeLocal(float width, float height){
        if (width > 0 && height > 0){
            Vertex[] v = this.getVerticesLocal();
            this.setVertices(new Vertex[]{
                    new Vertex(v[0].x,          v[0].y,         v[0].z, v[0].getTexCoordU(), v[0].getTexCoordV(), v[0].getR(), v[0].getG(), v[0].getB(), v[0].getA()), 
                    new Vertex(v[0].x+width,    v[1].y,         v[1].z, v[1].getTexCoordU(), v[1].getTexCoordV(), v[1].getR(), v[1].getG(), v[1].getB(), v[1].getA()), 
                    new Vertex(v[0].x+width,    v[1].y+height,  v[2].z, v[2].getTexCoordU(), v[2].getTexCoordV(), v[2].getR(), v[2].getG(), v[2].getB(), v[2].getA()), 
                    new Vertex(v[3].x,          v[0].y+height,  v[3].z, v[3].getTexCoordU(), v[3].getTexCoordV(), v[3].getR(), v[3].getG(), v[3].getB(), v[3].getA()), 
                    new Vertex(v[4].x,          v[4].y,         v[4].z, v[4].getTexCoordU(), v[4].getTexCoordV(), v[4].getR(), v[4].getG(), v[4].getB(), v[4].getA()), 
            });
        }
    }

To the new one:
public void setSizeLocal(float width, float height) {
        Vertex[] v = getVerticesLocal();
        v[1].x = v[0].x + width;
        v[2].x = v[1].x;
        v[2].y = v[1].y + height;
        v[3].y = v[2].y;
        setVertices(v);
    }

Original comment by rekie...@gmail.com on 29 Apr 2012 at 12:54