variable = null; Memory Cleanup

[Gordon Beaton]

I have several developers who insist on using the following method,
claiming that it helps reduce memory use by eliminating resources
faster than the garbage collector. Personally, I don't think this is
effective. Is this a useful method, or should we simply be letting
the garbage collector do its job?

/**
* This method closes the ResultSetMetaData if it is open.
*/
public static void closeResultSetMetaData(ResultSetMetaData rs)
{
try {
rs = null;
} catch (Exception exp) {
exp.printStackTrace();
}
}

The method is completely bogus.

No method can change the object reference still held by the caller,
and calling the method just to null the resulting local reference is
nonsensical. Also no exceptions can be thrown within the given try
block, so catching one here is pointless.

/gordon


--

 

[Angry Moth Town]

I have several developers who insist on using the following method,
claiming that it helps reduce memory use by eliminating resources
faster than the garbage collector. Personally, I don't think this is
effective. Is this a useful method, or should we simply be letting
the garbage collector do its job?

/**
* This method closes the ResultSetMetaData if it is open.
*/
public static void closeResultSetMetaData(ResultSetMetaData rs)
{
try {
rs = null;
} catch (Exception exp) {
exp.printStackTrace();
}
}

 

[Stefan Ram]

I have several developers who insist on using the following method,
[...]rs = null;

This only changes the parameter copy of the reference,
so it can not have any observable effect, and it will
never throw. Its author does not seem to know basic Java.

 

[Angry Moth Town]

This method seems utterly pointless. Are you sure you've
given a proper illustration of the technique you're asking about?

Eric,

Yes, this is the exact method, copied and pasted, and the explanation
I got was that it "clears up memory faster, because the garbage
collector is too slow."

I'm basically going to tell 25 offshore developers that this is
pointless, but I'm not a Java expert, so I wanted to run it through
the newsgroup before I insisted we change it.

This isn't as bad as it gets.

 

[Jan Thomä]

This isn't as bad as it gets.

I fear exactly this is going to happen. I wonder what other "optimizations"
of that kind you will find in the code....

Jan

 

[Angry Moth Town]

All right, tell them it's pointless. It's pointless
for more than one reason:

- First, setting `rs = null' affects only the method's
own copy of the argument value, and does not affect any copies
the caller might have. If the caller does

ResultSetMetadata myRs = ...;
closeResultSetMetaData(myRs);

then myRs is not changed at all. The object myRs refers to
is still "live" and is not eligible for garbage collection;
the method has made no change whatever in its status.

- Second, the try/catch block is pointless, because there
is nothing in `rs = null' that can throw an Exception. The
writer of the try/catch was a seriously confused person.

- Third, setting all the references to a particular object
to null does not avoid garbage collection, it just makes the
object eligible for collection. If the JVM decides to run the
garbage collector it will do so, and the collector may reclaim
the garbage object's memory. "Clearing up the memory faster"
is almost entirely bogus; all that might happen is that the
memory could be reclaimed sooner. Sooner != faster.

(The "almost" is because some garbage collectors must
work harder to reclaim long-lived objects than those that die
young. Setting all the references to null -- or pointing them
at other objects, or letting the references themselves die --
makes the object garbage, and making it garbage sooner rather
than later might let it get collected on the next GC instead
of on the one after it. Thus the effective lifetime might
be shortened and the collector might therefore work a little
less hard -- but this is an effect so small it would be hard
to measure.)

A suggestion: If your compatriots are so convinced that
their method produces a performance improvement, challenge them
to measure the size of the improvement by actual experiment.
Run the code with the method as it stands and run it again with
the guts of the method commented out, and ask them how much
difference it made. My bet: If the measurement is done with
great care, the commented-out version will run just a hair
faster than the "optimized" original.

Eric,

Thank you so much for this reply. This is exactly what I suspected,
and definitely what I wanted to hear.

I suspect this came about after someone discovered that connections
have to be closed to prevent memory leaks, and decided we needed to
'close' everything we can. I'm assuming that even:
ResultSet rs = ...
rs = null;
is pointless, since the collector will know to clean up rs after it is
no longer used?

The application this is being developed for is Sun Identity Manager,
so I can't really measure the performance of the difference, but the
logic you present above should be enough to convince people that this
is wrong, and start making changes.

Thanks again Eric, this is a big help. Now I just need to get people
to stop adding 'throws Exception' to the end of every method...

 

[Mark Space]

Yes, this is the exact method, copied and pasted, and the explanation
I got was that it "clears up memory faster, because the garbage
collector is too slow."

I'm basically going to tell 25 offshore developers that this is
pointless, but I'm not a Java expert, so I wanted to run it through
the newsgroup before I insisted we change it.

This isn't as bad as it gets.

I fear you're being taken for a ride. Developer is charging for
un-maintainable code. I'd cancel their contract immediately.

Sorry but that's what you get for going off shore.

 

[Mark Thornton]

void method() {
BigThing thing = new BigThing();
// ... use `thing' for a while
// ... memory-hungry code not using `thing'
}

The idea here is that the variable `thing' survives until
the method returns, so the BigThing it points to doesn't
become collectible until then. Its actual lifetime runs
all the way to the end of the method, but its "useful
lifetime" ends after the first comment. If a BigThing is
really really big and the second half of the method uses a
lot of memory and/or runs for a really long time, it might
be worth while to "kill" the reference early by inserting
`thing = null;' between the two comments.

Its real lifetime only extends to the point of last use which need not
be the end of the method. At least some JVM do collect such objects
before the end of the method.

Mark Thornton

 

[Abhijat Vatsyayan]

I have several developers who insist on using the following method,
claiming that it helps reduce memory use by eliminating resources
faster than the garbage collector. Personally, I don't think this is
effective. Is this a useful method, or should we simply be letting
the garbage collector do its job?

/**
* This method closes the ResultSetMetaData if it is open.
*/
public static void closeResultSetMetaData(ResultSetMetaData rs)
{
try {
rs = null;
} catch (Exception exp) {
exp.printStackTrace();
}
}

Just out of curiosity - is there another method named
closeResultSet(ResultSet rs) which might look something like this -

try {
rs.close() ;
rs = null;
} catch (Exception exp) {
exp.printStackTrace();
}
}

If there is and if method "closeResultSetMetaData" was added after
method "closeResultSet", there might be an amusing explanation (no,
there still is no excuse for this!). Its possible that at one time all
developers were expected to explicitly close connections and streams
using these type of methods. Later on, developers blindly followed the
example without really understanding the intent (they assumed it was for
garbage collection but the name tells me otherwise). And since
ResultSetMetaData does not have a close() method, you are left with this
ridiculous code! Its rather amusing and I would like to know what you
find out.

Also - don't these guys use logging (log4j/commons logging ... )?

Abhijat

 

[Abhijat Vatsyayan]

> I have several developers who insist on using the following method,
> claiming that it helps reduce memory use by eliminating resources
> faster than the garbage collector. Personally, I don't think this is
> effective. Is this a useful method, or should we simply be letting
> the garbage collector do its job?
>
> /**
> * This method closes the ResultSetMetaData if it is open.
> */
> public static void closeResultSetMetaData(ResultSetMetaData rs)
> {
> try {
> rs = null;
> } catch (Exception exp) {
> exp.printStackTrace();
> }
> }

Just out of curiosity - is there another method named
closeResultSet(ResultSet rs) which might look something like this -

try {
rs.close() ;
rs = null;
} catch (Exception exp) {
exp.printStackTrace();
}
}

If there is and if method "closeResultSetMetaData" was added after
method "closeResultSet", there might be an amusing explanation (no,
there still is no excuse for this!). Its possible that at one time all
developers were expected to explicitly close connections and streams
using these type of methods. Later on, developers blindly followed the
example without really understanding the intent (they assumed it was for
garbage collection but the name tells me otherwise). And since
ResultSetMetaData does not have a close() method, you are left with this
ridiculous code! Its rather amusing and I would like to know what you
find out.

Also - don't these guys use logging (log4j/commons logging ... )?

 

[Arne Vajhøj]

This isn't as bad as it gets.

Start writing your article for www.thedailywtf.com !

Arne

 

[Roedy Green]

try {
rs = null;
} catch (Exception exp) {
exp.printStackTrace();

there is no exception possible from rs=null;

It will however free the associated object. It is not usually
necessary for loca variables because you will usually exit the method
and all locals go out of scope within a millisecond anyway. However,
some methods are long running, and setting to null will indeed free
objects for potential later GC before the method terminates.

 

[Gordon Beaton]

It will however free the associated object.

It will not. The calling method continues to hold a reference to it.

/gordon

--

 

[Roedy Green]

It will not. The calling method continues to hold a reference to it.

It depends how it was created. If it were passed as a parameter, the
caller may have created it as an expression, holding no handle to it
other than the passed parameter which can be nullified.

If it were created in the current method, the caller knows nothing
about it.

If the caller allocated it, pointed to it with its own local variable
and passed it as a parameter, than the callee nullifying the reference
would not do any good.

Happily nullifying a reference is a fairly quick operation.

 

[Gordon Beaton]

If it were created in the current method, the caller knows nothing
about it.

It wasn't. Did you not read the thread?

Happily nullifying a reference is a fairly quick operation.

However creating a method to do so is, as many others have already
pointed out in this thread, nonsense.

/gordon

--