[ https://issues.apache.org/jira/browse/JOHNZON-293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16987940#comment-16987940 ]
Alexander Falb commented on JOHNZON-293: ---------------------------------------- I did a bit of debugging and profiling the HotSpot JVM 1.8_172. Romain, you were right. The ThreadLocalMap uses the ThreadLocal itself als WeakReference key. When the Reference to this ThreadLocal is garbage collected, the WeakReference becomes garbage collectable and ultimately removed based on some removal heuristics within ThreadLocalMap#set / ThreadLocal.ThreadLocalMap#cleanSomeSlots. Because the value (SimpleDateFormat in our case) is stored as Object field in WeakReference, it also gets garbage collected. Nevertheless, I'll have a look at porting the Converter over to use java.time.* internally. > Potential Memleak in DateConverter > ---------------------------------- > > Key: JOHNZON-293 > URL: https://issues.apache.org/jira/browse/JOHNZON-293 > Project: Johnzon > Issue Type: Bug > Components: Mapper > Reporter: Alexander Falb > Priority: Major > > [DateConverter|https://github.com/apache/johnzon/blob/master/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/converter/DateConverter.java] > creates a new SimpleDateFormat for every instance for every thread the > instance is used on and never cleans them up. This may cause a high memory > usage, if lots of those converters are created instead of reused. > > Two approaches to get rid of them are on my mind: > # Create a new SimpleDateFormat within the toString and fromString methods > as method variable instead of a class field. > # Use java.time.DateTimeFormatter as class field, because it is immutable > and thread-safe, and do some object conversion from java.util.Date to/from > java.time.ZonedDateTime. > > I did some JMH performance tests for both (full project can be found here > [https://github.com/elexx/dateconverter-benchmark]): > {noformat} > Benchmark Mode Cnt Score > Error Units > JavaTimeDateFormatterBenchmark.formatNewFormat avgt 5 828,623 ± > 8,836 ns/op > JavaTimeDateFormatterBenchmark.formatReuseFormat avgt 5 496,916 ± > 5,150 ns/op > JavaTimeDateFormatterBenchmark.parseNewFormat avgt 5 1430,276 ± > 11,084 ns/op > JavaTimeDateFormatterBenchmark.parseReuseFormat avgt 5 990,648 ± > 280,983 ns/op > SimpleDateFormatterBenchmark.formatNewFormat avgt 5 1308,144 ± > 13,993 ns/op > SimpleDateFormatterBenchmark.formatReuseFormat avgt 5 392,236 ± > 3,219 ns/op > SimpleDateFormatterBenchmark.parseNewFormat avgt 5 1848,772 ± > 19,412 ns/op > SimpleDateFormatterBenchmark.parseReuseFormat avgt 5 1121,955 ± > 12,417 ns/op > {noformat} > In this quick test it looks like creating a new SimpleDateFormatter in each > method is quite slow (1308ns/op + 1848ns/op). > Reusing the SimpleDateFormatter is faster (392ns/op + 1121ns/op), but no > option because it is not thread-safe. > Reusing the Java8-DateTimeFormatter is equivalent (496ns/op + 990ns/op) to > Reusing SimpleDateFormatter (parsing is faster, formatting is slower, avg is > about the same) > And just for completeness: Creating a Java8-DateTimeFormatter, which is > nonsense, because it is immutable and thread-safe. -- This message was sent by Atlassian Jira (v8.3.4#803005)