Skip to main content

Jsr199JavaCompiler possible memory leak?

11 replies [Last post]
finbarro
Offline
Joined: 2009-01-14

Hi folks.

First of all, first post, have to say that I've been using the Jsr199JavaCompiler option and it has delivered such a performance boost, well done.

However, I suspect there may be a memory leak in the implementation (as confirmed by some memory dump analysis)

From my read of the code there is a line to add the compiled bytecode to a list within a packageMap table stored within the JspRuntimeContext

Thats fine, except when the jsp reloads, the code continues to add new instances rather than replacing the existing one. I've had cases where a few hundred jsp's consume nearly 2gb of non gc'able memory.

I'd recommend the following:
+ // remove the file (if present)
+ packageFiles.remove(classFile);
packageFiles.add(classFile);
classFiles.add(classFile);

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
finbarro
Offline
Joined: 2009-01-14

Hey there, I may have been a bit quick off the mark as a heap dump shows the same behaviour still happening.

Bit more thinking and the ArrayList will never find a match for one version of a class in Bytecode form with the default implementation of the equals(Object o) method

Suggest overriding the equals(Object o) within the BytecodeFile definition to something like:

[code]
public boolean equals(Object o) {
if ( !(o instanceof BytecodeFile) ) return false;

return this.getClassName().equals(((BytecodeFile)o).getClassName());
}

[/code]

Im sure you could do something more elegant with hashcode but according to java api docs this is more relevant for searching within maps.

Then also make sure that all instances are properly removed before adding a new instance.

[code]
private JavaFileObject getOutputFile(final String className,
final URI uri) {

BytecodeFile classFile = new BytecodeFile(uri, className);

// File the class file away, by its package name
String packageName = className.substring(0, className.lastIndexOf("."));
Map > packageMap = rtctxt.getPackageMap(); ArrayList packageFiles = packageMap.get(packageName); if (packageFiles == null) { packageFiles = new ArrayList(); packageMap.put(packageName, packageFiles); } /* start patch */ /* scan through the list to make sure that it removes all previous instances * of the compiled class ArrayList.remove(Object o) just removes first * instance found so potential for leak still exists */ ArrayList matches = new ArrayList(); for ( Object o : packageFiles ) { if ( ((BytecodeFile)o).getClassName().equals(className) ) matches.add(o); } for ( Object m : matches ) { packageFiles.remove(m); } matches.clear(); matches = null; // then add it to keep most recent copy /* end patch */ packageFiles.add(classFile); classFiles.add(classFile); return classFile; } [/code]
kchung
Offline
Joined: 2004-05-06

You are right again, about the need to override equals method.

Instead of doing the work in your patch, I think it may be cleaner if we use a HashSet or HashMap instead of ArrayList. That way we can be sure that there is only one copy of the compiled bytecode. We can use the class name for the keys to the HashMap. That way we don't even need to override equals method.

What do you think? Thanks.

finbarro
Offline
Joined: 2009-01-14

Sounds reasonable, removes the need to create a temporary list and a double pass to safely delete.

(edit) had a look at JspRuntimeContext.java has the packagemap accessor defined as using an ArrayList
[code]
public Map > getPackageMap() { if (packageMap == null) { packageMap = new HashMap>(); } return packageMap; } [/code] Lastly, after making the attached changes, I now fail to see what the need for the classFiles list is. I added some logging in and the only methods which use the classFiles list are: getOutputFile() and setBytecode() Even though this array are referenced in saveClassFile() the method doesnt appear to get called by the web container. (see note below about my choice of java container, this may not hold true in all cases) The problem is that the classFiles list continues to grow and in my case, there are many instances of the Jsr199JavaCompiler class created, all of which grow its own list. Would there be any reason that we couldnt clear() or null the classFiles list once the Bytecode has been stored in the overall runtime context? Confession I have to make is that Im running this within Sun Web Server 7.0 ... so the above conditions may not hold 100% true for Glassfish implementation. Wish my organisation was brave enough to make the switch :) (edit2) I've looked again the the JspRuntimeContext class and it already has a method for writing bytecode to physical disk. More code snippits but I'd recommend: [code] if (ct.call()) { for (BytecodeFile bytecodeFile: classFiles) { rtctxt.setBytecode(bytecodeFile.getClassName(), bytecodeFile.getBytecode()); } classFiles.clear(); return null; } [/code] Message was edited by: finbarro
kchung
Offline
Joined: 2004-05-06

There are two reasons to keep the bytecodes around:

1. for loading the compiled JSP page. Once a class is loaded, we can certainly remove them from the memory.
2. for javac compilations. When a JSP page is compiled, if the page refers to a class produced by a previous compilation, we need to tell javac where the bytecodes for that class is. packageMap keeps track of such info. This gets tricky when tag files are involved, because tag files are compiled on the fly, so we may suspend a compilation to compile the referenced tags file first, so it is not easy to tell when some bytecodes can be released.

It is true that once a classFiles is created, it only got unloaded when the application is unloaded. That should be OK for development. For deploying real application, it is always a good idea to precompile JSP pages.

kchung
Offline
Joined: 2004-05-06

I now realize that you are talking about instance variable classfiles in Jsr199JavaCompiler. I think the way get rid of it is to get rid of the Jsr199JavaCompiler instance after a compilation is done. Because of the nested compilation issue I mentioned above, it'll have to be done at the top-most level. Let me think about it more...

aramkhas
Offline
Joined: 2010-07-14

We are having the same memory leak issue. Is there a fix released for this issue?

kchung
Offline
Joined: 2004-05-06

Ah.. You are so right! I'll apply your fix and it should be included in the next promotion build.

Also, welcome to the GlassFish community and thanks reporting the problem and providing the fix.

finbarro
Offline
Joined: 2009-01-14

No worries, glad to help.

Dominic McGinnis

On this note is the JSR199 in-memory compiler used as the default compiler for v2ur2 using JDK 1.6?  From my understanding that appears to be the case, but I'm having trouble finding any documentation that states this as a fact. 

Also if so, how does one bypass this and fall back to using the non in-memory compiler?

Thanks,
Dominic

________________________________
From: "glassfish@javadesktop.org"
To: users@glassfish.dev.java.net
Sent: Wednesday, January 14, 2009 2:14:26 PM
Subject: Re: Jsr199JavaCompiler possible memory leak?

No worries, glad to help.
[Message sent by forum member 'finbarro' (finbarro)]

http://forums.java.net/jive/thread.jspa?messageID=326074

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@glassfish.dev.java.net
For additional commands, e-mail: users-help@glassfish.dev.java.net

[att1.html]

Jan Luehe

Hi Dominic,

On 01/14/09 16:01, Dominic McGinnis wrote:
> On this note is the JSR199 in-memory compiler used as the default
> compiler for v2ur2 using JDK 1.6?
Yes.
> From my understanding that appears to be the case, but I'm having
> trouble finding any documentation that states this as a fact.
>
> Also if so, how does one bypass this and fall back to using the non
> in-memory compiler?

I don't think there is any way to fall back to the ant-based compiler
when using JDK 1.6,
but I will let Kin-man confirm.

Thanks!

Jan

>
> Thanks,
> Dominic
>
> ------------------------------------------------------------------------
> *From:* "glassfish@javadesktop.org"
> *To:* users@glassfish.dev.java.net
> *Sent:* Wednesday, January 14, 2009 2:14:26 PM
> *Subject:* Re: Jsr199JavaCompiler possible memory leak?
>
> No worries, glad to help.
> [Message sent by forum member 'finbarro' (finbarro)]
>
> http://forums.java.net/jive/thread.jspa?messageID=326074
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@glassfish.dev.java.net
>
> For additional commands, e-mail: users-help@glassfish.dev.java.net
>
>
>

[att1.html]

kchung
Offline
Joined: 2004-05-06

No, there is no option to use ant compiler with JDK1.6. Maybe one would be useful?

Of course if you precompiled your JSP pages, you won't run into this problem. :-)