mawiesne commented on code in PR #1003:
URL: https://github.com/apache/opennlp/pull/1003#discussion_r3055548848
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/lemmatizer/LemmatizerME.java:
##########
@@ -123,8 +132,12 @@ public List<List<String>> lemmatize(List<String> toks,
List<String> tags) {
* @return An array of possible lemma classes for each token in {@code toks}.
*/
public String[] predictSES(String[] toks, String[] tags) {
- bestSequence = model.bestSequence(toks, new Object[] {tags},
contextGenerator, sequenceValidator);
- List<String> ses = bestSequence.getOutcomes();
+ Sequence seq = model.bestSequence(toks, new Object[] {tags},
contextGenerator, sequenceValidator);
+ this.bestSequence = seq; // volatile write for backward-compatible probs()
access
Review Comment:
This assignment c/should be done after the subsequent null check, that is,
after line 139.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/BeamSearch.java:
##########
@@ -63,92 +84,82 @@ public BeamSearch(int size, MaxentModel model) {
}
/**
- * Initializes a {@link BeamSearch} instance.
+ * Initializes a {@link BeamSearch} instance with an optional per-thread
contexts cache.
*
* @param size The size of the beam (k).
* @param model The {@link MaxentModel} for assigning probabilities to the
sequence outcomes.
- * @param cacheSize The capacity of the {@link Cache} to use.
+ * @param cacheSize The capacity of the per-thread contexts cache. Use
{@code 0} to disable caching.
*/
public BeamSearch(int size, MaxentModel model, int cacheSize) {
this.size = size;
this.model = model;
-
- if (cacheSize > 0) {
- contextsCache = new Cache<>(cacheSize);
- }
-
- this.probs = new double[model.getNumOutcomes()];
+ this.cacheSize = cacheSize;
+ this.threadState = ThreadLocal.withInitial(
+ () -> new CacheState(model.getNumOutcomes(), cacheSize));
}
- /**
- * Computes the best sequence of outcomes based on the {@link MaxentModel}.
- *
- * @param numSequences The number of sequences.
- * @param sequence The input {@link T} sequence.
- * @param additionalContext An {@link Object[]} of additional context.
- * This is passed to the context generator blindly with the
- * assumption that the context are appropriate.
- * @param minSequenceScore The minimum sequence score to use.
- * @param cg The {@link BeamSearchContextGenerator context generator} to use.
- * @param validator The {@link SequenceValidator} to validate sequences.
- *
- * @return The top ranked {@link Sequence} of outcomes or {@code null}
- * if no sequence could be found.
- */
@Override
- public <T> Sequence[] bestSequences(int numSequences, T[] sequence,
- Object[] additionalContext, double minSequenceScore,
- BeamSearchContextGenerator<T> cg, SequenceValidator<T> validator) {
+ public <T> Sequence[] bestSequences(final int numSequences, final T[]
sequence,
Review Comment:
Why has the previous Javadoc gone missing? Pls explain or re-add it as it
was.
At least we should have {@inheritDoc} placed here.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/namefind/NameFinderME.java:
##########
@@ -112,9 +121,14 @@ public Span[] find(String[] tokens) {
public Span[] find(String[] tokens, String[][] additionalContext) {
additionalContextFeatureGenerator.setCurrentContext(additionalContext);
- bestSequence = model.bestSequence(tokens, additionalContext,
contextGenerator, sequenceValidator);
+ Sequence seq = model.bestSequence(tokens,
+ additionalContext, contextGenerator, sequenceValidator);
+ this.bestSequence = seq;
Review Comment:
This assignment c/should be done after the subsequent null check, that is,
after line 129.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/AdditionalContextFeatureGenerator.java:
##########
Review Comment:
See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/POSTaggerME.java:
##########
@@ -131,26 +140,45 @@ public POSTaggerME(POSModel model) {
}
/**
- * Initializes a {@link POSTaggerME} with the provided {@link POSModel
model}.
+ * Initializes a {@link POSTaggerME} with the provided
+ * {@link POSModel model}.
*
* @param model A valid {@link POSModel}.
* @param format A valid {@link POSTagFormat}.
*/
public POSTaggerME(POSModel model, POSTagFormat format) {
+ this(model, format, -1);
+ }
+
+ /**
+ * Initializes a {@link POSTaggerME} with the provided
+ * {@link POSModel model} and explicit cache configuration.
+ *
+ * @param model A valid {@link POSModel}.
+ * @param format A valid {@link POSTagFormat}.
+ * @param contextCacheSize Size of the per-thread context
+ * generator cache. Use {@code 0} to disable caching,
+ * or {@code -1} to use the default (beam size).
+ */
+ public POSTaggerME(POSModel model, POSTagFormat format,
+ int contextCacheSize) {
this.posTagFormat = format;
POSTaggerFactory factory = model.getFactory();
int beamSize = POSTaggerME.DEFAULT_BEAM_SIZE;
- String beamSizeString =
model.getManifestProperty(BeamSearch.BEAM_SIZE_PARAMETER);
+ String beamSizeString = model.getManifestProperty(
+ BeamSearch.BEAM_SIZE_PARAMETER);
if (beamSizeString != null) {
beamSize = Integer.parseInt(beamSizeString);
}
modelPackage = model;
- cg = factory.getPOSContextGenerator(beamSize);
+ int cacheSize = contextCacheSize >= 0
Review Comment:
This implementation does not adhere to the Javadoc statement on the behavior
for the caller.
The exact value of zero (`0`) should disable caching entirely. Afaics, it
does not.
The exact value of `-1` should use the default, aka beam size.
However, all other negative values, such as -2 or Integer.MIN_VALUE will
also show the behavior as with `-1`.
Please adjust/fix it to respect the claim made in the Javadoc.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -21,62 +21,90 @@
import java.util.ArrayList;
import java.util.List;
+import opennlp.tools.commons.ThreadSafe;
import opennlp.tools.util.Cache;
/**
* Caches features of the aggregated {@link AdaptiveFeatureGenerator
generators}.
+ * <p>
+ * The cache is maintained per-thread via {@link ThreadLocal}, making this
class safe for
+ * concurrent use from multiple threads. Each thread gets its own independent
cache that is
+ * cleared when a new sentence (token array) is encountered.
+ * <p>
+ * <b>Note:</b> In container environments with classloader isolation (e.g.
Jakarta EE),
+ * {@link ThreadLocal} state may pin the classloader. Ensure instances do not
outlive
+ * the application's lifecycle, or call {@link ThreadLocal#remove()} on pooled
threads.
*
* @see Cache
*/
+@ThreadSafe
public class CachedFeatureGenerator implements AdaptiveFeatureGenerator {
Review Comment:
See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/DocumentBeginFeatureGenerator.java:
##########
@@ -26,14 +26,16 @@
*/
public class DocumentBeginFeatureGenerator implements AdaptiveFeatureGenerator
{
Review Comment:
(a) Can this class be marked `@ThreadSafe` now?
(b) See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/BeamSearch.java:
##########
@@ -63,92 +84,82 @@ public BeamSearch(int size, MaxentModel model) {
}
/**
- * Initializes a {@link BeamSearch} instance.
+ * Initializes a {@link BeamSearch} instance with an optional per-thread
contexts cache.
*
* @param size The size of the beam (k).
* @param model The {@link MaxentModel} for assigning probabilities to the
sequence outcomes.
- * @param cacheSize The capacity of the {@link Cache} to use.
+ * @param cacheSize The capacity of the per-thread contexts cache. Use
{@code 0} to disable caching.
*/
public BeamSearch(int size, MaxentModel model, int cacheSize) {
this.size = size;
this.model = model;
-
- if (cacheSize > 0) {
- contextsCache = new Cache<>(cacheSize);
- }
-
- this.probs = new double[model.getNumOutcomes()];
+ this.cacheSize = cacheSize;
+ this.threadState = ThreadLocal.withInitial(
+ () -> new CacheState(model.getNumOutcomes(), cacheSize));
}
- /**
- * Computes the best sequence of outcomes based on the {@link MaxentModel}.
- *
- * @param numSequences The number of sequences.
- * @param sequence The input {@link T} sequence.
- * @param additionalContext An {@link Object[]} of additional context.
- * This is passed to the context generator blindly with the
- * assumption that the context are appropriate.
- * @param minSequenceScore The minimum sequence score to use.
- * @param cg The {@link BeamSearchContextGenerator context generator} to use.
- * @param validator The {@link SequenceValidator} to validate sequences.
- *
- * @return The top ranked {@link Sequence} of outcomes or {@code null}
- * if no sequence could be found.
- */
@Override
- public <T> Sequence[] bestSequences(int numSequences, T[] sequence,
- Object[] additionalContext, double minSequenceScore,
- BeamSearchContextGenerator<T> cg, SequenceValidator<T> validator) {
+ public <T> Sequence[] bestSequences(final int numSequences, final T[]
sequence,
+ final Object[] additionalContext, final double minSequenceScore,
+ final BeamSearchContextGenerator<T> cg, final SequenceValidator<T>
validator) {
+
+ final CacheState state = threadState.get();
Queue<Sequence> prev = new PriorityQueue<>(size);
Queue<Sequence> next = new PriorityQueue<>(size);
Queue<Sequence> tmp;
prev.add(new Sequence());
- if (additionalContext == null) {
- additionalContext = EMPTY_ADDITIONAL_CONTEXT;
+ Object[] context = additionalContext;
+ if (context == null) {
+ context = EMPTY_ADDITIONAL_CONTEXT;
}
for (int i = 0; i < sequence.length; i++) {
- int sz = StrictMath.min(size, prev.size());
+ final int sz = StrictMath.min(size, prev.size());
for (int sc = 0; prev.size() > 0 && sc < sz; sc++) {
- Sequence top = prev.remove();
- List<String> tmpOutcomes = top.getOutcomes();
- String[] outcomes = tmpOutcomes.toArray(new String[0]);
- String[] contexts = cg.getContext(i, sequence, outcomes,
additionalContext);
- double[] scores;
- if (contextsCache != null) {
- scores = contextsCache.computeIfAbsent(contexts, c -> model.eval(c,
probs));
+ final Sequence top = prev.remove();
+ final List<String> tmpOutcomes = top.getOutcomes();
+ final String[] outcomes = tmpOutcomes.toArray(new String[0]);
+ final String[] contexts = cg.getContext(i, sequence, outcomes,
context);
+ final double[] scores;
+ if (state.cache != null) {
+ scores = state.cache.computeIfAbsent(contexts, c -> {
+ double[] res = model.eval(c, state.probs);
+ double[] copy = new double[res.length];
Review Comment:
Why is this copy necessary here? Please explain and leave a (short) code
comment above this line.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/chunker/ChunkerME.java:
##########
@@ -93,8 +102,13 @@ public ChunkerME(ChunkerModel model) {
@Override
public String[] chunk(String[] toks, String[] tags) {
TokenTag[] tuples = TokenTag.create(toks, tags);
- bestSequence = model.bestSequence(tuples, new Object[] {},
contextGenerator, sequenceValidator);
- List<String> c = bestSequence.getOutcomes();
+ Sequence seq = model.bestSequence(tuples,
+ new Object[] {}, contextGenerator, sequenceValidator);
+ this.bestSequence = seq;
Review Comment:
This assignment c/should be done after the subsequent null check, that is,
after line 110.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/BeamSearch.java:
##########
@@ -63,92 +84,82 @@ public BeamSearch(int size, MaxentModel model) {
}
/**
- * Initializes a {@link BeamSearch} instance.
+ * Initializes a {@link BeamSearch} instance with an optional per-thread
contexts cache.
*
* @param size The size of the beam (k).
* @param model The {@link MaxentModel} for assigning probabilities to the
sequence outcomes.
- * @param cacheSize The capacity of the {@link Cache} to use.
+ * @param cacheSize The capacity of the per-thread contexts cache. Use
{@code 0} to disable caching.
Review Comment:
> Use {@code 0} to disable caching.
This is actually not respected in the code of the constructor. It was before
this change. Either we need to rewrite the Javadoc here or the lines below.
##########
opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/sentdetect/SentenceDetectorMEBenchmark.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.sentdetect;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+
+import opennlp.tools.formats.ResourceAsStreamFactory;
+import opennlp.tools.util.InputStreamFactory;
+import opennlp.tools.util.ObjectStream;
+import opennlp.tools.util.PlainTextByLineStream;
+import opennlp.tools.util.TrainingParameters;
+
+/**
+ * JMH benchmark for {@link SentenceDetectorME} thread-safety
+ * and instance allocation strategies.
+ */
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.SECONDS)
+@Warmup(iterations = 5, time = 2)
+@Measurement(iterations = 10, time = 2)
+@Fork(2)
+public class SentenceDetectorMEBenchmark {
Review Comment:
Please reformat the entire class to go for at least 110 characters per line,
as this is allowed per OpenNLP checkstyle rules.
##########
opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/tokenize/TokenizerMEBenchmark.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.tokenize;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+
+import opennlp.tools.formats.ResourceAsStreamFactory;
+import opennlp.tools.util.InputStreamFactory;
+import opennlp.tools.util.ObjectStream;
+import opennlp.tools.util.Parameters;
+import opennlp.tools.util.PlainTextByLineStream;
+import opennlp.tools.util.TrainingParameters;
+
+/**
+ * JMH benchmark for {@link TokenizerME} thread-safety and
+ * instance allocation strategies.
+ */
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.SECONDS)
+@Warmup(iterations = 5, time = 2)
+@Measurement(iterations = 10, time = 2)
+@Fork(2)
+public class TokenizerMEBenchmark {
Review Comment:
Please reformat the entire class to go for at least 110 characters per line,
as this is allowed per OpenNLP checkstyle rules.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/BeamSearch.java:
##########
@@ -63,92 +84,82 @@ public BeamSearch(int size, MaxentModel model) {
}
/**
- * Initializes a {@link BeamSearch} instance.
+ * Initializes a {@link BeamSearch} instance with an optional per-thread
contexts cache.
*
* @param size The size of the beam (k).
* @param model The {@link MaxentModel} for assigning probabilities to the
sequence outcomes.
- * @param cacheSize The capacity of the {@link Cache} to use.
+ * @param cacheSize The capacity of the per-thread contexts cache. Use
{@code 0} to disable caching.
*/
public BeamSearch(int size, MaxentModel model, int cacheSize) {
this.size = size;
this.model = model;
-
- if (cacheSize > 0) {
- contextsCache = new Cache<>(cacheSize);
- }
-
- this.probs = new double[model.getNumOutcomes()];
+ this.cacheSize = cacheSize;
+ this.threadState = ThreadLocal.withInitial(
Review Comment:
Should be:
```
this.threadState = cacheSize > 0 ? ThreadLocal.withInitial(
() -> new CacheState(model.getNumOutcomes(), cacheSize))
: null;
```
##########
opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/featuregen/CachedFeatureGeneratorTest.java:
##########
@@ -26,114 +26,77 @@
/**
* Test for the {@link CachedFeatureGenerator} class.
+ * <p>
+ * Caching has been removed for thread safety. These tests verify
+ * that the generator still delegates correctly to its underlying
+ * generator and that the deprecated cache stat methods return 0.
*/
public class CachedFeatureGeneratorTest {
- private final AdaptiveFeatureGenerator identityGenerator = new
IdentityFeatureGenerator();
+ private final AdaptiveFeatureGenerator identityGenerator =
Review Comment:
Please avoid/revert this line break.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/POSTaggerME.java:
##########
@@ -159,13 +187,14 @@ public POSTaggerME(POSModel model, POSTagFormat format) {
if (model.getPosSequenceModel() != null) {
this.model = model.getPosSequenceModel();
} else {
- this.model = new BeamSearch(beamSize,
model.getArtifact(POSModel.POS_MODEL_ENTRY_NAME), 0);
+ this.model = new BeamSearch(beamSize,
Review Comment:
Why has this line be put on a separate line? the formatting was okay before
and nothing has changed here.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -91,24 +119,21 @@ public void clearAdaptiveData() {
}
/**
- * @return Retrieves the number of times a cache hit occurred.
+ * @return Retrieves the number of cache hits for the current thread.
+ * @deprecated Cache statistics are no longer tracked.
*/
+ @Deprecated(since = "3.0.0")
public long getNumberOfCacheHits() {
- return numberOfCacheHits;
+ return 0;
}
/**
- * @return Retrieves the number of times a cache miss occurred.
+ * @return Retrieves the number of cache misses for the current thread.
+ * @deprecated Cache statistics are no longer tracked.
*/
+ @Deprecated(since = "3.0.0")
public long getNumberOfCacheMisses() {
- return numberOfCacheMisses;
Review Comment:
Should we instead throw a new `UnsupportedOperationException("Cache
statistics are no longer tracked.")` here?
The reason for letting this escalate:
"misses -> 0" is a statement to the caller that the case seems to work
perfectly fine / ideally even. However, this is wrong as zero used an
"unsupported" surrogate here... I'd rather escalate so callers can adapt their
code quickly without relying on incorrect "0" assumptions.
@rzo1 wdyt?
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/PreviousMapFeatureGenerator.java:
##########
@@ -27,11 +27,11 @@
*/
public class PreviousMapFeatureGenerator implements AdaptiveFeatureGenerator {
Review Comment:
(a) Can this class be marked `@ThreadSafe` now?
(b) See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/ConfigurablePOSContextGenerator.java:
##########
@@ -91,28 +102,28 @@ public String[] getContext(int index, String[] tokens,
String[] tags,
}
}
- String cacheKey = index + tagprev + tagprevprev;
- if (contextsCache != null) {
- if (wordsKey == tokens) {
- String[] cachedContexts = contextsCache.get(cacheKey);
+ if (threadState != null) {
+ CacheState state = threadState.get();
+ String cacheKey = index + tagprev + tagprevprev;
+ if (state.wordsKey == tokens) {
+ String[] cachedContexts = state.cache.get(cacheKey);
if (cachedContexts != null) {
return cachedContexts;
}
+ } else {
+ state.cache.clear();
+ state.wordsKey = tokens;
}
- else {
- contextsCache.clear();
- wordsKey = tokens;
- }
+
+ List<String> e = new ArrayList<>();
+ featureGenerator.createFeatures(e, tokens, index, tags);
+ String[] contexts = e.toArray(new String[0]);
+ state.cache.put(cacheKey, contexts);
+ return contexts;
}
List<String> e = new ArrayList<>();
-
featureGenerator.createFeatures(e, tokens, index, tags);
-
- String[] contexts = e.toArray(new String[0]);
- if (contextsCache != null) {
- contextsCache.put(cacheKey, contexts);
- }
- return contexts;
+ return e.toArray(new String[0]);
Review Comment:
There is no need to create an empty `List<String> e` for that. Hence, this
line can be simplified to:
````
return new String[0];
````
This should also be in an actual `else` block in case threadState is `null`,
right?
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/BeamSearch.java:
##########
@@ -34,23 +35,43 @@
* <p>
* This is based on the description in Ratnaparkhi (1998),
* PhD diss, Univ. of Pennsylvania.
+ * <p>
+ * This implementation is thread-safe. The contexts cache and probability
buffer
+ * are maintained per-thread via {@link ThreadLocal}.
+ * <p>
+ * <b>Note:</b> In container environments with classloader isolation (e.g.
Jakarta EE),
+ * {@link ThreadLocal} state may pin the classloader. Ensure instances do not
outlive
+ * the application's lifecycle, or call {@link ThreadLocal#remove()} on pooled
threads.
*
* @see Sequence
* @see SequenceValidator
* @see BeamSearchContextGenerator
*/
+@ThreadSafe
public class BeamSearch implements SequenceClassificationModel {
Review Comment:
Should BeamSearch now implement `AutoCloseable` to ensure consumers of
`BeamSearch` can leverage:
````
@Override
public void close() {
threadState.remove();
}
````
We have this supported for current ThreadSafe.... classes.
Please check and verify this for similar cases in changes introduced with
this PR.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/ThreadSafePOSTaggerME.java:
##########
@@ -42,7 +42,11 @@
* @see POSTagger
* @see POSTaggerME
* @see Probabilistic
+ *
+ * @deprecated As of OPENNLP-1816, {@link POSTaggerME} is
+ * itself thread-safe. Use it directly instead.
*/
+@Deprecated(since = "3.0.0")
@ThreadSafe
public class ThreadSafePOSTaggerME implements POSTagger, Probabilistic,
AutoCloseable {
Review Comment:
Cross note: See comment on `AutoCloseable` in `BeamSearch`
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/PreviousTwoMapFeatureGenerator.java:
##########
@@ -29,13 +29,14 @@
*/
public class PreviousTwoMapFeatureGenerator implements
AdaptiveFeatureGenerator {
Review Comment:
(a) Can this class be marked `@ThreadSafe` now?
(b) See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/AdditionalContextFeatureGenerator.java:
##########
@@ -28,11 +28,12 @@ public class AdditionalContextFeatureGenerator implements
AdaptiveFeatureGenerat
private static final String PREFIX = "ne=";
- private String[][] additionalContext;
+ private final ThreadLocal<String[][]> threadState = new ThreadLocal<>();
Review Comment:
Please leave a one line code comment on the purpose of the covered
String[][], previously named additional context. At least, these two words
should be left here.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/POSTaggerME.java:
##########
@@ -159,13 +187,14 @@ public POSTaggerME(POSModel model, POSTagFormat format) {
if (model.getPosSequenceModel() != null) {
this.model = model.getPosSequenceModel();
} else {
- this.model = new BeamSearch(beamSize,
model.getArtifact(POSModel.POS_MODEL_ENTRY_NAME), 0);
+ this.model = new BeamSearch(beamSize,
+ model.getArtifact(POSModel.POS_MODEL_ENTRY_NAME), 0);
}
- this.posTagFormatMapper = (format == POSTagFormat.CUSTOM)
+ this.posTagFormatMapper =
+ (format == POSTagFormat.CUSTOM)
Review Comment:
Why has this line be put on a separate line? the formatting was okay before
and nothing has changed here.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/POSTaggerME.java:
##########
@@ -188,8 +217,12 @@ public String[] tag(String[] sentence) {
*/
@Override
public String[] tag(String[] sentence, Object[] additionalContext) {
- bestSequence = model.bestSequence(sentence, additionalContext, cg,
sequenceValidator);
- final List<String> t = bestSequence.getOutcomes();
+ Sequence seq = model.bestSequence(sentence, additionalContext, cg,
sequenceValidator);
+ this.bestSequence = seq; // volatile write for backward-compatible probs()
access
Review Comment:
This assignment c/should be done after the subsequent null check, that is,
after line 224.
##########
opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/postag/POSTaggerMEBenchmark.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * 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.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import opennlp.tools.formats.ResourceAsStreamFactory;
+import opennlp.tools.util.InputStreamFactory;
+import opennlp.tools.util.ObjectStream;
+import opennlp.tools.util.Parameters;
+import opennlp.tools.util.PlainTextByLineStream;
+import opennlp.tools.util.TrainingParameters;
+import opennlp.tools.util.featuregen.CachedFeatureGenerator;
+import opennlp.tools.util.model.ModelType;
+
+/**
+ * JMH benchmark comparing POS tagger instance allocation
+ * strategies and cache configurations.
+ * <p>
+ * Measures 3 approaches x 2 cache configs = 6 combinations.
+ * The {@code allCaches} parameter controls both the context
+ * generator cache and the feature generator cache
+ * simultaneously, isolating the total impact of caching.
+ */
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.SECONDS)
+@Warmup(iterations = 5, time = 2)
+@Measurement(iterations = 10, time = 2)
+@Fork(2)
+public class POSTaggerMEBenchmark {
Review Comment:
Please reformat the entire class to go for at least 110 characters per line,
as this is allowed per OpenNLP checkstyle rules.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -21,62 +21,90 @@
import java.util.ArrayList;
import java.util.List;
+import opennlp.tools.commons.ThreadSafe;
import opennlp.tools.util.Cache;
/**
* Caches features of the aggregated {@link AdaptiveFeatureGenerator
generators}.
+ * <p>
+ * The cache is maintained per-thread via {@link ThreadLocal}, making this
class safe for
+ * concurrent use from multiple threads. Each thread gets its own independent
cache that is
+ * cleared when a new sentence (token array) is encountered.
+ * <p>
+ * <b>Note:</b> In container environments with classloader isolation (e.g.
Jakarta EE),
+ * {@link ThreadLocal} state may pin the classloader. Ensure instances do not
outlive
+ * the application's lifecycle, or call {@link ThreadLocal#remove()} on pooled
threads.
*
* @see Cache
*/
+@ThreadSafe
public class CachedFeatureGenerator implements AdaptiveFeatureGenerator {
+ /**
+ * System property to disable the feature cache globally.
+ * Set to {@code "true"} to bypass caching (useful for benchmarking).
+ */
+ public static final String DISABLE_CACHE_PROPERTY =
"opennlp.featuregen.cache.disabled";
+
private final AdaptiveFeatureGenerator generator;
+ private final boolean cacheEnabled;
- private String[] prevTokens;
+ private final ThreadLocal<CacheState> threadState =
+ ThreadLocal.withInitial(() -> new CacheState(new Cache<>(100)));
- private final Cache<Integer, List<String>> contextsCache;
+ private static final class CacheState {
+ private String[] prevTokens;
+ private final Cache<Integer, List<String>> cache;
- private long numberOfCacheHits;
- private long numberOfCacheMisses;
+ CacheState(Cache<Integer, List<String>> cache) {
+ this.cache = cache;
+ }
+ }
+ /**
+ * @deprecated Use {@link #CachedFeatureGenerator(AdaptiveFeatureGenerator)}
instead.
+ */
@Deprecated
public CachedFeatureGenerator(AdaptiveFeatureGenerator... generators) {
this.generator = new AggregatedFeatureGenerator(generators);
- contextsCache = new Cache<>(100);
+ this.cacheEnabled = !Boolean.getBoolean(DISABLE_CACHE_PROPERTY);
}
public CachedFeatureGenerator(AdaptiveFeatureGenerator generator) {
this.generator = generator;
- contextsCache = new Cache<>(100);
+ this.cacheEnabled = !Boolean.getBoolean(DISABLE_CACHE_PROPERTY);
}
@Override
public void createFeatures(List<String> features, String[] tokens, int index,
String[] previousOutcomes) {
+ if (!cacheEnabled) {
Review Comment:
Please add an explicit `else` block for the code below the `if` part here.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/DocumentBeginFeatureGenerator.java:
##########
@@ -26,14 +26,16 @@
*/
public class DocumentBeginFeatureGenerator implements AdaptiveFeatureGenerator
{
- private String[] firstSentence;
+ private final ThreadLocal<String[]> threadState = new ThreadLocal<>();
Review Comment:
Leave a code comment note on the meaning kept here, pls.
##########
opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/featuregen/CachedFeatureGeneratorTest.java:
##########
@@ -26,114 +26,77 @@
/**
* Test for the {@link CachedFeatureGenerator} class.
+ * <p>
+ * Caching has been removed for thread safety. These tests verify
+ * that the generator still delegates correctly to its underlying
+ * generator and that the deprecated cache stat methods return 0.
*/
public class CachedFeatureGeneratorTest {
- private final AdaptiveFeatureGenerator identityGenerator = new
IdentityFeatureGenerator();
+ private final AdaptiveFeatureGenerator identityGenerator =
+ new IdentityFeatureGenerator();
private String[] testSentence1;
-
private String[] testSentence2;
-
private List<String> features;
@BeforeEach
void setUp() {
-
testSentence1 = new String[] {"a1", "b1", "c1", "d1"};
-
testSentence2 = new String[] {"a2", "b2", "c2", "d2"};
-
features = new ArrayList<>();
}
- /**
- * Tests if cache works for one sentence and two different token indexes.
- */
@Test
- void testCachingOfSentence() {
- CachedFeatureGenerator generator = new
CachedFeatureGenerator(identityGenerator);
-
- int testIndex = 0;
-
- // after this call features are cached for testIndex
- generator.createFeatures(features, testSentence1, testIndex, null);
-
- Assertions.assertEquals(1, generator.getNumberOfCacheMisses());
- Assertions.assertEquals(0, generator.getNumberOfCacheHits());
-
- Assertions.assertTrue(features.contains(testSentence1[testIndex]));
-
- features.clear();
-
- // check if features are really cached
-
- final String expectedToken = testSentence1[testIndex];
-
- testSentence1[testIndex] = null;
-
- generator.createFeatures(features, testSentence1, testIndex, null);
-
- Assertions.assertEquals(1, generator.getNumberOfCacheMisses());
- Assertions.assertEquals(1, generator.getNumberOfCacheHits());
-
- Assertions.assertTrue(features.contains(expectedToken));
+ void testDelegatesToUnderlyingGenerator() {
+ CachedFeatureGenerator generator =
+ new CachedFeatureGenerator(identityGenerator);
+
+ generator.createFeatures(
+ features, testSentence1, 0, null);
+ Assertions.assertTrue(
+ features.contains(testSentence1[0]));
Assertions.assertEquals(1, features.size());
features.clear();
- // try caching with an other index
-
- int testIndex2 = testIndex + 1;
-
- generator.createFeatures(features, testSentence1, testIndex2, null);
-
- Assertions.assertEquals(2, generator.getNumberOfCacheMisses());
- Assertions.assertEquals(1, generator.getNumberOfCacheHits());
- Assertions.assertTrue(features.contains(testSentence1[testIndex2]));
-
- features.clear();
-
- // now check if cache still contains feature for testIndex
-
- generator.createFeatures(features, testSentence1, testIndex, null);
-
- Assertions.assertTrue(features.contains(expectedToken));
+ generator.createFeatures(
+ features, testSentence1, 1, null);
+ Assertions.assertTrue(
+ features.contains(testSentence1[1]));
+ Assertions.assertEquals(1, features.size());
}
- /**
- * Tests if the cache was cleared after the sentence changed.
- */
@Test
- void testCacheClearAfterSentenceChange() {
- CachedFeatureGenerator generator = new
CachedFeatureGenerator(identityGenerator);
-
- int testIndex = 0;
+ void testDifferentSentencesProduceCorrectFeatures() {
+ CachedFeatureGenerator generator =
+ new CachedFeatureGenerator(identityGenerator);
- // use generator with sentence 1
- generator.createFeatures(features, testSentence1, testIndex, null);
+ generator.createFeatures(
+ features, testSentence1, 0, null);
+ Assertions.assertTrue(
+ features.contains(testSentence1[0]));
features.clear();
- // use another sentence but same index
- generator.createFeatures(features, testSentence2, testIndex, null);
-
- Assertions.assertEquals(2, generator.getNumberOfCacheMisses());
- Assertions.assertEquals(0, generator.getNumberOfCacheHits());
-
- Assertions.assertTrue(features.contains(testSentence2[testIndex]));
+ generator.createFeatures(
+ features, testSentence2, 0, null);
+ Assertions.assertTrue(
+ features.contains(testSentence2[0]));
Assertions.assertEquals(1, features.size());
+ }
- features.clear();
-
- // check if features are really cached
- final String expectedToken = testSentence2[testIndex];
-
- testSentence2[testIndex] = null;
+ @Test
+ void testDeprecatedCacheStatsReturnZero() {
Review Comment:
This test should be discussed / adapted, see comment in class under test.
##########
opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/ThreadSafetyBenchmarkTest.java:
##########
@@ -0,0 +1,512 @@
+/*
+ * 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;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import opennlp.tools.chunker.ChunkSample;
+import opennlp.tools.chunker.ChunkSampleStream;
+import opennlp.tools.chunker.ChunkerFactory;
+import opennlp.tools.chunker.ChunkerME;
+import opennlp.tools.chunker.ChunkerModel;
+import opennlp.tools.formats.ResourceAsStreamFactory;
+import opennlp.tools.langdetect.LanguageDetectorFactory;
+import opennlp.tools.langdetect.LanguageDetectorME;
+import opennlp.tools.langdetect.LanguageDetectorModel;
+import opennlp.tools.langdetect.LanguageDetectorSampleStream;
+import opennlp.tools.lemmatizer.LemmaSample;
+import opennlp.tools.lemmatizer.LemmaSampleStream;
+import opennlp.tools.lemmatizer.LemmatizerFactory;
+import opennlp.tools.lemmatizer.LemmatizerME;
+import opennlp.tools.lemmatizer.LemmatizerModel;
+import opennlp.tools.namefind.BioCodec;
+import opennlp.tools.namefind.NameFinderME;
+import opennlp.tools.namefind.NameSample;
+import opennlp.tools.namefind.NameSampleDataStream;
+import opennlp.tools.namefind.TokenNameFinderFactory;
+import opennlp.tools.namefind.TokenNameFinderModel;
+import opennlp.tools.postag.POSModel;
+import opennlp.tools.postag.POSSample;
+import opennlp.tools.postag.POSTaggerFactory;
+import opennlp.tools.postag.POSTaggerME;
+import opennlp.tools.postag.WordTagSampleStream;
+import opennlp.tools.sentdetect.SentenceDetectorFactory;
+import opennlp.tools.sentdetect.SentenceDetectorME;
+import opennlp.tools.sentdetect.SentenceModel;
+import opennlp.tools.sentdetect.SentenceSample;
+import opennlp.tools.sentdetect.SentenceSampleStream;
+import opennlp.tools.tokenize.TokenSample;
+import opennlp.tools.tokenize.TokenSampleStream;
+import opennlp.tools.tokenize.TokenizerFactory;
+import opennlp.tools.tokenize.TokenizerME;
+import opennlp.tools.tokenize.TokenizerModel;
+import opennlp.tools.util.InputStreamFactory;
+import opennlp.tools.util.MockInputStreamFactory;
+import opennlp.tools.util.ObjectStream;
+import opennlp.tools.util.Parameters;
+import opennlp.tools.util.PlainTextByLineStream;
+import opennlp.tools.util.Span;
+import opennlp.tools.util.TrainingParameters;
+import opennlp.tools.util.model.ModelType;
+
+/**
+ * Thread-safety correctness tests for ME classes.
+ * <p>
+ * Each test shares a single ME instance across multiple threads
+ * that are barrier-synchronized to maximize contention. Every
+ * concurrent result is compared against a single-threaded baseline.
+ * <p>
+ * Uses {@link CyclicBarrier} to ensure all threads hit the critical
+ * section simultaneously, increasing the probability of surfacing
+ * races.
+ */
+public class ThreadSafetyBenchmarkTest {
Review Comment:
This seems to be a **heavy** load test to me.
Can we do these checks/verifications as part of an Integration Test in the
IT phase via failsafe-plugin of the `opennlp-runtime`?
Wdyt @rzo1 ?
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -91,24 +119,21 @@ public void clearAdaptiveData() {
}
/**
- * @return Retrieves the number of times a cache hit occurred.
+ * @return Retrieves the number of cache hits for the current thread.
+ * @deprecated Cache statistics are no longer tracked.
*/
+ @Deprecated(since = "3.0.0")
public long getNumberOfCacheHits() {
- return numberOfCacheHits;
+ return 0;
Review Comment:
Should we instead throw a new `UnsupportedOperationException("Cache
statistics are no longer tracked.")` here?
@rzo1 wdyt?
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/InSpanGenerator.java:
##########
Review Comment:
(a) Can this class be marked `@ThreadSafe` now?
(b) See comment in `BeamSearch` on `AutoCloseable`.
##########
opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/ThreadSafetyBenchmarkTest.java:
##########
@@ -0,0 +1,512 @@
+/*
+ * 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;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import opennlp.tools.chunker.ChunkSample;
+import opennlp.tools.chunker.ChunkSampleStream;
+import opennlp.tools.chunker.ChunkerFactory;
+import opennlp.tools.chunker.ChunkerME;
+import opennlp.tools.chunker.ChunkerModel;
+import opennlp.tools.formats.ResourceAsStreamFactory;
+import opennlp.tools.langdetect.LanguageDetectorFactory;
+import opennlp.tools.langdetect.LanguageDetectorME;
+import opennlp.tools.langdetect.LanguageDetectorModel;
+import opennlp.tools.langdetect.LanguageDetectorSampleStream;
+import opennlp.tools.lemmatizer.LemmaSample;
+import opennlp.tools.lemmatizer.LemmaSampleStream;
+import opennlp.tools.lemmatizer.LemmatizerFactory;
+import opennlp.tools.lemmatizer.LemmatizerME;
+import opennlp.tools.lemmatizer.LemmatizerModel;
+import opennlp.tools.namefind.BioCodec;
+import opennlp.tools.namefind.NameFinderME;
+import opennlp.tools.namefind.NameSample;
+import opennlp.tools.namefind.NameSampleDataStream;
+import opennlp.tools.namefind.TokenNameFinderFactory;
+import opennlp.tools.namefind.TokenNameFinderModel;
+import opennlp.tools.postag.POSModel;
+import opennlp.tools.postag.POSSample;
+import opennlp.tools.postag.POSTaggerFactory;
+import opennlp.tools.postag.POSTaggerME;
+import opennlp.tools.postag.WordTagSampleStream;
+import opennlp.tools.sentdetect.SentenceDetectorFactory;
+import opennlp.tools.sentdetect.SentenceDetectorME;
+import opennlp.tools.sentdetect.SentenceModel;
+import opennlp.tools.sentdetect.SentenceSample;
+import opennlp.tools.sentdetect.SentenceSampleStream;
+import opennlp.tools.tokenize.TokenSample;
+import opennlp.tools.tokenize.TokenSampleStream;
+import opennlp.tools.tokenize.TokenizerFactory;
+import opennlp.tools.tokenize.TokenizerME;
+import opennlp.tools.tokenize.TokenizerModel;
+import opennlp.tools.util.InputStreamFactory;
+import opennlp.tools.util.MockInputStreamFactory;
+import opennlp.tools.util.ObjectStream;
+import opennlp.tools.util.Parameters;
+import opennlp.tools.util.PlainTextByLineStream;
+import opennlp.tools.util.Span;
+import opennlp.tools.util.TrainingParameters;
+import opennlp.tools.util.model.ModelType;
+
+/**
+ * Thread-safety correctness tests for ME classes.
+ * <p>
+ * Each test shares a single ME instance across multiple threads
+ * that are barrier-synchronized to maximize contention. Every
+ * concurrent result is compared against a single-threaded baseline.
+ * <p>
+ * Uses {@link CyclicBarrier} to ensure all threads hit the critical
+ * section simultaneously, increasing the probability of surfacing
+ * races.
+ */
+public class ThreadSafetyBenchmarkTest {
Review Comment:
Please reformat the entire class to go for at least 110 characters per line,
as this is allowed per OpenNLP checkstyle rules.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
