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

Jacques Le Roux commented on OFBIZ-5395:
----------------------------------------

I thought a bit more about it. Installing JreMemoryLeakPreventionListener in 
OFBiz OOTB will not hurt. But it will not have any impact on "leaks". For 
instance the AWT context "leak" is the only major one proved so far. As long as 
we stay in the embedded Tomcat context, this is not a leak. Just a block of 
memory preempted by a third party lib used in OFBiz (certainly 
javax.imageio.ImageIO, maybe others). What will do 
JreMemoryLeakPreventionListener in this case is allocating the same block in 
its common class loader memory, rather than in one of the pool threads. Most of 
what JreMemoryLeakPreventionListener is working against are the same no real 
leaks OOTB. As soon as you stop OFBiz they are gone.

So we can install it OOTB: possible warnings in future could help. It's when 
using Tomcat as an external server that it really adds something. So I will 
first document that in the wiki page as said above.

This is not only related with the CompilerMatcher class.

Adrian, about your points above.  I refrained myself to revert the 2nd patch 
and to use the 1st one idea (extended to other concerned classes) not only 
because of the pattern cache. Also because it seems to me that allocating 
classes locally is more expansive (time) than reusing a static variable created 
once per thread. It's certainly is, but I have no ideas about the order of 
magnitude. This said the major reason I did not commit a patch I have ready, 
was indeed the cache. So, if you provide PatternFactory.java, we could go that 
way. 

Note that this does not solve the issue with the 2 other cases, I mean 
ThreadLocal<RollbackOnlyCause> and ThreadLocal<List<RollbackOnlyCause>>. Also 
though Tomcat users can rely on [JRE Memory Leak Prevention 
Listener|https://tomcat.apache.org/tomcat-7.0-doc/config/listeners.html#JRE_Memory_Leak_Prevention_Listener_-_org.apache.catalina.core.JreMemoryLeakPreventionListener],
 I don't know if users of other external servers have such a mechanism 
available.

Anyway, we can note that nobody never complained. The reason is that, as far as 
I could measure, the ThreadLocal leaks are very tiny (<500KB for the whole). So 
it would need a very long time and many redeploy cycles before they appear. 

It's not the same for the AWT context leak. I saw memory blocks of 10 to 15MB 
in almost all instances I checked on trunk demo. I'm curious why nobody 
complained about that... If javax.imageio.ImageIO, and only it, is the reason 
we could use [this 
cure|http://stackoverflow.com/questions/12357969/how-to-resolve-outofmemoryerror-with-imageio-plugins-as-the-cause].

> 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/%[email protected]%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
> # *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 (Note: this can only be done in OFBiz internally, for 
> app server the best is to document with a section in the [related wiki 
> page|https://cwiki.apache.org/confluence/display/OFBTECH/Run+OFBiz+under+outside+Application+Servers].
> ** Cons: none, this should had any noticeable effects when OFBiz is used OOTB 
> (Tomcat embedded)
> 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 (certainly 
> something like explainded in page 14 of the link from "History (29 pages 
> presentation)" above)
> I'm also considering to replace the current uses of java.util.regex.Pattern 
> by CompilerMatcher in cases of a static pattern is used. When the 
> CompilerMatcher cache makes sense (patterns do not change). The interest is 
> the better performance of the oro framework, at least so far (Java 1.6, see 
> below).
> 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)

Reply via email to