[ 
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)

Reply via email to