[
https://issues.apache.org/jira/browse/LOG4J2-1434?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15337187#comment-15337187
]
Luke Butters commented on LOG4J2-1434:
--------------------------------------
Note that my assumption is we want to remember to call {{close()}} But I guess
we don't already do that and is not the worst if we forget to call close() (at
least in the areas I don't care about :D). If not having garbage is more
important we could instead change {{ThreaLocalStringBuilderProvider}} to have a
field which is a {{StringBuilder}} not supplied by a {{ThreadLocal}} and place
{{ThreaLocalStringBuilderProvider}} in the thread local. So we have:
{code}
ThreadLocal<ThreaLocalStringBuilderProvider> threadLocal = new ThreadLocal<>();
{code}
our client code now looks like:
{code}
@Override
public byte[] toByteArray(final LogEvent event) {
//We get the string builder provider from a static method which gets it
from the ThreadLocal.
try(ThreadLocalStringBuilderProvider stringBuilderProvider =
SomeClass.getThreadLocalStringBuilderProvider()) {
StringBuilder sb = stringBuilderProvider.getStringBuilder();
final StringBuilder text = toText(event, sb, false);
final byte[] bytes = getBytes(text.toString());
return compressionType != CompressionType.OFF && bytes.length >
compressionThreshold ? compress(bytes) : bytes;
}
}
{code}
For small enough log messages (because we have to create a new StringBuilder if
the log message is large) we would not be creating new objects, so nothing
would need to be collected.
> 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
>
> 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
