[
https://issues.apache.org/jira/browse/LOG4J2-1434?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15339227#comment-15339227
]
Luke Butters commented on LOG4J2-1434:
--------------------------------------
Ah ok lets keep JDK classes in ThreadLocals.
One thing I do like about a Provider which is closable, is it is the only way
to get the StringBuilder (from the thread local). With the understanding that
this is getting complicated what we can could do is:
{code}
interface SingleThreadStringBuilderProvider extends Closeable {
/*
* Gets the string builder for creating a log4j message.
* Implementations may assume that a single thread will call this
* at most once, before calling close().
*/
public StringBuilder getStringBuilder();
public void close();
}
{code}
Inside of our AbstractStringLayout we implement the only implementation and
provide a protect getter method for our sub classes.
{code}
private static final ThreadLocal<StringBuilder> threadLocal = new
ThreadLocal<>();
private static final SingleThreadStringBuilderProvider
STRING_BUILDER_PROVIDER = new SingleThreadStringBuilderProvider() {
@Override
public StringBuilder getStringBuilder() {
return staticMehtodWhichGetsStringBuilder();
}
@Override
public void close() {
//If the StringBuilder is too long then shrink it down here.
}
};
protected SingleThreadStringBuilderProvider
getStringBuilderProviderThatIWillClose() {
//Return the static instance.
return STRING_BUILDER_PROVIDER;
}
{code}
Our client code which extends the Abstract class now looks like this:
{code}
public void logSomething(String s) {
try(SingleThreadStringBuilderProvider provider =
this.getStringBuilderProviderThatIWillClose()) {
StringBuilder sb = provider.getStringBuilder();
nowGoLog(sb.append(s).toString());
}
}
{code}
The nice thing about this is:
* we can use a more complicated StringBuilder provider later on, without
changing our child classes.
* we can't get the special StringBuilder unless we ask for out out of the thing
which must be closed.
* we don't create any classes.
* In our testing we can mock out the StringBuilderProvider easily.
* it solves the original problem.
> StringBuffer in ThreadLocal can cause excessive memory usage after large log
> messages
> -------------------------------------------------------------------------------------
>
> Key: LOG4J2-1434
> URL: https://issues.apache.org/jira/browse/LOG4J2-1434
> Project: Log4j 2
> Issue Type: Bug
> Affects Versions: 2.6.1
> Reporter: Luke Butters
> Assignee: Remko Popma
> Fix For: 2.6.2
>
>
> In an effort to speed up logging ThreadLocals have been introduced see
> LOG4J2-1125 however this does causes memory issues.
> The problem of the ThreadLocal occurs when threads are re-used which is an
> absolutely valid way of using java. For example an executor service can
> re-use threads as well as Jetty.
> Below I demonstrate a contrived example of the memory leak:
> {code}
> int stringSize = 1024*1024*10; //~10MB maybe 20MB for UTF-16
> StringBuilder sb = new StringBuilder(stringSize);
> for(int i = 0; i < stringSize; i++) {
> sb.append('a' + i % 5);
> }
>
> String largeString = sb.toString();
>
> sb = null; //Let it be GC'ed
> ExecutorService es = Executors.newFixedThreadPool(100);
> final CountDownLatch countDownLatch = new CountDownLatch(100);
> for(int i = 0; i < 100; i++) {
> es.execute(()-> {
> //Log the big string to demonstrate the issue.
> log.fatal(largeString);
>
> //Ensure we use all 100 of our threads by not releasing this
> thread yet.
> countDownLatch.countDown();
> });
>
> //We sleep for 2s so we more easily watch memory growth
> Thread.sleep(2000);
> }
> {code}
> I recommend that log4j2 immediately remove the ThreadLocal as a small gain in
> performance does not outweigh the problems associated with memory leaks.
> Finally other options for caching the StringBuilder with a ThreadLocal could
> be considered for example we might re-use StringBuilders that are no larger
> than 3k while removing the ones which are larger than 3k.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-dev-h...@logging.apache.org
