finding memory leaks using HAT

[qazmlp]

The following code has memory leaks(due to incorrect pop()
implementation). But, the 'Heap Analysis Tool' does not report any
such leaks. Can anybody clarify it?

public class Stack {

public static void main (String []a) {

Stack stackInst = new Stack() ;
String str = new String("String");
stackInst.push( str ) ;
stackInst.pop();

}

private static final int MAXLEN = 10;
private Object stk[] = new Object[MAXLEN];
private int stkp = -1;

public void push(Object p) {
stk[++stkp] = p;
}

public Object pop() {
return stk[stkp--];
}
}

Commands used:
java -Xrunhprof:file=java.hprof,format=b Stack
hat/bin/hat java.hprof

 

[Brad BARCLAY]

The following code has memory leaks(due to incorrect pop()
implementation). But, the 'Heap Analysis Tool' does not report any
such leaks. Can anybody clarify it?

Because there isn't any leaking going on in pop().

private static final int MAXLEN = 10;
private Object stk[] = new Object[MAXLEN];

Right here you're forcing the memory size of the stack array to contain
10 object references. This memory is reserved (and thus used) wether
you have 1 object in the array or 10.

private int stkp = -1;

Java arrays are 0-based, so you are wasting some memory by starting at 1.

public void push(Object p) {
stk[++stkp] = p;
}

You probably want to actually use "stk[stkp++] = p;" here. If you
pre-increment, you'll actually be starting by adding the first element
to array position 2, now wasting two slots in your array (meaning it
will only hold, at most, 8 elements).

public Object pop() {
return stk[stkp--];
}
}

Nothing wrong here -- as mentioned above, the space in the array for
the object reference is used anyhow. You're not going to get it back by
removing an element.

Note that, in Java, when an element is no longer referenced it will
automatically be garbage collected. Thus the actual object being
returned will have its memory automatically deallocated once it goes
completely out-of-scope.

The only minor issue I can see with pop() is that you're not taking the
popped object reference out of your array. Thus, you'll always have one
handle to it _until_ you push enough objects back onto the array to
overwrite its reference, at which point it will go out-of-scope
(assuming no other objects are holding a handle to it, that is).

You can influence its scope (and thus garbage collection) in pop() by
changing it to something like this:

public Object pop() {
Object ret = stk[stkp];
stk[stkp--] = null;
return ret;
}

Now whenever you remove an object from your stack, you'll be setting
its pointer in the array to null, thus deferencing it in the array. Now
when no other object holds a handle to the popped item it can be garbage
collected -- it won't stick around until you eventually push enough
objects onto the stack to overwrite it (which could be a long time once
you hit array element 9 if you pop a number of objects off the stack and
then only use it intermittently, or don't fill it up again -- the
objects at the top of the array could stick around for the rest of the
lifetime of your application!).

Brad BARCLAY

 

[John Howells]

The following code has memory leaks(due to incorrect pop()
implementation). But, the 'Heap Analysis Tool' does not report any
such leaks. Can anybody clarify it?

Because there isn't any leaking going on in pop().

private static final int MAXLEN = 10;
private Object stk[] = new Object[MAXLEN];

Right here you're forcing the memory size of the stack array to contain
10 object references. This memory is reserved (and thus used) wether
you have 1 object in the array or 10.

private int stkp = -1;

Java arrays are 0-based, so you are wasting some memory by starting at 1.

That is MINUS 1, not 1.

public void push(Object p) {
stk[++stkp] = p;
}

You probably want to actually use "stk[stkp++] = p;" here. If you
pre-increment, you'll actually be starting by adding the first element
to array position 2, now wasting two slots in your array (meaning it
will only hold, at most, 8 elements).

But with stkp initialized to MINUS 1 the first slot used is zero, and no
slots are wasted. Changing from "++stkp" for the push and "stkp--" for
the pop (as originally written) to "stkp++" and "--stkp" will have no
significant effect.

John Howells

 

[Brad BARCLAY]

That is MINUS 1, not 1.

Good catch -- I completely missed the minus sign for some reason.

To the OP -- ignore my diatribe about the whole 0-based array thing.
Mea culpa.

Brad BARCLAY