This code is part of a compiler and stores some metadata on an Abstract Syntax Tree node during byte-code creation. The profiler told me that there were thousands of ClosureWriter instances being kept around for the length of the compile (roughly one for each closure defined in the program). While not a huge amount of memory, this bothered me. For a while, I couldn't figure out why these references stuck around. Then it occurred to me that each reference to ClosureWriter.UseExistingReference.class keeps a back pointer to the enclosing ClosureWriter instance. By changing the UseExistingReference to static, the leak disappeared.
What's the lesson?
In Java, always, always declare your inner classes as static unless you are sure that you need back references to the enclosing instance.
Not sure of the exact context you extracted the code from, but: Are you really just using an interface type, or just its name, as a replacement for constants?
ReplyDeleteThen, the lession learnt should be: code acrobatics never pays off :-)
I didn't write this code and I'm not really sure why a class constant was used. It's not code that I maintain directly and so I didn't want to disturb anything in case there was a reason. That's another lesson I've learned: don't make changes to code unless you fully understand the consequences (or are at least fully willing to deal with them).
ReplyDeleteWeird, I'd expect static to matter if you were talking about an inner class (because instances of the inner class keep a reference to the enclosing instance of the outer class), but not for an inner interface (as instances of classes implementing an inner interface have no relationship to the outer class instances).
ReplyDeleteI can't reproduce this, and it shouldn't leak, since the class objects will be the same unless you are doing classloader tricks.
ReplyDeleteInterfaces have to concept of outer classes.
Rather, the compiler ensures that each instance which needs an outer ref has one, potentially using the odd and rare constructor placement syntax (which was slightly broken before JDK 7)
I can't paste my counter-example the code here, but it's at: http://pastebin.com/qpJ0gLQX
Please elaborate!
Also, see
ReplyDeletehttp://stackoverflow.com/questions/71625/why-would-a-static-inner-interface-be-used-in-java
Hmmm...looks like you're right. Thanks for correcting me. There was a memory leak in the code somewhere. ClosureWriters were being kept around when they shouldn't. Perhaps it was something else. When I get a chance, I'll have to take another look at the code.
ReplyDelete