[
https://issues.apache.org/jira/browse/OPENNLP-421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17796698#comment-17796698
]
ASF GitHub Bot commented on OPENNLP-421:
----------------------------------------
rzo1 opened a new pull request, #568:
URL: https://github.com/apache/opennlp/pull/568
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA
number you are trying to resolve? Pay particular attention to the hyphen "-"
character.
- [x] Has your PR been rebased against the latest commit within the target
branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn
clean install at the root opennlp folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [x] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main
LICENSE file in opennlp folder?
- [ ] If applicable, have you updated the NOTICE file, including the main
NOTICE file found in opennlp folder?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which
it is rendered?
### Note:
I found
[OPENNLP-421](https://issues.apache.org/jira/projects/OPENNLP/issues/OPENNLP-421)
in our issue tracker and added a simple JMH benchmark to look into the impact
of interning strings and our multiple copy-operations in `StringList`.
- Here is a JMH Benchmark for the vanilla version of OpenNLP:
https://gist.github.com/rzo1/d2ba3e48c6bc190977baf9ee42388823
- Here is a JMH Benchmark with String interning removed from `StringList`
and usage of `System.arraycopy(...)`:
https://gist.github.com/rzo1/327c84ebac18b62baf927f1a87ec7480
As stated by the original issue opener, String interning was most likely
used because of:
> 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:
https://www.oracle.com/java/technologies/javase/vmoptions-jsp.html)
So if we have huge `StringLists` or `Dictionaries`, users need to increase
`-XX:MaxPermSize=` option to avoid an OutOfMemoryException.
There might also be room for improvement in the `Dictionary` implemention as
we mess around with `StringLists`, `StringListWrappers`, etc. - but that would
be something to look into next.
Just want to have some thoughts on the interning removal here ;-)
> 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)
