[
https://issues.apache.org/jira/browse/OPENNLP-1168?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16298247#comment-16298247
]
ASF GitHub Bot commented on OPENNLP-1168:
-----------------------------------------
kottmann closed pull request #296: OPENNLP-1168: Resolved concurrency issue in
POS tagger.
URL: https://github.com/apache/opennlp/pull/296
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/opennlp-tools/src/main/java/opennlp/tools/postag/DefaultPOSContextGenerator.java
b/opennlp-tools/src/main/java/opennlp/tools/postag/DefaultPOSContextGenerator.java
index 3035ca523..3f4fe97ed 100644
---
a/opennlp-tools/src/main/java/opennlp/tools/postag/DefaultPOSContextGenerator.java
+++
b/opennlp-tools/src/main/java/opennlp/tools/postag/DefaultPOSContextGenerator.java
@@ -43,7 +43,6 @@
private Object wordsKey;
private Dictionary dict;
- private String[] dictGram;
/**
* Initializes the current instance.
@@ -62,7 +61,7 @@ public DefaultPOSContextGenerator(Dictionary dict) {
*/
public DefaultPOSContextGenerator(int cacheSize, Dictionary dict) {
this.dict = dict;
- dictGram = new String[1];
+
if (cacheSize > 0) {
contextsCache = new Cache<>(cacheSize);
}
@@ -148,8 +147,8 @@ public DefaultPOSContextGenerator(int cacheSize, Dictionary
dict) {
e.add("default");
// add the word itself
e.add("w=" + lex);
- dictGram[0] = lex;
- if (dict == null || !dict.contains(new StringList(dictGram))) {
+
+ if (dict == null || !dict.contains(new StringList(lex))) {
// do some basic suffix analysis
String[] suffs = getSuffixes(lex);
for (int i = 0; i < suffs.length; i++) {
diff --git a/opennlp-tools/src/main/java/opennlp/tools/postag/POSTaggerME.java
b/opennlp-tools/src/main/java/opennlp/tools/postag/POSTaggerME.java
index 1edcf4b5b..4801bbf40 100644
--- a/opennlp-tools/src/main/java/opennlp/tools/postag/POSTaggerME.java
+++ b/opennlp-tools/src/main/java/opennlp/tools/postag/POSTaggerME.java
@@ -222,8 +222,8 @@ public void probs(double[] probs) {
}
public static POSModel train(String languageCode,
- ObjectStream<POSSample> samples, TrainingParameters trainParams,
- POSTaggerFactory posFactory) throws IOException {
+ ObjectStream<POSSample> samples,
TrainingParameters trainParams,
+ POSTaggerFactory posFactory) throws IOException
{
int beamSize = trainParams.getIntParameter(BeamSearch.BEAM_SIZE_PARAMETER,
POSTaggerME.DEFAULT_BEAM_SIZE);
@@ -288,7 +288,7 @@ public static Dictionary
buildNGramDictionary(ObjectStream<POSSample> samples, i
}
public static void populatePOSDictionary(ObjectStream<POSSample> samples,
- MutableTagDictionary dict, int cutoff) throws IOException {
+ MutableTagDictionary dict, int
cutoff) throws IOException {
System.out.println("Expanding POS Dictionary ...");
long start = System.nanoTime();
diff --git
a/opennlp-tools/src/test/java/opennlp/tools/postag/DefaultPOSContextGeneratorTest.java
b/opennlp-tools/src/test/java/opennlp/tools/postag/DefaultPOSContextGeneratorTest.java
new file mode 100644
index 000000000..450bb2cc3
--- /dev/null
+++
b/opennlp-tools/src/test/java/opennlp/tools/postag/DefaultPOSContextGeneratorTest.java
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.postag;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import opennlp.tools.dictionary.Dictionary;
+import opennlp.tools.util.StringList;
+
+/**
+ *
+ * We encountered a concurrency issue in the pos tagger module in the class
+ * DefaultPOSContextGenerator.
+
+ The issue is demonstrated in DefaultPOSContextGeneratorTest.java. The test
"multithreading()"
+ consistently fails on our system with the current code if the number of
threads
+ (NUMBER_OF_THREADS) is set to 10. If the number of threads is set to 1
(effectively disabling
+ multithreading), the test consistently passes.
+
+ We resolved the issue by removing a field in DefaultPOSContextGenerator.java.
+ *
+ */
+
+
+public class DefaultPOSContextGeneratorTest {
+
+
+ public static final int NUMBER_OF_THREADS = 10;
+ private static Object[] tokens;
+ private static DefaultPOSContextGenerator defaultPOSContextGenerator;
+ private static String[] tags;
+
+ @BeforeClass
+ public static void setUp() {
+ final String matchingToken = "tokenC";
+
+ tokens = new Object[] {"tokenA", "tokenB", matchingToken, "tokenD"};
+
+ final StringList stringList = new StringList(new String[] {matchingToken});
+
+ Dictionary dictionary = new Dictionary();
+ dictionary.put(stringList);
+
+ defaultPOSContextGenerator = new DefaultPOSContextGenerator(dictionary);
+
+ tags = new String[] {"tagA", "tagB", "tagC", "tagD"};
+ }
+
+ @Test
+ public void noDictionaryMatch() {
+ int index = 1;
+
+ final String[] actual = defaultPOSContextGenerator.getContext(index,
tokens, tags);
+
+ final String[] expected = new String[] {
+ "default",
+ "w=tokenB",
+ "suf=B",
+ "suf=nB",
+ "suf=enB",
+ "suf=kenB",
+ "pre=t",
+ "pre=to",
+ "pre=tok",
+ "pre=toke",
+ "c",
+ "p=tokenA",
+ "t=tagA",
+ "pp=*SB*",
+ "n=tokenC",
+ "nn=tokenD"
+ };
+
+ Assert.assertArrayEquals("Calling with not matching index at: " + index +
+ "\nexpected \n" + Arrays.toString(expected) + " but actually was \n"
+ + Arrays.toString(actual), expected, actual);
+ }
+
+ @Test
+ public void dictionaryMatch() {
+ int indexWithDictionaryMatch = 2;
+
+ final String[] actual =
+ defaultPOSContextGenerator.getContext(indexWithDictionaryMatch,
tokens, tags);
+
+ final String[] expected = new String[] {
+ "default",
+ "w=tokenC",
+ "p=tokenB",
+ "t=tagB",
+ "pp=tokenA",
+ "t2=tagA,tagB",
+ "n=tokenD",
+ "nn=*SE*"
+ };
+
+ Assert.assertArrayEquals("Calling with index matching dictionary entry at:
"
+ + indexWithDictionaryMatch + "\nexpected \n" +
Arrays.toString(expected)
+ + " but actually was \n" + Arrays.toString(actual), expected, actual);
+ }
+
+ @Test
+ public void multithreading() {
+ Callable<Void> matching = () -> {
+
+ dictionaryMatch();
+
+ return null;
+ };
+
+ Callable<Void> notMatching = () -> {
+
+ noDictionaryMatch();
+
+ return null;
+ };
+
+ final List<Callable<Void>> callables = IntStream.range(0, 200000)
+ .mapToObj(index -> (index % 2 == 0) ? matching : notMatching)
+ .collect(Collectors.toList());
+
+ final ExecutorService executorService =
Executors.newFixedThreadPool(NUMBER_OF_THREADS);
+
+ try {
+ final List<Future<Void>> futures = executorService.invokeAll(callables);
+
+ executorService.shutdown();
+ executorService.awaitTermination(30, TimeUnit.SECONDS);
+
+ futures.forEach(future -> {
+
+ try {
+ future.get();
+ } catch (InterruptedException e) {
+ Assert.fail("Interrupted because of: " + e.getCause().getMessage());
+ } catch (ExecutionException ee) {
+ Assert.fail(ee.getCause().getMessage());
+ }
+
+ });
+ } catch (final InterruptedException e) {
+ Assert.fail("Test interrupted");
+ }
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Resolved concurrency issue in POS tagger.
> -----------------------------------------
>
> Key: OPENNLP-1168
> URL: https://issues.apache.org/jira/browse/OPENNLP-1168
> Project: OpenNLP
> Issue Type: Improvement
> Components: POS Tagger
> Affects Versions: 1.8.4
> Reporter: Niels Schuette
> Labels: easyfix, patch
> Fix For: 1.8.4
>
>
> We encountered a concurrency issue in the pos tagger module in the class
> DefaultPOSContextGenerator.
> The issue is demonstrated in DefaultPOSContextGeneratorTest.java. The test
> "multithreading()" consistently fails on our system with the current code if
> the number of threads (NUMBER_OF_THREADS) is set to 10. If the number of
> threads is set to 1 (effectively disabling multithreading), the test
> consistently passes.
> We resolved the issue by removing a field in DefaultPOSContextGenerator.java.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
