[GitHub] [accumulo] brianloss commented on pull request #1920: Use String.intern() instead of WeakHashMap

[GitBox <gi...@apache.org>]

brianloss commented on pull request #1920:
URL: https://github.com/apache/accumulo/pull/1920#issuecomment-776691790


   In the [discussion thread](https://lists.apache.org/thread.html/r716826039c63640d7b1d983acc60b8c9116ade3e7f68fac39913144c%40%3Cdev.accumulo.apache.org%3E) about this, the Guava code was cited since they recommend against String.intern. I looked at the git history, and the documentation of that recommendation is only 4 months old, so I started looking around for any performance comparisons, and I couldn't find anything recent. 
   
   I took a gist from a [stackoverflow post](https://stackoverflow.com/questions/10624232/performance-penalty-of-string-intern) discussing String.intern performance, and ran it on JDK 11 on a MacBook Pro laptop. While it's probably not the best benchmark, I found that even in JDK 11, String.intern can be significantly slower than a ConcurrentHashMap. It was mostly bad when I tried with a large number of interned strings. For a small number of interned strings, String.intern performance was comparable to ConcurrentHashMap, though lookup (calling intern on an already interned string) was always slower for String.intern. For large numbers of Strings, I was able to get improvements by setting the `-XX:StringTableSize` argument to the VM to increase the table from its JDK11 default of 65536 (JDK 7-10 defaulted to 60013).
   
   I think if you aren't exceeding the string table size, then String.intern performance is probably comparable enough. But if you exceed, then it starts to degrade quickly. I'm not suggesting to skip this change, but it's just something to think about.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org