[jira] [Commented] (OFBIZ-5395) Introduce Tomcat's JreMemoryLeakPreventionListener and why

["Adrian Crum (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OFBIZ-5395?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13824011#comment-13824011 ] 

Adrian Crum commented on OFBIZ-5395:
------------------------------------

[{quote} I checked OFBiz code. There is only 2 custom classes concerned{quote} I don't understand. There are many places where OFBiz uses ThreadLocal variables.

Another thing I don't understand: You start off pointing out how ThreadLocal variables can cause memory leaks, then you add a new ThreadLocal variable in your patch. Isn't that defeating your goal?

Personally, I never cared for ThreadLocal variables. They appear hackish and they indicate a poor OO design.

It seems to me you are trying to solve a pattern matching problem. It might help to focus on that first.


> Introduce Tomcat's JreMemoryLeakPreventionListener and why
> ----------------------------------------------------------
>
>                 Key: OFBIZ-5395
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-5395
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Jacques Le Roux
>            Assignee: Jacques Le Roux
>              Labels: JreMemoryLeakPreventionListener, ThreadLocal, leak
>         Attachments: OFBIZ-5395-CompilerMatcher.patch
>
>
> After reading few articles on possible memory leaks when using ThreadLocal variable with custom classes in application server context where a thread pool is used, I checked OFBiz code. There is only 2 custom classes  concerned: CompilerMatcher and RollbackOnlyCause (JDK classes are not concerned by ThreadLocal memory leaks).
> First I must tell, that the memory leak problem is more clearly described in those articles when you use an external Application Server (like Tomcat) and you deploy/undeploy applications. It seems there are no major issues when you use OFBiz OOTB (with Tomcat embedded).  Nevertheless, it's a concern by and large.
> I have not investigated RollbackOnlyCause, only CompilerMatcher. But, after some profiling, I believe both should only generate small amouts of memory leaks, almost not noticeable even after several deploy/undeploy cycles.
> Nevertheless I tried to find a good way to get rid of CompilerMatcher possible leaks. I thought about 3 ways:
> # *Reverts [CompilerMatcher related changes|http://svn.apache.org/viewvc?view=revision&revision=1075322]* done for OFBIZ-4107 (introduction of CompilerMatcher) by using Perl5Compiler.READ_ONLY_MASK which guarantees thread safety
> ** Pros: no need to introduce ThreadLocal
> ** Cons: performance, local Perl5 variables creation removes the patterns-compiled-cache CompilerMatcher introduced (note: [I found the origin of CompilerMatcher class here|http://mail-archives.apache.org/mod_mbox/jmeter-user/200212.mbox/%3Cse0ae21c.089@katun.com%3E])
> # *Uses ThreadLocal<CompilerMatcher> local variables* instead of private static members
> ** Pros: no need to worry about thread safety
> ** Cons: performance, local ThreadLocal local variables creation removes the patterns-compiled-cache ThreadLocal<CompilerMatcher> offers  when used as a private static member
> # *Uses ThreadLocal<CompilerMatcher> local variables* instead of private static members
> ** Pros: no need to worry about thread safety
> ** Cons: performance, local ThreadLocal local variables creation removes the patterns-compiled-cache ThreadLocal<CompilerMatcher> gives when used as a private static member
> # *Introduces [Tomcat's JreMemoryLeakPreventionListener|http://wiki.apache.org/tomcat/MemoryLeakProtection].* [What it does (in less than a minute)?|http://stackoverflow.com/questions/14882794/what-does-tomcats-threadlocalleakpreventionlistener-do-exactly] [Why JreMemoryLeakPreventionListener was created?|http://www.tomcatexpert.com/blog/2010/04/06/tomcats-new-memory-leak-prevention-and-detection] [History (29 pages presentation).|http://people.apache.org/~markt/presentations/2010-11-04-Memory-Leaks-60mins.pdf]  How it does it? [Read code!|http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java?view=annotate]
> ** Pros: 
> *** no changes related to CompilerMatcher, performance enhancement the cache introduces kept
> *** prevents memory leaks when an external Application Server is used or at least warn about them
> ** Cons: none, this should had any noticeable effects when OFBiz is used OOTB (Tomcat embedded)
> So of course I decided to go with the JreMemoryLeakPreventionListener solution. Another reason for that is that when I profiled OFBiz trunk using the demo site I found that we were having a large bloc of memory retained by [sun.awt.AppContext|http://www.docjar.com/docs/api/sun/awt/AppContext.html].  I think we should not have such a thing, the web truk demo does not use AWT at all! Fortunately jreMemoryLeakPreventionListener.setAppContextProtection prevents this, even if I have still no ideas from where this comes.
> I'm also considering to replace the current uses of java.util.regex.Pattern by CompilerMatcher in cases of a static pattern is used. Then the CompilerMatcher cache makes sense.
> Some interesting references I noted while analysing this issue:
> * [Oro is 6 times faster than regular Java regex|http://www.tusker.org/regex/regex_benchmark.html]. So, with its cache, CompilerMatcher is more than an interesting alternative to regular Java regex.
> * Java regex Javadoc: [Compiler|https://docs.oracle.com/javase/6/docs/api/java/util/regex/Pattern.html], [Matcher|https://docs.oracle.com/javase/6/docs/api/java/util/regex/Matcher.html]
> * Oro Javadoc: [Compiler|https://jakarta.apache.org/oro/api/org/apache/oro/text/regex/Perl5Compiler.html], [Matcher|https://jakarta.apache.org/oro/api/org/apache/oro/text/regex/Perl5Matcher.html]



--
This message was sent by Atlassian JIRA
(v6.1#6144)