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

[Nicolas Malin (Jira)]

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

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

I just looked over CompilerMatcher.java and the Jira issue that created it https://issues.apache.org/jira/browse/OFBIZ-4107 and it is clear to me the whole approach is wrong.

1. Mini-language was sharing non-thread-safe objects between threads. The solution is to change it so the objects are not shared between threads. There is a patch in the Jira issue that makes that change.

2. Another patch was submitted a month later that introduces the CompilerMatcher.java class. That class provides the same benefit as the first one, but in an overly-complicated way. You are still creating new compiler instances per thread, but you're doing it in a Rube Goldberg way.

3. The new class caches compiled RegEx patterns. That could be implemented separately (PatternFactory.java perhaps) and thereby eliminate all of the clunky code in the rest of the class.

4. So, to do the same thing without CompilerMatcher.java, create a local instance of Perl5Matcher, get the Pattern from PatternFactory, and do your thing. With CompilerMatcher.java gone, the ThreadLocal issue goes away.

> 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@...%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)