[ 
https://issues.apache.org/jira/browse/OPENNLP-421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17797054#comment-17797054
 ] 

ASF GitHub Bot commented on OPENNLP-421:
----------------------------------------

rzo1 commented on PR #568:
URL: https://github.com/apache/opennlp/pull/568#issuecomment-1857359451

   @kinow thanks for the details and the flight recording :-) 
   
   I think, that the current use of String interning was an attempt to optimize 
string comparisons by using the string constant pool (until it explodes). I 
guess, that the idea was also to save memory.
   
   There is a really nice blog article which summarizes the pros/cons: 
https://www.codecentric.de/wissens-hub/blog/save-memory-by-using-string-intern-in-java
   
   The state, that 
   
   > Only use String.intern() on Strings you know are occurring multiple times, 
and only do it to save memory
   
   I think, that we can remove a lot of the current object creation related to 
the use of `StringList`, `StringListWrapper` or the creation of `StringList` 
objects just for the sake of running a `contains(...)`, which should make up 
for the overhead due to the removal of String interning. 
   




> Large dictionaries cause JVM OutOfMemoryError: PermGen due to String interning
> ------------------------------------------------------------------------------
>
>                 Key: OPENNLP-421
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-421
>             Project: OpenNLP
>          Issue Type: Bug
>          Components: Name Finder
>    Affects Versions: tools-1.5.2-incubating
>         Environment: RedHat 5, JDK 1.6.0_29
>            Reporter: Jay Hacker
>            Assignee: Richard Zowalla
>            Priority: Minor
>              Labels: performance
>   Original Estimate: 168h
>  Remaining Estimate: 168h
>
> The current implementation of StringList:
> https://svn.apache.org/viewvc/incubator/opennlp/branches/opennlp-1.5.2-incubating/opennlp-tools/src/main/java/opennlp/tools/util/StringList.java?view=markup
>  
> calls intern() on every String.  Presumably this is an attempt to reduce 
> memory usage for duplicate tokens.  Interned Strings are stored in the JVM's 
> permanent generation, which has a small fixed size (seems to be about 83 MB 
> on modern 64-bit JVMs: 
> [http://www.oracle.com/technetwork/java/javase/tech/vmoptions-jsp-140102.html]).
>   Once this fills up, the JVM crashes with an OutOfMemoryError: PermGen 
> space.  
> The size of the PermGen can be increased with the -XX:MaxPermSize= option to 
> the JVM.  However, this option is non-standard and not well known, and it 
> would be nice if OpenNLP worked out of the box without deep JVM tuning.
> This immediate problem could be fixed by simply not interning Strings.  
> Looking at the Dictionary and DictionaryNameFinder code as a whole, however, 
> there is a huge amount of room for performance improvement.  Currently, 
> DictionaryNameFinder.find works something like this:
> for every token in every tokenlist in the dictionary:
>     copy it into a "meta dictionary" of single tokens
> for every possible subsequence of tokens in the sentence:        // of which 
> there are O(N^2)
>     copy the sequence into a new array
>     if the last token is in the "meta dictionary":
>         make a StringList from the tokens
>         look it up in the dictionary
> Dictionary itself is very heavyweight: it's a Set<StringListWrapper>, which 
> wraps StringList, which wraps Array<String>.  Every entry in the dictionary 
> requires at least four allocated objects (in addition to the Strings): Array, 
> StringList, StringListWrapper, and HashMap.Entry.  Even contains and remove 
> allocate new objects!
> From this comment in DictionaryNameFinder:
>         // TODO: improve performance here
> It seems like improvements would be welcome.  :)  Removing some of the object 
> overhead would more than make up for interning strings.  Should I create a 
> new Jira ticket to propose a more efficient design?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to