From 8c8d27d5e1a3edb0d25aab2a5f23a15e7a7a5d68 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Wed, 8 Jul 2026 01:22:58 -0400 Subject: [PATCH 01/12] OPENNLP-1883: Make SnowballStemmer thread-safe; add StemmerFactory and SharingStemmer SnowballStemmer now routes each call to a per-thread generated engine via OwnerOrPerThreadState, the same pattern as the thread-safe *ME components, so one instance can be shared across threads without touching the generated Snowball code. StemmerFactory (api) captures stemmer configuration as an immutable, thread-safe seam for stateful engines; SharingStemmer adapts a factory into a shareable Stemmer for implementations that are not thread-safe themselves (e.g. PorterStemmer). NormalizationProfile.matchingAnalyzer() and TermAnalyzer are now safe to share across threads when stemming. --- .../java/opennlp/tools/stemmer/Stemmer.java | 29 ++++- .../opennlp/tools/stemmer/StemmerFactory.java | 53 ++++++++ .../tools/stemmer/PorterStemmerFactory.java | 32 +++++ .../opennlp/tools/stemmer/SharingStemmer.java | 53 ++++++++ .../stemmer/snowball/SnowballStemmer.java | 102 ++++++++------- .../snowball/SnowballStemmerFactory.java | 64 +++++++++ .../util/normalizer/NormalizationProfile.java | 21 ++- .../tools/util/normalizer/TermAnalyzer.java | 27 +++- .../tools/stemmer/StemmerFactoryTest.java | 122 ++++++++++++++++++ .../normalizer/NormalizationProfilesTest.java | 20 +++ .../util/normalizer/TermAnalyzerTest.java | 7 +- 11 files changed, 470 insertions(+), 60 deletions(-) create mode 100644 opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java create mode 100644 opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmerFactory.java create mode 100644 opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java create mode 100644 opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java create mode 100644 opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java index 750890eb65..6e8996312f 100644 --- a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java @@ -4,7 +4,7 @@ * 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 + * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * @@ -17,10 +17,35 @@ package opennlp.tools.stemmer; +import java.util.List; + /** - * The stemmer is reducing a word to its stem. + * Reduces a word to its root form. + * + *

Thread safety is implementation-specific: check the implementation's documentation before + * sharing an instance across threads. Stateful engines mutate internal buffers on each + * {@link #stem(CharSequence)} call; when an implementation is not documented as thread-safe, + * share a {@link StemmerFactory} across threads and confine each {@code Stemmer} to one thread, + * or wrap the factory in a thread-local adapter when a single {@code Stemmer} reference must be + * shared.

*/ public interface Stemmer { + /** + * Stems {@code word}. + * + * @param word The input word. Must not be {@code null}. + * @return The stemmed form. + */ CharSequence stem(CharSequence word); + + /** + * {@return every stem form for {@code word}} The default returns a single-element list with + * {@link #stem(CharSequence)}. Multi-output engines (Hunspell, lemmatizers) override this. + * + * @param word The input word. Must not be {@code null}. + */ + default List stemAll(CharSequence word) { + return List.of(stem(word)); + } } diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java new file mode 100644 index 0000000000..5f978835ac --- /dev/null +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java @@ -0,0 +1,53 @@ +/* + * 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.stemmer; + +/** + * An immutable, thread-safe factory for {@link Stemmer} instances. + * + *

Generated and stateful stemmers (Snowball, Porter, Hunspell, and similar) hold per-call + * mutable buffers and must not be shared across threads. A {@code StemmerFactory} captures the + * configuration (algorithm, repeat count, dictionary path, ...) and mints a fresh {@link Stemmer} + * on each {@link #newStemmer()} call. The factory itself is safe to share; confine each returned + * {@link Stemmer} to a single thread, or route through a thread-local adapter when a component must + * be shared.

+ * + *

Implementations must be immutable and thread-safe after construction.

+ */ +public interface StemmerFactory { + + /** + * {@return a new {@link Stemmer} confined to the calling thread} Implementations must return a + * distinct instance on each call when the underlying engine is stateful. + */ + Stemmer newStemmer(); + + /** + * Stems one word using a freshly minted {@link Stemmer}. + * + *

This is a convenience for one-off use. Pipelines that stem many tokens should call + * {@link #newStemmer()} once per thread instead of invoking this + * method per token.

+ * + * @param word The input word. Must not be {@code null}. + * @return The stemmed form. + */ + default CharSequence stem(CharSequence word) { + return newStemmer().stem(word); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmerFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmerFactory.java new file mode 100644 index 0000000000..992772778b --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmerFactory.java @@ -0,0 +1,32 @@ +/* + * 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.stemmer; + +import opennlp.tools.commons.ThreadSafe; + +/** + * A thread-safe factory for {@link PorterStemmer} instances. + */ +@ThreadSafe +public record PorterStemmerFactory() implements StemmerFactory { + + @Override + public Stemmer newStemmer() { + return new PorterStemmer(); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java new file mode 100644 index 0000000000..b199d2b6d1 --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java @@ -0,0 +1,53 @@ +/* + * 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.stemmer; + +import opennlp.tools.commons.ThreadSafe; +import opennlp.tools.util.OwnerOrPerThreadState; + +/** + * A {@link Stemmer} that is safe to share across threads by routing each call to a per-thread + * delegate minted from a {@link StemmerFactory}. Use it to share a stemmer whose implementation is + * not itself thread-safe, such as {@link PorterStemmer}; thread-safe implementations like + * {@code SnowballStemmer} do not need this wrapper. + * + *

The first thread uses a single owner delegate without touching {@link ThreadLocal} until a + * second thread appears, matching the {@link OwnerOrPerThreadState} pattern used by the + * thread-safe {@code *ME} components.

+ */ +@ThreadSafe +public final class SharingStemmer implements Stemmer { + + private final OwnerOrPerThreadState delegates; + + /** + * @param factory The factory that mints per-thread delegates. Must not be {@code null}. + * @throws IllegalArgumentException if {@code factory} is {@code null}. + */ + public SharingStemmer(StemmerFactory factory) { + if (factory == null) { + throw new IllegalArgumentException("factory must not be null"); + } + this.delegates = new OwnerOrPerThreadState<>(factory::newStemmer, stemmer -> { }); + } + + @Override + public CharSequence stem(CharSequence word) { + return delegates.get().stem(word); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java index 7b8b1636ae..99388728da 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java @@ -17,68 +17,76 @@ package opennlp.tools.stemmer.snowball; +import java.util.function.Supplier; + +import opennlp.tools.commons.ThreadSafe; import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.util.OwnerOrPerThreadState; +/** + * A {@link Stemmer} backed by the generated Snowball engines. + * + *

The generated engines hold mutable per-call buffers, so this class routes each call to a + * per-thread engine via {@link OwnerOrPerThreadState}, the same pattern the thread-safe {@code *ME} + * components use. A single {@code SnowballStemmer} instance is therefore safe to share across + * threads.

+ */ +@ThreadSafe public class SnowballStemmer implements Stemmer { - private final AbstractSnowballStemmer stemmer; + private final OwnerOrPerThreadState delegates; private final int repeat; + /** + * Creates a stemmer for the given algorithm and repeat count. + * + * @param algorithm The Snowball algorithm. Must not be {@code null}. + * @param repeat How many times to apply the stemmer per word. + */ public SnowballStemmer(ALGORITHM algorithm, int repeat) { this.repeat = repeat; - - if (ALGORITHM.ARABIC.equals(algorithm)) { - stemmer = new arabicStemmer(); - } else if (ALGORITHM.DANISH.equals(algorithm)) { - stemmer = new danishStemmer(); - } else if (ALGORITHM.DUTCH.equals(algorithm)) { - stemmer = new dutchStemmer(); - } else if (ALGORITHM.CATALAN.equals(algorithm)) { - stemmer = new catalanStemmer(); - } else if (ALGORITHM.ENGLISH.equals(algorithm)) { - stemmer = new englishStemmer(); - } else if (ALGORITHM.FINNISH.equals(algorithm)) { - stemmer = new finnishStemmer(); - } else if (ALGORITHM.FRENCH.equals(algorithm)) { - stemmer = new frenchStemmer(); - } else if (ALGORITHM.GERMAN.equals(algorithm)) { - stemmer = new germanStemmer(); - } else if (ALGORITHM.GREEK.equals(algorithm)) { - stemmer = new greekStemmer(); - } else if (ALGORITHM.HUNGARIAN.equals(algorithm)) { - stemmer = new hungarianStemmer(); - } else if (ALGORITHM.INDONESIAN.equals(algorithm)) { - stemmer = new indonesianStemmer(); - } else if (ALGORITHM.IRISH.equals(algorithm)) { - stemmer = new irishStemmer(); - } else if (ALGORITHM.ITALIAN.equals(algorithm)) { - stemmer = new italianStemmer(); - } else if (ALGORITHM.NORWEGIAN.equals(algorithm)) { - stemmer = new norwegianStemmer(); - } else if (ALGORITHM.PORTER.equals(algorithm)) { - stemmer = new porterStemmer(); - } else if (ALGORITHM.PORTUGUESE.equals(algorithm)) { - stemmer = new portugueseStemmer(); - } else if (ALGORITHM.ROMANIAN.equals(algorithm)) { - stemmer = new romanianStemmer(); - } else if (ALGORITHM.RUSSIAN.equals(algorithm)) { - stemmer = new russianStemmer(); - } else if (ALGORITHM.SPANISH.equals(algorithm)) { - stemmer = new spanishStemmer(); - } else if (ALGORITHM.SWEDISH.equals(algorithm)) { - stemmer = new swedishStemmer(); - } else if (ALGORITHM.TURKISH.equals(algorithm)) { - stemmer = new turkishStemmer(); - } else { - throw new IllegalStateException("Unexpected stemmer algorithm: " + algorithm); - } + final Supplier engine = engineFor(algorithm); + this.delegates = new OwnerOrPerThreadState<>(engine, stemmer -> { }); } + /** + * Creates a stemmer for the given algorithm with {@code repeat = 1}. + * + * @param algorithm The Snowball algorithm. Must not be {@code null}. + */ public SnowballStemmer(ALGORITHM algorithm) { this(algorithm, 1); } + private static Supplier engineFor(ALGORITHM algorithm) { + return switch (algorithm) { + case ARABIC -> arabicStemmer::new; + case DANISH -> danishStemmer::new; + case DUTCH -> dutchStemmer::new; + case CATALAN -> catalanStemmer::new; + case ENGLISH -> englishStemmer::new; + case FINNISH -> finnishStemmer::new; + case FRENCH -> frenchStemmer::new; + case GERMAN -> germanStemmer::new; + case GREEK -> greekStemmer::new; + case HUNGARIAN -> hungarianStemmer::new; + case INDONESIAN -> indonesianStemmer::new; + case IRISH -> irishStemmer::new; + case ITALIAN -> italianStemmer::new; + case NORWEGIAN -> norwegianStemmer::new; + case PORTER -> porterStemmer::new; + case PORTUGUESE -> portugueseStemmer::new; + case ROMANIAN -> romanianStemmer::new; + case RUSSIAN -> russianStemmer::new; + case SPANISH -> spanishStemmer::new; + case SWEDISH -> swedishStemmer::new; + case TURKISH -> turkishStemmer::new; + }; + } + + @Override public CharSequence stem(CharSequence word) { + final AbstractSnowballStemmer stemmer = delegates.get(); stemmer.setCurrent(word.toString()); diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java new file mode 100644 index 0000000000..7426e22660 --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java @@ -0,0 +1,64 @@ +/* + * 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.stemmer.snowball; + +import java.util.Objects; + +import opennlp.tools.commons.ThreadSafe; +import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.stemmer.StemmerFactory; + +/** + * A thread-safe factory for {@link SnowballStemmer} instances. Since {@link SnowballStemmer} is + * itself thread-safe, this factory mainly serves as an immutable capture of the stemmer + * configuration for APIs that accept a {@link StemmerFactory}. + * + * @param algorithm The Snowball algorithm. Must not be {@code null}. + * @param repeat How many times to apply the stemmer per word; must be positive. + */ +@ThreadSafe +public record SnowballStemmerFactory(SnowballStemmer.ALGORITHM algorithm, int repeat) + implements StemmerFactory { + + /** + * Creates a factory with {@code repeat = 1}. + * + * @param algorithm The Snowball algorithm. Must not be {@code null}. + */ + public SnowballStemmerFactory(SnowballStemmer.ALGORITHM algorithm) { + this(algorithm, 1); + } + + /** + * Validates the components. + * + * @throws NullPointerException if {@code algorithm} is {@code null}. + * @throws IllegalArgumentException if {@code repeat} is not positive. + */ + public SnowballStemmerFactory { + Objects.requireNonNull(algorithm, "algorithm"); + if (repeat <= 0) { + throw new IllegalArgumentException("repeat must be positive"); + } + } + + @Override + public Stemmer newStemmer() { + return new SnowballStemmer(algorithm, repeat); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java index e7a47477ed..d632ff4bb8 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java @@ -4,7 +4,7 @@ * 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 + * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * @@ -17,7 +17,9 @@ package opennlp.tools.util.normalizer; import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.stemmer.StemmerFactory; import opennlp.tools.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; /** * Per-language normalization settings, mirroring how OpenNLP already selects a Snowball stemmer by @@ -61,17 +63,24 @@ public record NormalizationProfile(String language, SnowballStemmer.ALGORITHM st } /** - * {@return a new {@link Stemmer} for this language} A fresh instance is returned on each call - * because the Snowball stemmers are stateful and not thread-safe. + * {@return a thread-safe factory for this language's Snowball stemmer} + */ + public StemmerFactory stemmerFactory() { + return new SnowballStemmerFactory(stemmerAlgorithm); + } + + /** + * {@return a new {@link Stemmer} for this language} The returned {@link SnowballStemmer} is + * thread-safe and may be shared across threads. */ public Stemmer newStemmer() { - return new SnowballStemmer(stemmerAlgorithm); + return stemmerFactory().newStemmer(); } /** * Returns a matching analyzer for this language: NFC, case folding, the language's - * {@linkplain #accentFold() diacritic fold} when it has one, then stemming. Each call builds an - * independent analyzer with its own stemmer, so use one per thread when stemming. + * {@linkplain #accentFold() diacritic fold} when it has one, then stemming. The returned + * analyzer is thread-safe: it stems through a thread-safe {@link SnowballStemmer}. * * @return the analyzer. */ diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java index 39f213aaad..4766c704e3 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java @@ -25,7 +25,9 @@ import java.util.Set; import opennlp.tools.lemmatizer.Lemmatizer; +import opennlp.tools.stemmer.SharingStemmer; import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.stemmer.StemmerFactory; import opennlp.tools.tokenize.uax29.WordTokenizer; import opennlp.tools.util.Span; @@ -41,10 +43,12 @@ * {@link Dimension#LEMMA} are enabled by supplying a {@link Stemmer} or {@link Lemmatizer}.

* *

An instance is immutable and is thread-safe when its configured transforms are. The built-in - * character normalizers are stateless, but the Snowball stemmers are not, so an analyzer configured - * with a {@link Stemmer} (for example through {@code NormalizationProfile.matchingAnalyzer()}) should - * not be shared across threads when {@link Dimension#STEM} is used. Build one with - * {@link #builder()}.

+ * character normalizers are stateless and the Snowball stemmers are thread-safe. When + * {@link Dimension#STEM} is enabled through {@link Builder#stem(StemmerFactory)}, stemming is + * routed through a {@link SharingStemmer} and the analyzer is safe to share across threads even + * when the factory mints stateful stemmers. A raw {@link Stemmer} passed to + * {@link Builder#stem(Stemmer)} is only safe when that stemmer is itself thread-safe or the + * analyzer is confined to one thread.

*/ public final class TermAnalyzer { @@ -392,6 +396,21 @@ public Builder stem(Stemmer value) { return this; } + /** + * Enables {@link Dimension#STEM} through a {@link StemmerFactory}. The analyzer receives a + * {@link SharingStemmer} so it can be shared across threads. + * + * @param factory The stemmer factory. Must not be {@code null}. + * @return this builder + * @throws IllegalArgumentException if {@code factory} is {@code null}. + */ + public Builder stem(StemmerFactory factory) { + if (factory == null) { + throw new IllegalArgumentException("factory must not be null"); + } + return stem(new SharingStemmer(factory)); + } + /** * Enables {@link Dimension#LEMMA} through the given lemmatizer. * diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java new file mode 100644 index 0000000000..7c7feeccab --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java @@ -0,0 +1,122 @@ +/* + * 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.stemmer; + +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import opennlp.tools.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; + +class StemmerFactoryTest { + + @Test + void snowballFactoryMatchesDirectStemmer() { + StemmerFactory factory = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + Stemmer direct = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + Assertions.assertEquals(direct.stem("running"), factory.newStemmer().stem("running")); + Assertions.assertEquals(direct.stem("running"), factory.stem("running")); + } + + @Test + void porterFactoryMatchesDirectStemmer() { + StemmerFactory factory = new PorterStemmerFactory(); + Stemmer direct = new PorterStemmer(); + Assertions.assertEquals(direct.stem("running"), factory.newStemmer().stem("running")); + } + + @Test + void stemAllDefaultReturnsSingleForm() { + StemmerFactory factory = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + List forms = factory.newStemmer().stemAll("running"); + Assertions.assertEquals(1, forms.size()); + Assertions.assertEquals("run", forms.getFirst()); + } + + @Test + void factoryRejectsInvalidRepeat() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH, 0)); + } + + @Test + void sharingStemmerMatchesSingleThreadReference() throws Exception { + StemmerFactory factory = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + Stemmer reference = factory.newStemmer(); + SharingStemmer shared = new SharingStemmer(factory); + + Assertions.assertEquals(reference.stem("running"), shared.stem("running")); + Assertions.assertEquals(reference.stem("this"), shared.stem("this")); + } + + @Test + void sharingStemmerIsSafeUnderConcurrentUse() throws Exception { + StemmerFactory factory = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + SharingStemmer shared = new SharingStemmer(factory); + CharSequence expectedRunning = factory.newStemmer().stem("running"); + CharSequence expectedDeclining = factory.newStemmer().stem("declining"); + + try (ExecutorService pool = Executors.newFixedThreadPool(8)) { + Future[] tasks = new Future[64]; + for (int i = 0; i < tasks.length; i++) { + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 200; n++) { + Assertions.assertEquals(expectedRunning, shared.stem("running")); + Assertions.assertEquals(expectedDeclining, shared.stem("declining")); + } + }); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void snowballStemmerIsSafeUnderConcurrentUse() throws Exception { + Stemmer shared = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + CharSequence expectedRunning = shared.stem("running"); + CharSequence expectedDeclining = shared.stem("declining"); + + try (ExecutorService pool = Executors.newFixedThreadPool(8)) { + Future[] tasks = new Future[64]; + for (int i = 0; i < tasks.length; i++) { + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 200; n++) { + Assertions.assertEquals(expectedRunning, shared.stem("running")); + Assertions.assertEquals(expectedDeclining, shared.stem("declining")); + } + }); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void sharingStemmerRejectsNullFactory() { + Assertions.assertThrows(IllegalArgumentException.class, () -> new SharingStemmer(null)); + } +} diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java index ba39a5cf1f..969261f951 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java @@ -201,4 +201,24 @@ public String[] getSupportedLanguages() { assertThrows(IllegalArgumentException.class, () -> NormalizationProfiles.detect(null, detector)); assertThrows(IllegalArgumentException.class, () -> NormalizationProfiles.detect("text", null)); } + + @Test + void testMatchingAnalyzerIsThreadSafeForStemming() throws Exception { + final TermAnalyzer analyzer = NormalizationProfiles.forLanguage("eng").orElseThrow().matchingAnalyzer(); + final String expected = analyzer.analyze("running").getFirst().at(Dimension.STEM); + + try (var pool = java.util.concurrent.Executors.newFixedThreadPool(8)) { + var tasks = new java.util.concurrent.Future[32]; + for (int i = 0; i < tasks.length; i++) { + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 50; n++) { + assertEquals(expected, analyzer.analyze("running").getFirst().at(Dimension.STEM)); + } + }); + } + for (var task : tasks) { + task.get(30, java.util.concurrent.TimeUnit.SECONDS); + } + } + } } diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java index d7fd52474e..ed2ce46ad3 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java @@ -30,6 +30,8 @@ import opennlp.tools.lemmatizer.Lemmatizer; import opennlp.tools.stemmer.PorterStemmer; +import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.stemmer.StemmerFactory; import opennlp.tools.util.Span; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -306,7 +308,10 @@ void testBuilderRejectsNullArguments() { .transform(null, CaseFoldCharSequenceNormalizer.getInstance())); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().transform(Dimension.CASE_FOLD, null)); - assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().stem(null)); + assertThrows(IllegalArgumentException.class, + () -> TermAnalyzer.builder().stem((Stemmer) null)); + assertThrows(IllegalArgumentException.class, + () -> TermAnalyzer.builder().stem((StemmerFactory) null)); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().lemmatize(null)); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().tokenizer(null)); } From 7ab7a01b3a63f6957fae6667de229d16a6df4286 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Wed, 8 Jul 2026 04:05:03 -0400 Subject: [PATCH 02/12] OPENNLP-1883: Add multi-threaded stemmer eval and expand concurrency unit tests MultiThreadedStemmerEval hammers shared SnowballStemmer instances for every algorithm, a SharingStemmer-wrapped PorterStemmer, and every NormalizationProfile matching analyzer from 8 threads, comparing each result against a single-threaded reference, mirroring MultiThreadedToolsEval. The unit tests now also cover all 21 algorithms under concurrency, the owner thread stemming concurrently with pool threads, virtual threads (a fresh thread per task), and Porter sharing. --- .../tools/stemmer/StemmerFactoryTest.java | 116 ++++++++++++ .../tools/eval/MultiThreadedStemmerEval.java | 175 ++++++++++++++++++ 2 files changed, 291 insertions(+) create mode 100644 opennlp-eval-tests/src/test/java/opennlp/tools/eval/MultiThreadedStemmerEval.java diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java index 7c7feeccab..0fc6f9efbb 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java @@ -17,7 +17,9 @@ package opennlp.tools.stemmer; +import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -25,6 +27,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import opennlp.tools.stemmer.snowball.SnowballStemmer; import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; @@ -119,4 +123,116 @@ void snowballStemmerIsSafeUnderConcurrentUse() throws Exception { void sharingStemmerRejectsNullFactory() { Assertions.assertThrows(IllegalArgumentException.class, () -> new SharingStemmer(null)); } + + @ParameterizedTest + @EnumSource(SnowballStemmer.ALGORITHM.class) + void everyAlgorithmIsSafeUnderConcurrentUse(SnowballStemmer.ALGORITHM algorithm) + throws Exception { + // Words across scripts; stemming any input is deterministic, so a mismatch under + // concurrency signals corrupted shared state, regardless of the input language. + List words = List.of("running", "declining", "ab'yle", "prévoyant", + "επιστροφή", "яблоками", "استفتياكما", "skrøbeligheder"); + Stemmer reference = new SnowballStemmer(algorithm); + List expected = new ArrayList<>(words.size()); + for (String word : words) { + expected.add(reference.stem(word).toString()); + } + + Stemmer shared = new SnowballStemmer(algorithm); + try (ExecutorService pool = Executors.newFixedThreadPool(4)) { + Future[] tasks = new Future[16]; + for (int i = 0; i < tasks.length; i++) { + final int offset = i; + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 50; n++) { + for (int w = 0; w < words.size(); w++) { + int idx = (w + offset) % words.size(); + Assertions.assertEquals(expected.get(idx), shared.stem(words.get(idx)).toString()); + } + } + }); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void ownerThreadCanStemConcurrentlyWithOtherThreads() throws Exception { + // The calling thread claims the owner fast path first; other threads must transition the + // stemmer to per-thread state while the owner keeps stemming. + Stemmer shared = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + CharSequence expectedRunning = shared.stem("running"); + CharSequence expectedDeclining = shared.stem("declining"); + + CountDownLatch started = new CountDownLatch(4); + try (ExecutorService pool = Executors.newFixedThreadPool(4)) { + Future[] tasks = new Future[4]; + for (int i = 0; i < tasks.length; i++) { + tasks[i] = pool.submit(() -> { + started.countDown(); + for (int n = 0; n < 500; n++) { + Assertions.assertEquals(expectedRunning, shared.stem("running")); + Assertions.assertEquals(expectedDeclining, shared.stem("declining")); + } + }); + } + Assertions.assertTrue(started.await(10, TimeUnit.SECONDS)); + // Owner thread stems concurrently with the pool threads. + for (int n = 0; n < 500; n++) { + Assertions.assertEquals(expectedRunning, shared.stem("running")); + Assertions.assertEquals(expectedDeclining, shared.stem("declining")); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void snowballStemmerIsSafeOnVirtualThreads() throws Exception { + // Every task runs on a fresh virtual thread, exercising per-thread state creation on each + // access rather than reuse from a fixed pool. + Stemmer shared = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + CharSequence expected = shared.stem("running"); + + try (ExecutorService pool = Executors.newVirtualThreadPerTaskExecutor()) { + List> tasks = new ArrayList<>(256); + for (int i = 0; i < 256; i++) { + tasks.add(pool.submit(() -> { + for (int n = 0; n < 20; n++) { + Assertions.assertEquals(expected, shared.stem("running")); + } + })); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void sharingPorterStemmerIsSafeUnderConcurrentUse() throws Exception { + // PorterStemmer is the remaining stateful engine; SharingStemmer must isolate it per thread. + Stemmer reference = new PorterStemmer(); + CharSequence expectedRunning = reference.stem("running"); + CharSequence expectedConspiracies = reference.stem("conspiracies"); + Stemmer shared = new SharingStemmer(new PorterStemmerFactory()); + + try (ExecutorService pool = Executors.newFixedThreadPool(8)) { + Future[] tasks = new Future[64]; + for (int i = 0; i < tasks.length; i++) { + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 200; n++) { + Assertions.assertEquals(expectedRunning, shared.stem("running")); + Assertions.assertEquals(expectedConspiracies, shared.stem("conspiracies")); + } + }); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } } diff --git a/opennlp-eval-tests/src/test/java/opennlp/tools/eval/MultiThreadedStemmerEval.java b/opennlp-eval-tests/src/test/java/opennlp/tools/eval/MultiThreadedStemmerEval.java new file mode 100644 index 0000000000..aaac3c4c2b --- /dev/null +++ b/opennlp-eval-tests/src/test/java/opennlp/tools/eval/MultiThreadedStemmerEval.java @@ -0,0 +1,175 @@ +/* + * 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.eval; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Random; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import opennlp.tools.commons.ThreadSafe; +import opennlp.tools.stemmer.PorterStemmerFactory; +import opennlp.tools.stemmer.SharingStemmer; +import opennlp.tools.stemmer.Stemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmer.ALGORITHM; +import opennlp.tools.util.normalizer.Dimension; +import opennlp.tools.util.normalizer.NormalizationProfiles; +import opennlp.tools.util.normalizer.Term; +import opennlp.tools.util.normalizer.TermAnalyzer; + +/** + * Tests that the thread-safe stemmer implementations really are thread-safe by hammering shared + * instances from many threads and comparing every result against a single-threaded reference. + * + * @see ThreadSafe + * @see MultiThreadedToolsEval + */ +public class MultiThreadedStemmerEval extends AbstractEvalTest { + + private static final int NUM_THREADS = 8; + private static final int RUNS_PER_THREAD = 500; + + /** + * Words across scripts and languages, including apostrophes, diacritics, Greek, Cyrillic and + * Arabic. Stemming is deterministic for any input, so every algorithm is run over the whole + * list; a mismatch signals cross-thread corruption of the stemmer's internal buffers. + */ + private static final List WORDS = List.of( + "running", "accompanying", "malediction", "softeners", "declining", + "importantíssimes", "besar", "accidentalment", + "abbattimento", "aborrecimentos", "importantísimas", + "esiintymispaikasta", "aabenbaringen", "skrøbeligheder", "aftonringningen", + "buchbindergesellen", "mitverursacht", "prévoyant", "examinateurs", + "ab'yle", "kaçmamaktadır", "sarayı'nı", + "abbahagynám", "konstrukciójából", "vliegtuigtransport", + "absurdităţilor", "saracilor", "peledakan", "bhfeidhm", + "επιστροφή", "στρατιωτών", "красивая", "яблоками", + "استفتياكما", "استنتاجاتهما"); + + @Test + public void runSharedSnowballStemmersMultiThreaded() throws Exception { + for (ALGORITHM algorithm : ALGORITHM.values()) { + List expected = referenceStems(new SnowballStemmer(algorithm)); + hammer(new SnowballStemmer(algorithm), expected, "Snowball " + algorithm); + } + } + + @Test + public void runSharedPorterStemmerMultiThreaded() throws Exception { + SharingStemmer shared = new SharingStemmer(new PorterStemmerFactory()); + List expected = referenceStems(new PorterStemmerFactory().newStemmer()); + hammer(shared, expected, "SharingStemmer(Porter)"); + } + + @Test + public void runMatchingAnalyzersMultiThreaded() throws Exception { + for (String language : NormalizationProfiles.supportedLanguages()) { + TermAnalyzer analyzer = + NormalizationProfiles.forLanguage(language).orElseThrow().matchingAnalyzer(); + + List expected = new ArrayList<>(WORDS.size()); + for (String word : WORDS) { + expected.add(stemDimension(analyzer, word)); + } + + runInThreads(threadIndex -> { + List order = shuffledIndexes(threadIndex); + for (int run = 0; run < RUNS_PER_THREAD / 10; run++) { + for (int i : order) { + Assertions.assertEquals(expected.get(i), stemDimension(analyzer, WORDS.get(i)), + () -> "matchingAnalyzer(" + language + ") corrupted under concurrency"); + } + } + }); + } + } + + private static String stemDimension(TermAnalyzer analyzer, String word) { + List terms = analyzer.analyze(word); + if (terms.isEmpty()) { + return ""; + } + StringBuilder sb = new StringBuilder(); + for (Term term : terms) { + sb.append(term.at(Dimension.STEM)).append(' '); + } + return sb.toString(); + } + + private static List referenceStems(Stemmer reference) { + List expected = new ArrayList<>(WORDS.size()); + for (String word : WORDS) { + expected.add(reference.stem(word).toString()); + } + return expected; + } + + private static void hammer(Stemmer shared, List expected, String label) throws Exception { + runInThreads(threadIndex -> { + List order = shuffledIndexes(threadIndex); + for (int run = 0; run < RUNS_PER_THREAD; run++) { + for (int i : order) { + Assertions.assertEquals(expected.get(i), shared.stem(WORDS.get(i)).toString(), + () -> label + " corrupted under concurrency for input '" + WORDS.get(i) + "'"); + } + } + }); + } + + /** + * Each thread visits the word list in its own order so that different threads are (almost) + * always inside different words at the same time, maximizing the chance of exposing shared + * mutable state. + */ + private static List shuffledIndexes(int seed) { + List order = new ArrayList<>(WORDS.size()); + for (int i = 0; i < WORDS.size(); i++) { + order.add(i); + } + Collections.shuffle(order, new Random(seed)); + return order; + } + + private interface ThreadBody { + void run(int threadIndex) throws Exception; + } + + private static void runInThreads(ThreadBody body) throws Exception { + try (ExecutorService pool = Executors.newFixedThreadPool(NUM_THREADS)) { + List> tasks = new ArrayList<>(NUM_THREADS); + for (int t = 0; t < NUM_THREADS; t++) { + final int threadIndex = t; + tasks.add(pool.submit(() -> { + body.run(threadIndex); + return null; + })); + } + for (Future task : tasks) { + task.get(120, TimeUnit.SECONDS); + } + } + } +} From cb9df986a655e1dab7b2c41dcb5ce88e40075a4d Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Wed, 8 Jul 2026 07:02:58 -0400 Subject: [PATCH 03/12] OPENNLP-1883: Add SnowballStemmer JMH benchmark: thread-safe vs pre-patch baseline Measures the thread-safe SnowballStemmer shared across threads and one per thread against a replica of the pre-patch plain-field implementation, at 1/8/32 threads. Results in BENCHMARKS.md: the OwnerOrPerThreadState overhead is within error bars up to 8 threads and only shows (~1.5x) at full 32-thread saturation. --- opennlp-core/opennlp-runtime/BENCHMARKS.md | 24 +++ .../snowball/SnowballStemmerBenchmark.java | 153 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java diff --git a/opennlp-core/opennlp-runtime/BENCHMARKS.md b/opennlp-core/opennlp-runtime/BENCHMARKS.md index 68c3ac185e..80b51eae4e 100644 --- a/opennlp-core/opennlp-runtime/BENCHMARKS.md +++ b/opennlp-core/opennlp-runtime/BENCHMARKS.md @@ -13,6 +13,7 @@ variance reporting. | `TokenizerMEBenchmark` | TokenizerME | 3 approaches | | `SentenceDetectorMEBenchmark` | SentenceDetectorME | 3 approaches | | `POSTaggerMEBenchmark` | POSTaggerME | 3 approaches x 2 cache configs | +| `SnowballStemmerBenchmark` | SnowballStemmer | 3 approaches (incl. pre-patch baseline) | ### Approaches measured @@ -56,6 +57,29 @@ builds. The throughput numbers should be within JMH's error margin. # ... build and run as above, compare ``` +### SnowballStemmer results (Linux, JDK 25, 32 cores, 2 forks x 10 iterations) + +`SnowballStemmerBenchmark` compares the thread-safe `SnowballStemmer` +(engine behind `OwnerOrPerThreadState`) against a replica of the +pre-patch implementation (engine in a plain field, not shareable). +One op = stemming 16 English words. + +| Strategy | 1 thread | 8 threads | 32 threads | +|----------|---------:|----------:|-----------:| +| `sharedInstance` (patched, one shared stemmer) | 560k ± 3k ops/s | 1.55M ± 0.17M | 3.16M ± 0.34M | +| `instancePerThread` (patched, stemmer per thread) | 509k ± 26k ops/s | 1.60M ± 0.17M | 2.94M ± 0.11M | +| `legacyInstancePerThread` (pre-patch, stemmer per thread) | 544k ± 19k ops/s | 1.46M ± 0.16M | 4.77M ± 0.39M | + +At 1 and 8 threads the three strategies are within (or nearly within) +each other's error bars: the `OwnerOrPerThreadState` lookup is not +measurable against the cost of stemming itself. Only at full +saturation (32 threads, hyperthreaded) does the legacy plain-field +baseline pull ahead (~1.5x): with every hardware thread busy, the +per-call owner check plus `ThreadLocal` lookup is no longer hidden by +memory-level parallelism. Real pipelines stem as one stage among many, +so the saturated-microbenchmark gap is an upper bound, and the legacy +strategy was not shareable across threads in the first place. + ### POSTagger cache impact The `POSTaggerMEBenchmark` uses `@Param({"0", "3"})` for cache diff --git a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java new file mode 100644 index 0000000000..e4821aa8c1 --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java @@ -0,0 +1,153 @@ +/* + * 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.stemmer.snowball; + +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.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.stemmer.Stemmer; + +/** + * JMH benchmark for the thread-safe {@link SnowballStemmer} versus the pre-patch implementation + * that held its generated engine in a plain field. + * + *

Three strategies are measured, all at {@link Threads#MAX}:

+ *
    + *
  • {@code sharedInstance} — one thread-safe {@link SnowballStemmer} shared by every thread + * (one thread runs on the owner fast path, the rest on {@link ThreadLocal} state)
    • + *
  • {@code instancePerThread} — one thread-safe {@link SnowballStemmer} per thread (every + * thread is the owner of its own instance, so this isolates the owner-fast-path cost)
    • + *
  • {@code legacyInstancePerThread} — the old non-thread-safe implementation, one per thread + * (the pre-patch baseline; sharing it across threads would be a correctness bug)
    • + *
+ */ +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 10, time = 2) +@Fork(2) +public class SnowballStemmerBenchmark { + + private static final String[] INPUT = { + "running", "accompanying", "malediction", "softeners", "declining", + "conspiracies", "monotonically", "annotations", "internationalization", + "denormalization", "photographers", "responsibilities", "acknowledgement", + "this", "cat", "querying" + }; + + /** + * Replica of the pre-patch {@code SnowballStemmer}: the generated engine lives in a plain + * field, so an instance must not be shared across threads. + */ + static final class LegacySnowballStemmer implements Stemmer { + + private final AbstractSnowballStemmer stemmer = new englishStemmer(); + + @Override + public CharSequence stem(CharSequence word) { + stemmer.setCurrent(word.toString()); + stemmer.stem(); + return stemmer.getCurrent(); + } + } + + @State(Scope.Benchmark) + public static class SharedState { + Stemmer stemmer; + + @Setup(Level.Trial) + public void create() { + stemmer = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + } + } + + @State(Scope.Thread) + public static class PerThreadState { + Stemmer stemmer; + + @Setup(Level.Trial) + public void create() { + stemmer = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + } + } + + @State(Scope.Thread) + public static class LegacyPerThreadState { + Stemmer stemmer; + + @Setup(Level.Trial) + public void create() { + stemmer = new LegacySnowballStemmer(); + } + } + + @Benchmark + @Threads(Threads.MAX) + public void sharedInstance(SharedState st, Blackhole bh) { + for (String s : INPUT) { + bh.consume(st.stemmer.stem(s)); + } + } + + @Benchmark + @Threads(Threads.MAX) + public void instancePerThread(PerThreadState pt, Blackhole bh) { + for (String s : INPUT) { + bh.consume(pt.stemmer.stem(s)); + } + } + + @Benchmark + @Threads(Threads.MAX) + public void legacyInstancePerThread(LegacyPerThreadState pt, Blackhole bh) { + for (String s : INPUT) { + bh.consume(pt.stemmer.stem(s)); + } + } + + /** + * Quick local iteration only: {@code forks(0)} disables JVM fork isolation + * (unlike {@code mvn} with the {@code jmh} profile). + * Use the Maven-invoked configuration for publishable numbers. + */ + public static void main(String[] args) throws Exception { + Options opt = new OptionsBuilder() + .include(SnowballStemmerBenchmark.class.getSimpleName()) + .forks(0) + .warmupIterations(3) + .measurementIterations(5) + .build(); + new Runner(opt).run(); + } +} From 6de3ee762157c24148a30d6ba15ed35c033294b1 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Wed, 8 Jul 2026 07:51:16 -0400 Subject: [PATCH 04/12] OPENNLP-1883: Add CachingStemmer: per-thread LRU memoization of word-to-stem mappings Natural text is Zipf-distributed, so the same words are stemmed constantly. CachingStemmer wraps a StemmerFactory with a bounded per-thread LRU (default 1024 entries) following the same OwnerOrPerThreadState pattern as the *ME components: no cross-thread sharing, thread-safe regardless of the delegate. On the JMH zipf workload the cache is a ~34x throughput multiplier; on a cache-hostile uniform workload over 8x the cache capacity it still breaks even. Results and a corrected JMH invocation are documented in BENCHMARKS.md. --- opennlp-core/opennlp-runtime/BENCHMARKS.md | 43 +++- .../stemmer/CachingStemmerBenchmark.java | 199 ++++++++++++++++++ .../opennlp/tools/stemmer/CachingStemmer.java | 119 +++++++++++ .../tools/stemmer/CachingStemmerTest.java | 139 ++++++++++++ 4 files changed, 492 insertions(+), 8 deletions(-) create mode 100644 opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java create mode 100644 opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java create mode 100644 opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/CachingStemmerTest.java diff --git a/opennlp-core/opennlp-runtime/BENCHMARKS.md b/opennlp-core/opennlp-runtime/BENCHMARKS.md index 80b51eae4e..fe7b47c8c0 100644 --- a/opennlp-core/opennlp-runtime/BENCHMARKS.md +++ b/opennlp-core/opennlp-runtime/BENCHMARKS.md @@ -14,6 +14,7 @@ variance reporting. | `SentenceDetectorMEBenchmark` | SentenceDetectorME | 3 approaches | | `POSTaggerMEBenchmark` | POSTaggerME | 3 approaches x 2 cache configs | | `SnowballStemmerBenchmark` | SnowballStemmer | 3 approaches (incl. pre-patch baseline) | +| `CachingStemmerBenchmark` | CachingStemmer | cached vs uncached x 2 workloads | ### Approaches measured @@ -31,17 +32,19 @@ mvn test-compile -Pjmh \ -pl opennlp-core/opennlp-runtime -am \ -Dforbiddenapis.skip=true -Dcheckstyle.skip=true +# Materialize the test classpath once (JMH's forked JVMs inherit +# java.class.path, which mvn exec:java does not populate — running +# through exec:java fails with ClassNotFoundException: ForkedMain) +mvn dependency:build-classpath -pl opennlp-core/opennlp-runtime \ + -Pjmh -DincludeScope=test -Dmdep.outputFile=/tmp/cp.txt + +CP="opennlp-core/opennlp-runtime/target/classes:opennlp-core/opennlp-runtime/target/test-classes:$(cat /tmp/cp.txt)" + # Run all ME benchmarks -mvn exec:java -pl opennlp-core/opennlp-runtime \ - -Pjmh -Dexec.classpathScope=test \ - -Dexec.mainClass=org.openjdk.jmh.Main \ - -Dexec.args="opennlp.tools.*.ME*" +java -cp "$CP" org.openjdk.jmh.Main 'opennlp.tools.*.ME*' # Run POSTagger only (includes cacheSize param) -mvn exec:java -pl opennlp-core/opennlp-runtime \ - -Pjmh -Dexec.classpathScope=test \ - -Dexec.mainClass=org.openjdk.jmh.Main \ - -Dexec.args="POSTaggerMEBenchmark" +java -cp "$CP" org.openjdk.jmh.Main POSTaggerMEBenchmark ``` ### Regression testing (stock vs patched) @@ -80,6 +83,30 @@ memory-level parallelism. Real pipelines stem as one stage among many, so the saturated-microbenchmark gap is an upper bound, and the legacy strategy was not shareable across threads in the first place. +### CachingStemmer results (same environment) + +`CachingStemmerBenchmark` compares a `CachingStemmer` (per-thread LRU, +default 1024 entries, wrapping the English Snowball stemmer) against +the uncached shared stemmer. One op = 16 tokens from a 64k-token +stream. The `zipf` workload samples a 512-word vocabulary with 1/rank +weights (real-text repetition; the cache holds the whole vocabulary); +`diverse` samples an 8192-word vocabulary uniformly (8x cache +capacity: mostly misses plus constant eviction). + +| Workload | Strategy | 8 threads | 32 threads | +|----------|----------|----------:|-----------:| +| `zipf` | `cachedShared` | 48.5M ± 0.7M ops/s | 95.4M ± 0.9M | +| `zipf` | `uncachedShared` | 1.43M ± 0.13M | 2.75M ± 0.34M | +| `diverse` | `cachedShared` | 1.81M ± 0.51M | 3.45M ± 0.18M | +| `diverse` | `uncachedShared` | 1.08M ± 0.09M | 3.13M ± 0.12M | + +On the Zipf workload the cache is a ~34x throughput multiplier (raw +stemming becomes a hash lookup for the dominant vocabulary). On the +cache-hostile workload it still does not lose: the ~12% residual hit +rate pays for the eviction overhead. The cache more than recovers the +`OwnerOrPerThreadState` lookup cost observed in +`SnowballStemmerBenchmark` at full saturation. + ### POSTagger cache impact The `POSTaggerMEBenchmark` uses `@Param({"0", "3"})` for cache diff --git a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java new file mode 100644 index 0000000000..13881b1c06 --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java @@ -0,0 +1,199 @@ +/* + * 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.stemmer; + +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +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.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; + +/** + * JMH benchmark for {@link CachingStemmer} against an uncached shared {@link SnowballStemmer}. + * + *

Two workloads drive both strategies:

+ *
    + *
  • {@code zipf} — a 64k-token stream sampled with 1/rank weights from a 512-word + * vocabulary. This models real text, where a small vocabulary dominates; the default + * 1024-entry cache holds the whole vocabulary.
    • + *
  • {@code diverse} — a 64k-token stream sampled uniformly from an 8192-word vocabulary, + * 8x the cache capacity. This is the cache-hostile case: mostly misses plus constant + * eviction, so it bounds the overhead the cache can add.
    • + *
+ * + *

One op stems 16 consecutive tokens from the stream; each benchmark thread walks the stream + * from its own cursor.

+ */ +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 10, time = 2) +@Fork(2) +public class CachingStemmerBenchmark { + + private static final int STREAM_LENGTH = 65536; + private static final int WORDS_PER_OP = 16; + + private static final String[] ROOTS = { + "run", "walk", "talk", "develop", "nation", "connect", "form", "create", + "act", "direct", "govern", "manage", "operate", "organize", "present", "relate", + "report", "state", "structure", "test", "train", "transform", "translate", "value", + "view", "wonder", "yield", "zone", "note", "mark", "place", "point" + }; + private static final String[] PREFIXES = { + "", "re", "un", "over", "under", "out", "pre", "post", + "non", "anti", "de", "dis", "mis", "sub", "super", "inter" + }; + private static final String[] SUFFIXES = { + "", "s", "ed", "ing", "er", "ers", "ation", "ations", + "ly", "ness", "ment", "ments", "ize", "ized", "izing", "al" + }; + + @State(Scope.Benchmark) + public static class WorkloadState { + + @Param({"zipf", "diverse"}) + String workload; + + String[] stream; + + @Setup(Level.Trial) + public void build() { + Random random = new Random(42); + List vocabulary = new ArrayList<>(); + if ("zipf".equals(workload)) { + // 32 roots x 16 suffixes = 512 unique words, sampled with 1/rank weights. + for (String root : ROOTS) { + for (String suffix : SUFFIXES) { + vocabulary.add(root + suffix); + } + } + double[] cumulative = new double[vocabulary.size()]; + double sum = 0; + for (int rank = 0; rank < vocabulary.size(); rank++) { + sum += 1.0 / (rank + 1); + cumulative[rank] = sum; + } + stream = new String[STREAM_LENGTH]; + for (int i = 0; i < STREAM_LENGTH; i++) { + double r = random.nextDouble() * sum; + int idx = 0; + while (cumulative[idx] < r) { + idx++; + } + stream[i] = vocabulary.get(idx); + } + } else { + // 16 prefixes x 32 roots x 16 suffixes = 8192 unique words, sampled uniformly. + for (String prefix : PREFIXES) { + for (String root : ROOTS) { + for (String suffix : SUFFIXES) { + vocabulary.add(prefix + root + suffix); + } + } + } + stream = new String[STREAM_LENGTH]; + for (int i = 0; i < STREAM_LENGTH; i++) { + stream[i] = vocabulary.get(random.nextInt(vocabulary.size())); + } + } + } + } + + @State(Scope.Benchmark) + public static class UncachedState { + Stemmer stemmer; + + @Setup(Level.Trial) + public void create() { + stemmer = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + } + } + + @State(Scope.Benchmark) + public static class CachedState { + Stemmer stemmer; + + @Setup(Level.Trial) + public void create() { + stemmer = new CachingStemmer( + new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH)); + } + } + + @State(Scope.Thread) + public static class Cursor { + int position; + + @Setup(Level.Trial) + public void randomize() { + position = new Random().nextInt(STREAM_LENGTH); + } + } + + @Benchmark + @Threads(Threads.MAX) + public void uncachedShared(WorkloadState w, UncachedState st, Cursor cursor, Blackhole bh) { + for (int i = 0; i < WORDS_PER_OP; i++) { + bh.consume(st.stemmer.stem(w.stream[cursor.position++ & (STREAM_LENGTH - 1)])); + } + } + + @Benchmark + @Threads(Threads.MAX) + public void cachedShared(WorkloadState w, CachedState st, Cursor cursor, Blackhole bh) { + for (int i = 0; i < WORDS_PER_OP; i++) { + bh.consume(st.stemmer.stem(w.stream[cursor.position++ & (STREAM_LENGTH - 1)])); + } + } + + /** + * Quick local iteration only: {@code forks(0)} disables JVM fork isolation + * (unlike {@code mvn} with the {@code jmh} profile). + * Use the Maven-invoked configuration for publishable numbers. + */ + public static void main(String[] args) throws Exception { + Options opt = new OptionsBuilder() + .include(CachingStemmerBenchmark.class.getSimpleName()) + .forks(0) + .warmupIterations(3) + .measurementIterations(5) + .build(); + new Runner(opt).run(); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java new file mode 100644 index 0000000000..bb4bc2bb2c --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -0,0 +1,119 @@ +/* + * 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.stemmer; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import opennlp.tools.commons.ThreadSafe; +import opennlp.tools.util.OwnerOrPerThreadState; + +/** + * A {@link Stemmer} that memoizes word-to-stem mappings in a bounded per-thread LRU cache. + * + *

Natural language is Zipf-distributed: a small vocabulary accounts for most tokens, so the + * same words are stemmed over and over. Wrapping a stemmer in a {@code CachingStemmer} turns + * those repeats into a hash lookup instead of a full stemming pass.

+ * + *

Each thread gets its own delegate stemmer (minted from the supplied {@link StemmerFactory}) + * and its own cache, following the {@link OwnerOrPerThreadState} pattern of the thread-safe + * {@code *ME} components. There is no cross-thread sharing at all, so instances are safe to share + * regardless of whether the factory's stemmers are. Memory is bounded by + * {@code capacity * averageWordLength} characters per thread that stems.

+ * + *

{@link #stemAll(CharSequence)} is forwarded to the delegate uncached, so multi-output + * engines keep their full result list.

+ */ +@ThreadSafe +public final class CachingStemmer implements Stemmer { + + /** + * Covers the high-frequency vocabulary of most corpora while keeping the per-thread footprint + * small. + */ + public static final int DEFAULT_CAPACITY = 1024; + + private final OwnerOrPerThreadState state; + + /** + * Creates a caching stemmer with the {@linkplain #DEFAULT_CAPACITY default capacity}. + * + * @param factory The factory that mints one delegate per thread. Must not be {@code null}. + * @throws IllegalArgumentException if {@code factory} is {@code null}. + */ + public CachingStemmer(StemmerFactory factory) { + this(factory, DEFAULT_CAPACITY); + } + + /** + * Creates a caching stemmer. + * + * @param factory The factory that mints one delegate per thread. Must not be {@code null}. + * @param capacity The maximum number of word-to-stem entries kept per thread; must be positive. + * @throws IllegalArgumentException if {@code factory} is {@code null} or {@code capacity} is + * not positive. + */ + public CachingStemmer(StemmerFactory factory, int capacity) { + if (factory == null) { + throw new IllegalArgumentException("factory must not be null"); + } + if (capacity <= 0) { + throw new IllegalArgumentException("capacity must be positive, got " + capacity); + } + this.state = new OwnerOrPerThreadState<>( + () -> new ThreadState(factory.newStemmer(), capacity), + threadState -> threadState.cache.clear()); + } + + @Override + public CharSequence stem(CharSequence word) { + final ThreadState ts = state.get(); + final String key = word.toString(); + final String cached = ts.cache.get(key); + if (cached != null) { + return cached; + } + // toString() detaches the result from any buffer the delegate may reuse internally. + final String stemmed = ts.delegate.stem(key).toString(); + ts.cache.put(key, stemmed); + return stemmed; + } + + @Override + public List stemAll(CharSequence word) { + return state.get().delegate.stemAll(word); + } + + private static final class ThreadState { + + private final Stemmer delegate; + private final LinkedHashMap cache; + + private ThreadState(Stemmer delegate, int capacity) { + this.delegate = delegate; + // Access-ordered so iteration order is least-recently-used first. + this.cache = new LinkedHashMap<>(Math.min(capacity, 4096), 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > capacity; + } + }; + } + } +} diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/CachingStemmerTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/CachingStemmerTest.java new file mode 100644 index 0000000000..6890dbca80 --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/CachingStemmerTest.java @@ -0,0 +1,139 @@ +/* + * 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.stemmer; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import opennlp.tools.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; + +class CachingStemmerTest { + + private static final SnowballStemmerFactory ENGLISH = + new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + + @Test + void cachedResultsMatchUncachedResults() { + Stemmer reference = ENGLISH.newStemmer(); + CachingStemmer cached = new CachingStemmer(ENGLISH); + + List words = List.of("running", "declining", "conspiracies", "this", + "running", "declining", "ab'yle", "running"); + for (String word : words) { + Assertions.assertEquals(reference.stem(word).toString(), cached.stem(word).toString(), + "mismatch for '" + word + "'"); + } + } + + @Test + void repeatedWordsHitTheCache() { + AtomicInteger delegateCalls = new AtomicInteger(); + StemmerFactory countingFactory = () -> { + Stemmer delegate = ENGLISH.newStemmer(); + return word -> { + delegateCalls.incrementAndGet(); + return delegate.stem(word); + }; + }; + + CachingStemmer cached = new CachingStemmer(countingFactory); + for (int i = 0; i < 100; i++) { + Assertions.assertEquals("run", cached.stem("running").toString()); + } + Assertions.assertEquals(1, delegateCalls.get()); + } + + @Test + void evictionKeepsResultsCorrect() { + AtomicInteger delegateCalls = new AtomicInteger(); + StemmerFactory countingFactory = () -> { + Stemmer delegate = ENGLISH.newStemmer(); + return word -> { + delegateCalls.incrementAndGet(); + return delegate.stem(word); + }; + }; + + // Capacity 2 with a 3-word cycle: every access evicts the word needed two steps later. + CachingStemmer cached = new CachingStemmer(countingFactory, 2); + Stemmer reference = ENGLISH.newStemmer(); + List words = List.of("running", "declining", "conspiracies"); + for (int round = 0; round < 5; round++) { + for (String word : words) { + Assertions.assertEquals(reference.stem(word).toString(), cached.stem(word).toString()); + } + } + Assertions.assertTrue(delegateCalls.get() > 3, + "expected evictions to force repeated delegate calls, got " + delegateCalls.get()); + } + + @Test + void stemAllBypassesTheCache() { + CachingStemmer cached = new CachingStemmer(ENGLISH); + List forms = cached.stemAll("running"); + Assertions.assertEquals(1, forms.size()); + Assertions.assertEquals("run", forms.getFirst().toString()); + } + + @Test + void isSafeUnderConcurrentUse() throws Exception { + Stemmer reference = ENGLISH.newStemmer(); + List words = List.of("running", "declining", "conspiracies", "annotations", + "photographers", "responsibilities", "this", "querying"); + List expected = new ArrayList<>(words.size()); + for (String word : words) { + expected.add(reference.stem(word).toString()); + } + + // Capacity below the vocabulary size, so threads continuously evict within their own cache. + CachingStemmer cached = new CachingStemmer(ENGLISH, 4); + try (ExecutorService pool = Executors.newFixedThreadPool(8)) { + Future[] tasks = new Future[32]; + for (int i = 0; i < tasks.length; i++) { + final int offset = i; + tasks[i] = pool.submit(() -> { + for (int n = 0; n < 200; n++) { + for (int w = 0; w < words.size(); w++) { + int idx = (w + offset) % words.size(); + Assertions.assertEquals(expected.get(idx), cached.stem(words.get(idx)).toString()); + } + } + }); + } + for (Future task : tasks) { + task.get(30, TimeUnit.SECONDS); + } + } + } + + @Test + void rejectsInvalidArguments() { + Assertions.assertThrows(IllegalArgumentException.class, () -> new CachingStemmer(null)); + Assertions.assertThrows(IllegalArgumentException.class, () -> new CachingStemmer(ENGLISH, 0)); + Assertions.assertThrows(IllegalArgumentException.class, () -> new CachingStemmer(ENGLISH, -1)); + } +} From bc0b474227f3e128865eb5e47c79ad209a1dbd6d Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Wed, 8 Jul 2026 08:05:10 -0400 Subject: [PATCH 05/12] OPENNLP-1883: Route TermAnalyzer factory-based stemming through CachingStemmer Builder.stem(StemmerFactory) now wraps the factory in a CachingStemmer (default capacity), with a stem(StemmerFactory, int) overload for explicit cache sizing, and NormalizationProfile.matchingAnalyzer() builds through the factory again so profile analyzers get the per-thread stem cache for free. Analyzer results are unchanged; repeated words resolve from the cache instead of being re-stemmed. --- .../util/normalizer/NormalizationProfile.java | 6 ++-- .../tools/util/normalizer/TermAnalyzer.java | 34 +++++++++++++------ .../util/normalizer/TermAnalyzerTest.java | 21 ++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java index d632ff4bb8..c35f198064 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java @@ -80,7 +80,9 @@ public Stemmer newStemmer() { /** * Returns a matching analyzer for this language: NFC, case folding, the language's * {@linkplain #accentFold() diacritic fold} when it has one, then stemming. The returned - * analyzer is thread-safe: it stems through a thread-safe {@link SnowballStemmer}. + * analyzer is thread-safe, and repeated words resolve from a bounded per-thread stem cache + * instead of being re-stemmed (natural text is Zipf-distributed, so most tokens are cache + * hits). * * @return the analyzer. */ @@ -89,6 +91,6 @@ public TermAnalyzer matchingAnalyzer() { if (accentFold != null) { builder.transform(Dimension.ACCENT_FOLD, accentFold); } - return builder.stem(newStemmer()).build(); + return builder.stem(stemmerFactory()).build(); } } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java index 4766c704e3..8521e2e3ef 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java @@ -25,7 +25,7 @@ import java.util.Set; import opennlp.tools.lemmatizer.Lemmatizer; -import opennlp.tools.stemmer.SharingStemmer; +import opennlp.tools.stemmer.CachingStemmer; import opennlp.tools.stemmer.Stemmer; import opennlp.tools.stemmer.StemmerFactory; import opennlp.tools.tokenize.uax29.WordTokenizer; @@ -45,10 +45,10 @@ *

An instance is immutable and is thread-safe when its configured transforms are. The built-in * character normalizers are stateless and the Snowball stemmers are thread-safe. When * {@link Dimension#STEM} is enabled through {@link Builder#stem(StemmerFactory)}, stemming is - * routed through a {@link SharingStemmer} and the analyzer is safe to share across threads even - * when the factory mints stateful stemmers. A raw {@link Stemmer} passed to - * {@link Builder#stem(Stemmer)} is only safe when that stemmer is itself thread-safe or the - * analyzer is confined to one thread.

+ * routed through a {@link CachingStemmer}: the analyzer is safe to share across threads even when + * the factory mints stateful stemmers, and repeated words resolve from a bounded per-thread cache + * instead of being re-stemmed. A raw {@link Stemmer} passed to {@link Builder#stem(Stemmer)} is + * only safe when that stemmer is itself thread-safe or the analyzer is confined to one thread.

*/ public final class TermAnalyzer { @@ -398,17 +398,31 @@ public Builder stem(Stemmer value) { /** * Enables {@link Dimension#STEM} through a {@link StemmerFactory}. The analyzer receives a - * {@link SharingStemmer} so it can be shared across threads. + * {@link CachingStemmer} with the {@linkplain CachingStemmer#DEFAULT_CAPACITY default cache + * capacity}: it can be shared across threads, and repeated words resolve from a bounded + * per-thread cache instead of being re-stemmed. * * @param factory The stemmer factory. Must not be {@code null}. * @return this builder * @throws IllegalArgumentException if {@code factory} is {@code null}. */ public Builder stem(StemmerFactory factory) { - if (factory == null) { - throw new IllegalArgumentException("factory must not be null"); - } - return stem(new SharingStemmer(factory)); + return stem(factory, CachingStemmer.DEFAULT_CAPACITY); + } + + /** + * Enables {@link Dimension#STEM} through a {@link StemmerFactory} with an explicit stem cache + * capacity, otherwise identical to {@link #stem(StemmerFactory)}. + * + * @param factory The stemmer factory. Must not be {@code null}. + * @param cacheSize The maximum number of word-to-stem entries kept per thread; must be + * positive. + * @return this builder + * @throws IllegalArgumentException if {@code factory} is {@code null} or {@code cacheSize} is + * not positive. + */ + public Builder stem(StemmerFactory factory, int cacheSize) { + return stem(new CachingStemmer(factory, cacheSize)); } /** diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java index ed2ce46ad3..e956a66ff6 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/TermAnalyzerTest.java @@ -32,6 +32,8 @@ import opennlp.tools.stemmer.PorterStemmer; import opennlp.tools.stemmer.Stemmer; import opennlp.tools.stemmer.StemmerFactory; +import opennlp.tools.stemmer.snowball.SnowballStemmer; +import opennlp.tools.stemmer.snowball.SnowballStemmerFactory; import opennlp.tools.util.Span; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -312,10 +314,29 @@ void testBuilderRejectsNullArguments() { () -> TermAnalyzer.builder().stem((Stemmer) null)); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().stem((StemmerFactory) null)); + assertThrows(IllegalArgumentException.class, + () -> TermAnalyzer.builder().stem(new SnowballStemmerFactory( + SnowballStemmer.ALGORITHM.ENGLISH), 0)); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().lemmatize(null)); assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().tokenizer(null)); } + @Test + void testStemFactoryCachedResultsMatchDirectStemmer() { + final TermAnalyzer cached = TermAnalyzer.builder().caseFold() + .stem(new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH)).build(); + final TermAnalyzer direct = TermAnalyzer.builder().caseFold() + .stem(new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH)).build(); + + final String text = "Running runners kept running while the RUNNING continued"; + final List cachedTerms = cached.analyze(text); + final List directTerms = direct.analyze(text); + assertEquals(directTerms.size(), cachedTerms.size()); + for (int i = 0; i < directTerms.size(); i++) { + assertEquals(directTerms.get(i).at(Dimension.STEM), cachedTerms.get(i).at(Dimension.STEM)); + } + } + @Test void testMaxTokenLengthRejectsNonPositiveValues() { assertThrows(IllegalArgumentException.class, () -> TermAnalyzer.builder().maxTokenLength(0)); From b76eef40ebffc83963ef5807271305f594ec7fd0 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Fri, 10 Jul 2026 18:40:08 -0400 Subject: [PATCH 06/12] OPENNLP-1883: Address review: wrapper forwarding, state release, zero-indirection factory products SharingStemmer forwards stemAll to its per-thread delegate so a multi-output engine keeps its full result list through the wrapper, and all three thread-routing stemmers expose clearThreadLocalState(), mirroring the thread-safe ME components, so pooled threads can release their engine and cache instead of retaining them until the instance is collected. The caching javadoc and BENCHMARKS.md now state that the memoization is keyed to the physical thread and what that means on a virtual-thread-per-task executor. SnowballStemmerFactory products are now thread-confined stemmers driving their engine through a plain field, so the one-stemmer-per-thread pattern and the per-thread delegates inside the caching and sharing wrappers pay none of the shared-instance thread routing. The SnowballStemmer constructor validates algorithm and repeat like the factory, validation is IllegalArgumentException across the new surface, the LRU map is sized for its load factor so a full cache never rehashes, and StemmerFactory documents that it is a plain supplier, not one of the BaseToolFactory tool factories; its one-shot stem convenience method is gone. Tests cover the multi-output forwarding, the repeat pass-through with a witness word that stems differently at repeat one and two, the owner cache reset and worker-thread delegate release behind clearThreadLocalState, and the constructor validation. --- .../java/opennlp/tools/stemmer/Stemmer.java | 4 +- .../opennlp/tools/stemmer/StemmerFactory.java | 30 ++---- opennlp-core/opennlp-runtime/BENCHMARKS.md | 7 ++ .../opennlp/tools/stemmer/CachingStemmer.java | 21 +++- .../opennlp/tools/stemmer/SharingStemmer.java | 16 ++++ .../stemmer/snowball/SnowballStemmer.java | 22 ++++- .../snowball/SnowballStemmerFactory.java | 47 +++++++-- .../util/normalizer/NormalizationProfile.java | 9 +- .../tools/util/normalizer/TermAnalyzer.java | 4 +- .../tools/stemmer/StemmerFactoryTest.java | 95 ++++++++++++++++++- .../normalizer/NormalizationProfilesTest.java | 9 +- 11 files changed, 221 insertions(+), 43 deletions(-) diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java index 6e8996312f..30035b59b1 100644 --- a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java @@ -41,7 +41,9 @@ public interface Stemmer { /** * {@return every stem form for {@code word}} The default returns a single-element list with - * {@link #stem(CharSequence)}. Multi-output engines (Hunspell, lemmatizers) override this. + * {@link #stem(CharSequence)}. Dictionary-based engines that can produce several root forms for + * one word override this; wrappers that delegate to another {@code Stemmer} must forward this + * method so a multi-output delegate keeps its full result list. * * @param word The input word. Must not be {@code null}. */ diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java index 5f978835ac..2bc86fe751 100644 --- a/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java @@ -20,12 +20,16 @@ /** * An immutable, thread-safe factory for {@link Stemmer} instances. * - *

Generated and stateful stemmers (Snowball, Porter, Hunspell, and similar) hold per-call - * mutable buffers and must not be shared across threads. A {@code StemmerFactory} captures the - * configuration (algorithm, repeat count, dictionary path, ...) and mints a fresh {@link Stemmer} - * on each {@link #newStemmer()} call. The factory itself is safe to share; confine each returned - * {@link Stemmer} to a single thread, or route through a thread-local adapter when a component must - * be shared.

+ *

Stateful stemming engines hold per-call mutable buffers and must not be shared across + * threads. A {@code StemmerFactory} captures the configuration (algorithm, repeat count, + * dictionary path, ...) and mints a fresh {@link Stemmer} on each {@link #newStemmer()} call. The + * factory itself is safe to share; confine each returned {@link Stemmer} to a single thread, or + * route through a thread-local adapter when a component must be shared.

+ * + *

Despite the name, this is not one of the {@code BaseToolFactory}-based tool factories (such + * as {@code LemmatizerFactory}) that are instantiated by name from a model manifest. It is a + * plain functional supplier of configured {@link Stemmer} instances and takes no part in the + * model-loading mechanism.

* *

Implementations must be immutable and thread-safe after construction.

*/ @@ -36,18 +40,4 @@ public interface StemmerFactory { * distinct instance on each call when the underlying engine is stateful. */ Stemmer newStemmer(); - - /** - * Stems one word using a freshly minted {@link Stemmer}. - * - *

This is a convenience for one-off use. Pipelines that stem many tokens should call - * {@link #newStemmer()} once per thread instead of invoking this - * method per token.

- * - * @param word The input word. Must not be {@code null}. - * @return The stemmed form. - */ - default CharSequence stem(CharSequence word) { - return newStemmer().stem(word); - } } diff --git a/opennlp-core/opennlp-runtime/BENCHMARKS.md b/opennlp-core/opennlp-runtime/BENCHMARKS.md index fe7b47c8c0..e3c9ac047b 100644 --- a/opennlp-core/opennlp-runtime/BENCHMARKS.md +++ b/opennlp-core/opennlp-runtime/BENCHMARKS.md @@ -107,6 +107,13 @@ rate pays for the eviction overhead. The cache more than recovers the `OwnerOrPerThreadState` lookup cost observed in `SnowballStemmerBenchmark` at full saturation. +The cache is keyed to the physical thread, and these runs use a fixed +platform-thread pool whose threads live for the whole measurement. On +a virtual-thread-per-task executor every task starts with an empty +cache, so the multiplier only applies to repeats within one task; +workloads that stem a handful of words per task should expect +uncached-level throughput there. + ### POSTagger cache impact The `POSTaggerMEBenchmark` uses `@Param({"0", "3"})` for cache diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index bb4bc2bb2c..c6d944229b 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -37,6 +37,12 @@ * regardless of whether the factory's stemmers are. Memory is bounded by * {@code capacity * averageWordLength} characters per thread that stems.

* + *

Because the cache is keyed to the thread, the memoization pays off on threads that are + * reused across many words, such as a fixed platform-thread pool. On a virtual-thread-per-task + * executor every task starts with an empty cache, so repeats are only served within one task. + * Call {@link #clearThreadLocalState()} before returning a pooled thread that should not retain + * its delegate and cache.

+ * *

{@link #stemAll(CharSequence)} is forwarded to the delegate uncached, so multi-output * engines keep their full result list.

*/ @@ -100,6 +106,15 @@ public List stemAll(CharSequence word) { return state.get().delegate.stemAll(word); } + /** + * Removes this thread's delegate and cache to prevent classloader leaks in container + * environments. Call when the thread is returned to a pool or the stemmer is no longer needed, + * mirroring {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. + */ + public void clearThreadLocalState() { + state.clearForCurrentThread(); + } + private static final class ThreadState { private final Stemmer delegate; @@ -107,8 +122,10 @@ private static final class ThreadState { private ThreadState(Stemmer delegate, int capacity) { this.delegate = delegate; - // Access-ordered so iteration order is least-recently-used first. - this.cache = new LinkedHashMap<>(Math.min(capacity, 4096), 0.75f, true) { + // Access-ordered so iteration order is least-recently-used first. The initial table size + // accounts for the 0.75 load factor so a cache filled to capacity never rehashes; the clamp + // keeps the eager allocation reasonable for very large capacities. + this.cache = new LinkedHashMap<>(Math.min((int) (capacity / 0.75f) + 1, 4096), 0.75f, true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { return size() > capacity; diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java index b199d2b6d1..a66d7863dc 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java @@ -17,6 +17,8 @@ package opennlp.tools.stemmer; +import java.util.List; + import opennlp.tools.commons.ThreadSafe; import opennlp.tools.util.OwnerOrPerThreadState; @@ -50,4 +52,18 @@ public SharingStemmer(StemmerFactory factory) { public CharSequence stem(CharSequence word) { return delegates.get().stem(word); } + + @Override + public List stemAll(CharSequence word) { + return delegates.get().stemAll(word); + } + + /** + * Removes this thread's delegate to prevent classloader leaks in container environments. Call + * when the thread is returned to a pool or the stemmer is no longer needed, mirroring + * {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. + */ + public void clearThreadLocalState() { + delegates.clearForCurrentThread(); + } } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java index 99388728da..10817c1f8e 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java @@ -41,9 +41,17 @@ public class SnowballStemmer implements Stemmer { * Creates a stemmer for the given algorithm and repeat count. * * @param algorithm The Snowball algorithm. Must not be {@code null}. - * @param repeat How many times to apply the stemmer per word. + * @param repeat How many times to apply the stemmer per word; must be positive. + * @throws IllegalArgumentException Thrown if {@code algorithm} is {@code null} or + * {@code repeat} is not positive. */ public SnowballStemmer(ALGORITHM algorithm, int repeat) { + if (algorithm == null) { + throw new IllegalArgumentException("algorithm must not be null"); + } + if (repeat <= 0) { + throw new IllegalArgumentException("repeat must be positive, got " + repeat); + } this.repeat = repeat; final Supplier engine = engineFor(algorithm); this.delegates = new OwnerOrPerThreadState<>(engine, stemmer -> { }); @@ -58,7 +66,8 @@ public SnowballStemmer(ALGORITHM algorithm) { this(algorithm, 1); } - private static Supplier engineFor(ALGORITHM algorithm) { + // Shared with SnowballStemmerFactory, whose products embed one engine directly. + static Supplier engineFor(ALGORITHM algorithm) { return switch (algorithm) { case ARABIC -> arabicStemmer::new; case DANISH -> danishStemmer::new; @@ -97,6 +106,15 @@ public CharSequence stem(CharSequence word) { return stemmer.getCurrent(); } + /** + * Removes this thread's engine to prevent classloader leaks in container environments. Call + * when the thread is returned to a pool or the stemmer is no longer needed, mirroring + * {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. + */ + public void clearThreadLocalState() { + delegates.clearForCurrentThread(); + } + public enum ALGORITHM { ARABIC, DANISH, diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java index 7426e22660..4deb46ca89 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java @@ -17,16 +17,19 @@ package opennlp.tools.stemmer.snowball; -import java.util.Objects; - import opennlp.tools.commons.ThreadSafe; import opennlp.tools.stemmer.Stemmer; import opennlp.tools.stemmer.StemmerFactory; /** - * A thread-safe factory for {@link SnowballStemmer} instances. Since {@link SnowballStemmer} is - * itself thread-safe, this factory mainly serves as an immutable capture of the stemmer - * configuration for APIs that accept a {@link StemmerFactory}. + * A thread-safe factory that captures a Snowball stemmer configuration for APIs that accept a + * {@link StemmerFactory}. + * + *

{@link #newStemmer()} returns a thread-confined stemmer that drives its generated engine + * through a plain field, with none of the per-call thread routing of the shareable + * {@link SnowballStemmer}. This keeps the one-stemmer-per-thread pattern (and per-thread + * delegates inside caching or sharing wrappers) free of that indirection; use + * {@link SnowballStemmer} directly when a single shared instance is wanted instead.

* * @param algorithm The Snowball algorithm. Must not be {@code null}. * @param repeat How many times to apply the stemmer per word; must be positive. @@ -47,18 +50,42 @@ public SnowballStemmerFactory(SnowballStemmer.ALGORITHM algorithm) { /** * Validates the components. * - * @throws NullPointerException if {@code algorithm} is {@code null}. - * @throws IllegalArgumentException if {@code repeat} is not positive. + * @throws IllegalArgumentException Thrown if {@code algorithm} is {@code null} or + * {@code repeat} is not positive. */ public SnowballStemmerFactory { - Objects.requireNonNull(algorithm, "algorithm"); + if (algorithm == null) { + throw new IllegalArgumentException("algorithm must not be null"); + } if (repeat <= 0) { - throw new IllegalArgumentException("repeat must be positive"); + throw new IllegalArgumentException("repeat must be positive, got " + repeat); } } @Override public Stemmer newStemmer() { - return new SnowballStemmer(algorithm, repeat); + return new ConfinedStemmer(SnowballStemmer.engineFor(algorithm).get(), repeat); + } + + // One engine in a plain field: the zero-indirection product for thread-confined use, per the + // StemmerFactory contract. Not thread-safe. + private static final class ConfinedStemmer implements Stemmer { + + private final AbstractSnowballStemmer engine; + private final int repeat; + + private ConfinedStemmer(AbstractSnowballStemmer engine, int repeat) { + this.engine = engine; + this.repeat = repeat; + } + + @Override + public CharSequence stem(CharSequence word) { + engine.setCurrent(word.toString()); + for (int i = 0; i < repeat; i++) { + engine.stem(); + } + return engine.getCurrent(); + } } } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java index c35f198064..d66b402eb9 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java @@ -63,7 +63,9 @@ public record NormalizationProfile(String language, SnowballStemmer.ALGORITHM st } /** - * {@return a thread-safe factory for this language's Snowball stemmer} + * {@return a thread-safe factory for this language's Snowball stemmer} The factory may be + * shared; each {@linkplain StemmerFactory#newStemmer() minted stemmer} is confined to the + * calling thread. */ public StemmerFactory stemmerFactory() { return new SnowballStemmerFactory(stemmerAlgorithm); @@ -74,7 +76,7 @@ public StemmerFactory stemmerFactory() { * thread-safe and may be shared across threads. */ public Stemmer newStemmer() { - return stemmerFactory().newStemmer(); + return new SnowballStemmer(stemmerAlgorithm); } /** @@ -82,7 +84,8 @@ public Stemmer newStemmer() { * {@linkplain #accentFold() diacritic fold} when it has one, then stemming. The returned * analyzer is thread-safe, and repeated words resolve from a bounded per-thread stem cache * instead of being re-stemmed (natural text is Zipf-distributed, so most tokens are cache - * hits). + * hits). The cache is keyed to the thread, so build the analyzer once and reuse it from + * threads that live across many calls, such as a fixed platform-thread pool. * * @return the analyzer. */ diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java index 8521e2e3ef..934002d7eb 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/TermAnalyzer.java @@ -400,7 +400,9 @@ public Builder stem(Stemmer value) { * Enables {@link Dimension#STEM} through a {@link StemmerFactory}. The analyzer receives a * {@link CachingStemmer} with the {@linkplain CachingStemmer#DEFAULT_CAPACITY default cache * capacity}: it can be shared across threads, and repeated words resolve from a bounded - * per-thread cache instead of being re-stemmed. + * per-thread cache instead of being re-stemmed. The cache is keyed to the thread, so the + * memoization pays off on threads reused across many tokens (a fixed platform-thread pool); + * on a virtual-thread-per-task executor every task starts with an empty cache. * * @param factory The stemmer factory. Must not be {@code null}. * @return this builder diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java index 0fc6f9efbb..6415432f95 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java @@ -40,7 +40,6 @@ void snowballFactoryMatchesDirectStemmer() { StemmerFactory factory = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); Stemmer direct = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); Assertions.assertEquals(direct.stem("running"), factory.newStemmer().stem("running")); - Assertions.assertEquals(direct.stem("running"), factory.stem("running")); } @Test @@ -235,4 +234,98 @@ void sharingPorterStemmerIsSafeUnderConcurrentUse() throws Exception { } } } + @Test + void factoryPassesRepeatThrough() { + // "internationalization" stems differently at repeat 1 and 2, witnessing the pass-through. + final Stemmer once = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH, 1).newStemmer(); + final Stemmer twice = new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH, 2).newStemmer(); + Assertions.assertEquals("internation", once.stem("internationalization").toString()); + Assertions.assertEquals("intern", twice.stem("internationalization").toString()); + Assertions.assertEquals( + new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH, 2).stem("internationalization").toString(), + twice.stem("internationalization").toString()); + } + + @Test + void factoryRejectsNullAlgorithmWithIllegalArgument() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SnowballStemmerFactory(null, 1)); + } + + @Test + void snowballConstructorValidatesLikeTheFactory() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SnowballStemmer(null, 1)); + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH, 0)); + } + + @Test + void sharingStemmerForwardsStemAllToMultiOutputDelegate() { + // A multi-output delegate must keep its full result list through the wrapper. + final StemmerFactory multiOutput = () -> new Stemmer() { + @Override + public CharSequence stem(CharSequence word) { + return word.toString() + "-a"; + } + + @Override + public List stemAll(CharSequence word) { + return List.of(word.toString() + "-a", word.toString() + "-b"); + } + }; + final SharingStemmer sharing = new SharingStemmer(multiOutput); + Assertions.assertEquals(2, sharing.stemAll("run").size()); + Assertions.assertEquals("run-b", sharing.stemAll("run").get(1).toString()); + } + + @Test + void clearThreadLocalStateEmptiesTheOwnerCache() { + // On the owning thread the state object is retained and reset: the cache must be empty + // afterwards, observable as a fresh delegate call for a previously cached word. + final List delegateCalls = new ArrayList<>(); + final StemmerFactory counting = () -> word -> { + delegateCalls.add(word.toString()); + return word.toString() + "-s"; + }; + final CachingStemmer caching = new CachingStemmer(counting); + caching.stem("dog"); + caching.stem("dog"); + Assertions.assertEquals(1, delegateCalls.size(), "second lookup is served from the cache"); + caching.clearThreadLocalState(); + Assertions.assertEquals("dog-s", caching.stem("dog").toString()); + Assertions.assertEquals(2, delegateCalls.size(), "the clear emptied the cache"); + } + + @Test + void clearThreadLocalStateReleasesAWorkerThreadsDelegate() throws Exception { + // On a non-owner thread the per-thread delegate is removed and rebuilt on next use. + final List built = new ArrayList<>(); + final StemmerFactory counting = () -> { + synchronized (built) { + built.add(built.size()); + } + return word -> word.toString() + "-s"; + }; + final SharingStemmer sharing = new SharingStemmer(counting); + sharing.stem("owner"); + final int builtForOwner = built.size(); + final Thread worker = new Thread(() -> { + sharing.stem("dog"); + sharing.stem("cat"); + final int beforeClear = built.size(); + sharing.clearThreadLocalState(); + sharing.stem("fish"); + Assertions.assertEquals(beforeClear + 1, built.size(), + "a fresh delegate is built after the clear"); + }); + worker.start(); + worker.join(30_000); + Assertions.assertTrue(built.size() > builtForOwner, "the worker built its own delegate"); + + final SnowballStemmer snowball = new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH); + Assertions.assertEquals("run", snowball.stem("running").toString()); + snowball.clearThreadLocalState(); + Assertions.assertEquals("run", snowball.stem("running").toString()); + } } diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java index 969261f951..149139847b 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NormalizationProfilesTest.java @@ -19,6 +19,9 @@ import java.util.EnumSet; import java.util.List; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; @@ -207,8 +210,8 @@ void testMatchingAnalyzerIsThreadSafeForStemming() throws Exception { final TermAnalyzer analyzer = NormalizationProfiles.forLanguage("eng").orElseThrow().matchingAnalyzer(); final String expected = analyzer.analyze("running").getFirst().at(Dimension.STEM); - try (var pool = java.util.concurrent.Executors.newFixedThreadPool(8)) { - var tasks = new java.util.concurrent.Future[32]; + try (ExecutorService pool = Executors.newFixedThreadPool(8)) { + final Future[] tasks = new Future[32]; for (int i = 0; i < tasks.length; i++) { tasks[i] = pool.submit(() -> { for (int n = 0; n < 50; n++) { @@ -216,7 +219,7 @@ void testMatchingAnalyzerIsThreadSafeForStemming() throws Exception { } }); } - for (var task : tasks) { + for (final Future task : tasks) { task.get(30, java.util.concurrent.TimeUnit.SECONDS); } } From bfeb0bde343fbfd3946002f0b8b0ff922326d781 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sat, 11 Jul 2026 15:06:51 -0400 Subject: [PATCH 07/12] OPENNLP-1883: Address review: concise contracts, shared thread routing, cache controls The Stemmer and StemmerFactory interfaces state their contracts without implementation or threading claims, and null words throw IllegalArgumentException across the API (Snowball, factory products, caching and sharing wrappers, Porter), pinned by tests. SnowballStemmer now composes SharingStemmer so the per-thread routing pattern lives in one class. CachingStemmer gains clearCache() for the calling thread, interns cached stems through StringInterners, and both stateful classes document the container release path of the *ME components. --- .../java/opennlp/tools/stemmer/Stemmer.java | 18 ++++----- .../opennlp/tools/stemmer/StemmerFactory.java | 18 +++------ .../opennlp/tools/stemmer/CachingStemmer.java | 23 +++++++++-- .../opennlp/tools/stemmer/PorterStemmer.java | 3 ++ .../stemmer/snowball/SnowballStemmer.java | 38 +++++++------------ .../snowball/SnowballStemmerFactory.java | 11 +++--- .../tools/stemmer/StemmerFactoryTest.java | 36 ++++++++++++++++++ 7 files changed, 89 insertions(+), 58 deletions(-) diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java index 30035b59b1..fb6bdcffb6 100644 --- a/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java @@ -4,7 +4,7 @@ * 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 + * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * @@ -22,12 +22,7 @@ /** * Reduces a word to its root form. * - *

Thread safety is implementation-specific: check the implementation's documentation before - * sharing an instance across threads. Stateful engines mutate internal buffers on each - * {@link #stem(CharSequence)} call; when an implementation is not documented as thread-safe, - * share a {@link StemmerFactory} across threads and confine each {@code Stemmer} to one thread, - * or wrap the factory in a thread-local adapter when a single {@code Stemmer} reference must be - * shared.

+ *

Thread safety is implementation specific.

*/ public interface Stemmer { @@ -36,16 +31,17 @@ public interface Stemmer { * * @param word The input word. Must not be {@code null}. * @return The stemmed form. + * @throws IllegalArgumentException Thrown if {@code word} is {@code null}. */ CharSequence stem(CharSequence word); /** - * {@return every stem form for {@code word}} The default returns a single-element list with - * {@link #stem(CharSequence)}. Dictionary-based engines that can produce several root forms for - * one word override this; wrappers that delegate to another {@code Stemmer} must forward this - * method so a multi-output delegate keeps its full result list. + * {@return all stem forms for {@code word}} Defaults to a single-element list from + * {@link #stem(CharSequence)}. Dictionary-based engines may override this to return + * multiple roots; delegating wrappers must forward it to preserve the full list. * * @param word The input word. Must not be {@code null}. + * @throws IllegalArgumentException Thrown if {@code word} is {@code null}. */ default List stemAll(CharSequence word) { return List.of(stem(word)); diff --git a/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java index 2bc86fe751..10e5003901 100644 --- a/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java +++ b/opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java @@ -18,26 +18,18 @@ package opennlp.tools.stemmer; /** - * An immutable, thread-safe factory for {@link Stemmer} instances. - * - *

Stateful stemming engines hold per-call mutable buffers and must not be shared across - * threads. A {@code StemmerFactory} captures the configuration (algorithm, repeat count, - * dictionary path, ...) and mints a fresh {@link Stemmer} on each {@link #newStemmer()} call. The - * factory itself is safe to share; confine each returned {@link Stemmer} to a single thread, or - * route through a thread-local adapter when a component must be shared.

+ * A factory for {@link Stemmer} instances: it captures a stemmer configuration (algorithm, + * repeat count, dictionary path, ...) once and mints configured stemmers on demand. * *

Despite the name, this is not one of the {@code BaseToolFactory}-based tool factories (such * as {@code LemmatizerFactory}) that are instantiated by name from a model manifest. It is a - * plain functional supplier of configured {@link Stemmer} instances and takes no part in the - * model-loading mechanism.

- * - *

Implementations must be immutable and thread-safe after construction.

+ * plain supplier of configured {@link Stemmer} instances and takes no part in the model-loading + * mechanism.

*/ public interface StemmerFactory { /** - * {@return a new {@link Stemmer} confined to the calling thread} Implementations must return a - * distinct instance on each call when the underlying engine is stateful. + * {@return a new {@link Stemmer}} */ Stemmer newStemmer(); } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index c6d944229b..5992f93e22 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -23,6 +23,7 @@ import opennlp.tools.commons.ThreadSafe; import opennlp.tools.util.OwnerOrPerThreadState; +import opennlp.tools.util.jvm.StringInterners; /** * A {@link Stemmer} that memoizes word-to-stem mappings in a bounded per-thread LRU cache. @@ -40,8 +41,10 @@ *

Because the cache is keyed to the thread, the memoization pays off on threads that are * reused across many words, such as a fixed platform-thread pool. On a virtual-thread-per-task * executor every task starts with an empty cache, so repeats are only served within one task. - * Call {@link #clearThreadLocalState()} before returning a pooled thread that should not retain - * its delegate and cache.

+ * As with the thread-safe {@code *ME} components, long-running environments such as application + * containers should call {@link #clearThreadLocalState()} when a pooled thread no longer uses + * this stemmer; otherwise the thread retains its delegate and cache until the instance is + * unreachable, which can pin the defining classloader on redeploys.

* *

{@link #stemAll(CharSequence)} is forwarded to the delegate uncached, so multi-output * engines keep their full result list.

@@ -89,14 +92,18 @@ public CachingStemmer(StemmerFactory factory, int capacity) { @Override public CharSequence stem(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } final ThreadState ts = state.get(); final String key = word.toString(); final String cached = ts.cache.get(key); if (cached != null) { return cached; } - // toString() detaches the result from any buffer the delegate may reuse internally. - final String stemmed = ts.delegate.stem(key).toString(); + // toString() detaches the result from any buffer the delegate may reuse internally, and + // interning stores one instance per distinct stem across all threads' caches. + final String stemmed = StringInterners.intern(ts.delegate.stem(key).toString()); ts.cache.put(key, stemmed); return stemmed; } @@ -115,6 +122,14 @@ public void clearThreadLocalState() { state.clearForCurrentThread(); } + /** + * Empties the calling thread's cache while keeping its delegate, forcing every subsequent + * word through a fresh stemming pass. Only the calling thread's cache is affected. + */ + public void clearCache() { + state.get().cache.clear(); + } + private static final class ThreadState { private final Stemmer delegate; diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java index 623a2d3d2c..7a6416449a 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java @@ -599,6 +599,9 @@ public String stem(String s) { * Returns the result as a CharSequence. */ public CharSequence stem(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } return stem(word.toString()); } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java index 10817c1f8e..08a45c5829 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java @@ -20,22 +20,26 @@ import java.util.function.Supplier; import opennlp.tools.commons.ThreadSafe; +import opennlp.tools.stemmer.SharingStemmer; import opennlp.tools.stemmer.Stemmer; -import opennlp.tools.util.OwnerOrPerThreadState; /** * A {@link Stemmer} backed by the generated Snowball engines. * *

The generated engines hold mutable per-call buffers, so this class routes each call to a - * per-thread engine via {@link OwnerOrPerThreadState}, the same pattern the thread-safe {@code *ME} - * components use. A single {@code SnowballStemmer} instance is therefore safe to share across - * threads.

+ * per-thread engine, the same pattern the thread-safe {@code *ME} components use. A single + * {@code SnowballStemmer} instance is therefore safe to share across threads.

+ * + *

As with the {@code *ME} components, long-running environments such as application + * containers should call {@link #clearThreadLocalState()} when a pooled thread no longer uses + * this stemmer; otherwise the thread retains its engine until the instance is unreachable, + * which can pin the defining classloader on redeploys.

*/ @ThreadSafe public class SnowballStemmer implements Stemmer { - private final OwnerOrPerThreadState delegates; - private final int repeat; + // All per-thread routing is delegated, so the pattern lives in exactly one class. + private final SharingStemmer sharing; /** * Creates a stemmer for the given algorithm and repeat count. @@ -46,15 +50,7 @@ public class SnowballStemmer implements Stemmer { * {@code repeat} is not positive. */ public SnowballStemmer(ALGORITHM algorithm, int repeat) { - if (algorithm == null) { - throw new IllegalArgumentException("algorithm must not be null"); - } - if (repeat <= 0) { - throw new IllegalArgumentException("repeat must be positive, got " + repeat); - } - this.repeat = repeat; - final Supplier engine = engineFor(algorithm); - this.delegates = new OwnerOrPerThreadState<>(engine, stemmer -> { }); + this.sharing = new SharingStemmer(new SnowballStemmerFactory(algorithm, repeat)); } /** @@ -95,15 +91,7 @@ static Supplier engineFor(ALGORITHM algorithm) { @Override public CharSequence stem(CharSequence word) { - final AbstractSnowballStemmer stemmer = delegates.get(); - - stemmer.setCurrent(word.toString()); - - for (int i = 0; i < repeat; i++) { - stemmer.stem(); - } - - return stemmer.getCurrent(); + return sharing.stem(word); } /** @@ -112,7 +100,7 @@ public CharSequence stem(CharSequence word) { * {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. */ public void clearThreadLocalState() { - delegates.clearForCurrentThread(); + sharing.clearThreadLocalState(); } public enum ALGORITHM { diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java index 4deb46ca89..45aa7b25f5 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java @@ -25,11 +25,9 @@ * A thread-safe factory that captures a Snowball stemmer configuration for APIs that accept a * {@link StemmerFactory}. * - *

{@link #newStemmer()} returns a thread-confined stemmer that drives its generated engine - * through a plain field, with none of the per-call thread routing of the shareable - * {@link SnowballStemmer}. This keeps the one-stemmer-per-thread pattern (and per-thread - * delegates inside caching or sharing wrappers) free of that indirection; use - * {@link SnowballStemmer} directly when a single shared instance is wanted instead.

+ *

Use this factory for the classic one-stemmer-per-thread pattern: {@link #newStemmer()} + * returns a plain, thread-confined stemmer without the per-call thread routing of the + * shareable {@link SnowballStemmer}.

* * @param algorithm The Snowball algorithm. Must not be {@code null}. * @param repeat How many times to apply the stemmer per word; must be positive. @@ -81,6 +79,9 @@ private ConfinedStemmer(AbstractSnowballStemmer engine, int repeat) { @Override public CharSequence stem(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } engine.setCurrent(word.toString()); for (int i = 0; i < repeat; i++) { engine.stem(); diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java index 6415432f95..58531f4b2b 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java @@ -24,6 +24,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -328,4 +329,39 @@ void clearThreadLocalStateReleasesAWorkerThreadsDelegate() throws Exception { snowball.clearThreadLocalState(); Assertions.assertEquals("run", snowball.stem("running").toString()); } + + @Test + void nullWordThrowsIllegalArgumentAcrossTheStemmerApi() { + final SnowballStemmerFactory factory = + new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH); + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SnowballStemmer(SnowballStemmer.ALGORITHM.ENGLISH).stem(null)); + Assertions.assertThrows(IllegalArgumentException.class, + () -> factory.newStemmer().stem(null)); + Assertions.assertThrows(IllegalArgumentException.class, + () -> new CachingStemmer(factory).stem(null)); + Assertions.assertThrows(IllegalArgumentException.class, + () -> new SharingStemmer(factory).stem(null)); + Assertions.assertThrows(IllegalArgumentException.class, + () -> new PorterStemmer().stem((CharSequence) null)); + } + + @Test + void clearCacheForcesRestemmingOnTheCallingThread() { + final AtomicInteger delegateCalls = new AtomicInteger(); + final StemmerFactory counting = () -> word -> { + delegateCalls.incrementAndGet(); + return word.toString(); + }; + final CachingStemmer cached = new CachingStemmer(counting); + + cached.stem("running"); + cached.stem("running"); + Assertions.assertEquals(1, delegateCalls.get(), "the repeat is served from the cache"); + + cached.clearCache(); + cached.stem("running"); + Assertions.assertEquals(2, delegateCalls.get(), + "after clearCache() the word goes through the delegate again"); + } } From 97328a1de4f235a9a7c68c8cf342d565ab278ef4 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sun, 12 Jul 2026 08:43:32 -0400 Subject: [PATCH 08/12] OPENNLP-1883: Quality pass: unbounded interner growth and capacity overflow The cached stems are no longer routed through the JVM-wide interner: its default implementation holds strong references forever, so open-vocabulary input would grow without bound and defeat the documented per-thread cache cap. The LRU alone keeps the bound honest. The LRU table sizing is computed in double arithmetic because the int expression overflows to a negative capacity near Integer.MAX_VALUE; a test pins construction at the extreme. clearCache() documents that it initializes state for a thread that has not stemmed yet. --- .../opennlp/tools/stemmer/CachingStemmer.java | 18 +++++++++++------- .../tools/stemmer/StemmerFactoryTest.java | 9 +++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index 5992f93e22..41ebb8ffde 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -23,7 +23,6 @@ import opennlp.tools.commons.ThreadSafe; import opennlp.tools.util.OwnerOrPerThreadState; -import opennlp.tools.util.jvm.StringInterners; /** * A {@link Stemmer} that memoizes word-to-stem mappings in a bounded per-thread LRU cache. @@ -101,9 +100,10 @@ public CharSequence stem(CharSequence word) { if (cached != null) { return cached; } - // toString() detaches the result from any buffer the delegate may reuse internally, and - // interning stores one instance per distinct stem across all threads' caches. - final String stemmed = StringInterners.intern(ts.delegate.stem(key).toString()); + // toString() detaches the result from any buffer the delegate may reuse internally. The + // result is deliberately NOT interned: the JVM-wide interner holds strong references + // forever, which would defeat this class's bounded-memory guarantee on open vocabularies. + final String stemmed = ts.delegate.stem(key).toString(); ts.cache.put(key, stemmed); return stemmed; } @@ -124,7 +124,9 @@ public void clearThreadLocalState() { /** * Empties the calling thread's cache while keeping its delegate, forcing every subsequent - * word through a fresh stemming pass. Only the calling thread's cache is affected. + * word through a fresh stemming pass. Only the calling thread's cache is affected. A thread + * that has not stemmed yet has its state initialized by this call, so invoke it on the + * thread whose cache should be emptied, not from an unrelated maintenance thread. */ public void clearCache() { state.get().cache.clear(); @@ -139,8 +141,10 @@ private ThreadState(Stemmer delegate, int capacity) { this.delegate = delegate; // Access-ordered so iteration order is least-recently-used first. The initial table size // accounts for the 0.75 load factor so a cache filled to capacity never rehashes; the clamp - // keeps the eager allocation reasonable for very large capacities. - this.cache = new LinkedHashMap<>(Math.min((int) (capacity / 0.75f) + 1, 4096), 0.75f, true) { + // keeps the eager allocation reasonable for very large capacities. Sized in double + // arithmetic: the int expression overflows to a negative capacity near Integer.MAX_VALUE. + this.cache = new LinkedHashMap<>((int) Math.min(capacity / 0.75d + 1.0d, 4096.0d), + 0.75f, true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { return size() > capacity; diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java index 58531f4b2b..9a19d01109 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/stemmer/StemmerFactoryTest.java @@ -346,6 +346,15 @@ void nullWordThrowsIllegalArgumentAcrossTheStemmerApi() { () -> new PorterStemmer().stem((CharSequence) null)); } + @Test + void hugeCapacityConstructsAndStems() { + // Pins the overflow-safe table sizing: capacities near Integer.MAX_VALUE are documented + // as legal ("must be positive") and must not wrap the LinkedHashMap initial capacity. + final CachingStemmer cached = new CachingStemmer( + new SnowballStemmerFactory(SnowballStemmer.ALGORITHM.ENGLISH), Integer.MAX_VALUE); + Assertions.assertEquals("run", cached.stem("running").toString()); + } + @Test void clearCacheForcesRestemmingOnTheCallingThread() { final AtomicInteger delegateCalls = new AtomicInteger(); From 2302621051e0235b807f51bb18e5c86105dc22e2 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sun, 12 Jul 2026 16:11:01 -0400 Subject: [PATCH 09/12] OPENNLP-1883: Extract a DelegatingStemmer base to share the per-thread wrapper plumbing --- .../opennlp/tools/stemmer/CachingStemmer.java | 40 +++++------ .../tools/stemmer/DelegatingStemmer.java | 70 +++++++++++++++++++ .../opennlp/tools/stemmer/SharingStemmer.java | 22 ++---- 3 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/DelegatingStemmer.java diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index 41ebb8ffde..c745a42917 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -20,9 +20,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import opennlp.tools.commons.ThreadSafe; -import opennlp.tools.util.OwnerOrPerThreadState; /** * A {@link Stemmer} that memoizes word-to-stem mappings in a bounded per-thread LRU cache. @@ -49,7 +49,7 @@ * engines keep their full result list.

*/ @ThreadSafe -public final class CachingStemmer implements Stemmer { +public final class CachingStemmer extends DelegatingStemmer { /** * Covers the high-frequency vocabulary of most corpora while keeping the per-thread footprint @@ -57,8 +57,6 @@ public final class CachingStemmer implements Stemmer { */ public static final int DEFAULT_CAPACITY = 1024; - private final OwnerOrPerThreadState state; - /** * Creates a caching stemmer with the {@linkplain #DEFAULT_CAPACITY default capacity}. * @@ -78,15 +76,26 @@ public CachingStemmer(StemmerFactory factory) { * not positive. */ public CachingStemmer(StemmerFactory factory, int capacity) { - if (factory == null) { - throw new IllegalArgumentException("factory must not be null"); - } + super(threadStateSupplier(factory, capacity), threadState -> threadState.cache.clear()); + } + + /** + * Validates the constructor arguments eagerly, then returns a supplier that mints one delegate + * and its per-thread cache. Validating here keeps a null factory or non-positive capacity a + * construction-time failure rather than a deferred first-use one. + * + * @param factory The factory that mints one delegate per thread. Must not be {@code null}. + * @param capacity The maximum number of word-to-stem entries kept per thread; must be positive. + * @return a supplier of fresh per-thread state. + * @throws IllegalArgumentException if {@code factory} is {@code null} or {@code capacity} is + * not positive. + */ + private static Supplier threadStateSupplier(StemmerFactory factory, int capacity) { + requireFactory(factory); if (capacity <= 0) { throw new IllegalArgumentException("capacity must be positive, got " + capacity); } - this.state = new OwnerOrPerThreadState<>( - () -> new ThreadState(factory.newStemmer(), capacity), - threadState -> threadState.cache.clear()); + return () -> new ThreadState(factory.newStemmer(), capacity); } @Override @@ -113,15 +122,6 @@ public List stemAll(CharSequence word) { return state.get().delegate.stemAll(word); } - /** - * Removes this thread's delegate and cache to prevent classloader leaks in container - * environments. Call when the thread is returned to a pool or the stemmer is no longer needed, - * mirroring {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. - */ - public void clearThreadLocalState() { - state.clearForCurrentThread(); - } - /** * Empties the calling thread's cache while keeping its delegate, forcing every subsequent * word through a fresh stemming pass. Only the calling thread's cache is affected. A thread @@ -132,7 +132,7 @@ public void clearCache() { state.get().cache.clear(); } - private static final class ThreadState { + static final class ThreadState { private final Stemmer delegate; private final LinkedHashMap cache; diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/DelegatingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/DelegatingStemmer.java new file mode 100644 index 0000000000..5f57fd738d --- /dev/null +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/DelegatingStemmer.java @@ -0,0 +1,70 @@ +/* + * 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.stemmer; + +import java.util.function.Consumer; +import java.util.function.Supplier; + +import opennlp.tools.util.OwnerOrPerThreadState; + +/** + * Shared plumbing for {@link Stemmer} wrappers that route each call to a per-thread payload minted + * from a {@link StemmerFactory}, following the {@link OwnerOrPerThreadState} pattern of the + * thread-safe {@code *ME} components. Subclasses differ only in the payload they carry per thread + * and in what {@link #stem(CharSequence)} does with it. + * + * @param

the per-thread payload: a bare delegate stemmer, or a delegate paired with per-thread + * working state such as a cache. + */ +abstract class DelegatingStemmer

implements Stemmer { + + /** The per-thread payload holder. */ + final OwnerOrPerThreadState

state; + + /** + * @param init Mints the per-thread payload on first use by the owner and by each extra thread. + * @param cleanup Releases a per-thread payload when its thread clears state. + */ + DelegatingStemmer(Supplier

init, Consumer

cleanup) { + this.state = new OwnerOrPerThreadState<>(init, cleanup); + } + + /** + * Validates a factory argument before it is captured by a subclass supplier, so a {@code null} + * factory fails at construction rather than on first use. + * + * @param factory The factory to validate. Must not be {@code null}. + * @return {@code factory}. + * @throws IllegalArgumentException if {@code factory} is {@code null}. + */ + static StemmerFactory requireFactory(StemmerFactory factory) { + if (factory == null) { + throw new IllegalArgumentException("factory must not be null"); + } + return factory; + } + + /** + * Removes this thread's payload to prevent classloader leaks in container environments. Call + * when the thread is returned to a pool or the stemmer is no longer needed, mirroring + * {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. + */ + public void clearThreadLocalState() { + state.clearForCurrentThread(); + } +} diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java index a66d7863dc..af3558f9b8 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java @@ -33,37 +33,23 @@ * thread-safe {@code *ME} components.

*/ @ThreadSafe -public final class SharingStemmer implements Stemmer { - - private final OwnerOrPerThreadState delegates; +public final class SharingStemmer extends DelegatingStemmer { /** * @param factory The factory that mints per-thread delegates. Must not be {@code null}. * @throws IllegalArgumentException if {@code factory} is {@code null}. */ public SharingStemmer(StemmerFactory factory) { - if (factory == null) { - throw new IllegalArgumentException("factory must not be null"); - } - this.delegates = new OwnerOrPerThreadState<>(factory::newStemmer, stemmer -> { }); + super(requireFactory(factory)::newStemmer, stemmer -> { }); } @Override public CharSequence stem(CharSequence word) { - return delegates.get().stem(word); + return state.get().stem(word); } @Override public List stemAll(CharSequence word) { - return delegates.get().stemAll(word); - } - - /** - * Removes this thread's delegate to prevent classloader leaks in container environments. Call - * when the thread is returned to a pool or the stemmer is no longer needed, mirroring - * {@code clearThreadLocalState()} on the thread-safe {@code *ME} components. - */ - public void clearThreadLocalState() { - delegates.clearForCurrentThread(); + return state.get().stemAll(word); } } From 81a0c636d0e7dc00e8eadcf7834b847013aa3620 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sun, 12 Jul 2026 20:46:48 -0400 Subject: [PATCH 10/12] OPENNLP-1883: Tighten javadoc to contracts, document overrides, validate wrapper input Applies the review conventions from the OPENNLP-1869 review: em dashes and history framing leave the benchmark docs, rationale essays shrink to contract sentences, every override carries inheritDoc, and the stemmer wrappers validate their arguments at the boundary. --- opennlp-core/opennlp-runtime/BENCHMARKS.md | 4 +- .../stemmer/CachingStemmerBenchmark.java | 4 +- .../snowball/SnowballStemmerBenchmark.java | 16 ++--- .../opennlp/tools/stemmer/CachingStemmer.java | 60 ++++++++++--------- .../opennlp/tools/stemmer/PorterStemmer.java | 7 ++- .../opennlp/tools/stemmer/SharingStemmer.java | 16 +++++ .../stemmer/snowball/SnowballStemmer.java | 11 +++- .../snowball/SnowballStemmerFactory.java | 14 ++++- 8 files changed, 87 insertions(+), 45 deletions(-) diff --git a/opennlp-core/opennlp-runtime/BENCHMARKS.md b/opennlp-core/opennlp-runtime/BENCHMARKS.md index e3c9ac047b..a059fea896 100644 --- a/opennlp-core/opennlp-runtime/BENCHMARKS.md +++ b/opennlp-core/opennlp-runtime/BENCHMARKS.md @@ -33,7 +33,7 @@ mvn test-compile -Pjmh \ -Dforbiddenapis.skip=true -Dcheckstyle.skip=true # Materialize the test classpath once (JMH's forked JVMs inherit -# java.class.path, which mvn exec:java does not populate — running +# java.class.path, which mvn exec:java does not populate, running # through exec:java fails with ClassNotFoundException: ForkedMain) mvn dependency:build-classpath -pl opennlp-core/opennlp-runtime \ -Pjmh -DincludeScope=test -Dmdep.outputFile=/tmp/cp.txt @@ -47,7 +47,7 @@ java -cp "$CP" org.openjdk.jmh.Main 'opennlp.tools.*.ME*' java -cp "$CP" org.openjdk.jmh.Main POSTaggerMEBenchmark ``` -### Regression testing (stock vs patched) +### Baseline comparison Run the `newInstancePerCall` benchmark on both stock and patched builds. The throughput numbers should be within JMH's error margin. diff --git a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java index 13881b1c06..ae0050b2b0 100644 --- a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java +++ b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/CachingStemmerBenchmark.java @@ -48,10 +48,10 @@ * *

Two workloads drive both strategies:

*
    - *
  • {@code zipf} — a 64k-token stream sampled with 1/rank weights from a 512-word + *
  • {@code zipf}: a 64k-token stream sampled with 1/rank weights from a 512-word * vocabulary. This models real text, where a small vocabulary dominates; the default * 1024-entry cache holds the whole vocabulary.
    • - *
  • {@code diverse} — a 64k-token stream sampled uniformly from an 8192-word vocabulary, + *
  • {@code diverse}: a 64k-token stream sampled uniformly from an 8192-word vocabulary, * 8x the cache capacity. This is the cache-hostile case: mostly misses plus constant * eviction, so it bounds the overhead the cache can add.
    • *
diff --git a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java index e4821aa8c1..62eb09031c 100644 --- a/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java +++ b/opennlp-core/opennlp-runtime/src/jmh/java/opennlp/tools/stemmer/snowball/SnowballStemmerBenchmark.java @@ -39,17 +39,17 @@ import opennlp.tools.stemmer.Stemmer; /** - * JMH benchmark for the thread-safe {@link SnowballStemmer} versus the pre-patch implementation - * that held its generated engine in a plain field. + * JMH benchmark for the thread-safe {@link SnowballStemmer} versus a baseline replica of the + * previous implementation that held its generated engine in a plain field. * *

Three strategies are measured, all at {@link Threads#MAX}:

*
    - *
  • {@code sharedInstance} — one thread-safe {@link SnowballStemmer} shared by every thread + *
  • {@code sharedInstance}: one thread-safe {@link SnowballStemmer} shared by every thread * (one thread runs on the owner fast path, the rest on {@link ThreadLocal} state)
    • - *
  • {@code instancePerThread} — one thread-safe {@link SnowballStemmer} per thread (every + *
  • {@code instancePerThread}: one thread-safe {@link SnowballStemmer} per thread (every * thread is the owner of its own instance, so this isolates the owner-fast-path cost)
    • - *
  • {@code legacyInstancePerThread} — the old non-thread-safe implementation, one per thread - * (the pre-patch baseline; sharing it across threads would be a correctness bug)
    • + *
  • {@code legacyInstancePerThread}: the old non-thread-safe implementation, one per thread + * (the baseline; sharing it across threads would be a correctness bug)
    • *
*/ @BenchmarkMode(Mode.Throughput) @@ -67,8 +67,8 @@ public class SnowballStemmerBenchmark { }; /** - * Replica of the pre-patch {@code SnowballStemmer}: the generated engine lives in a plain - * field, so an instance must not be shared across threads. + * Baseline replica of the previous {@code SnowballStemmer} implementation: the generated engine + * lives in a plain field, so an instance must not be shared across threads. */ static final class LegacySnowballStemmer implements Stemmer { diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index c745a42917..ec83114707 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -27,26 +27,15 @@ /** * A {@link Stemmer} that memoizes word-to-stem mappings in a bounded per-thread LRU cache. * - *

Natural language is Zipf-distributed: a small vocabulary accounts for most tokens, so the - * same words are stemmed over and over. Wrapping a stemmer in a {@code CachingStemmer} turns - * those repeats into a hash lookup instead of a full stemming pass.

- * *

Each thread gets its own delegate stemmer (minted from the supplied {@link StemmerFactory}) - * and its own cache, following the {@link OwnerOrPerThreadState} pattern of the thread-safe - * {@code *ME} components. There is no cross-thread sharing at all, so instances are safe to share - * regardless of whether the factory's stemmers are. Memory is bounded by - * {@code capacity * averageWordLength} characters per thread that stems.

- * - *

Because the cache is keyed to the thread, the memoization pays off on threads that are - * reused across many words, such as a fixed platform-thread pool. On a virtual-thread-per-task - * executor every task starts with an empty cache, so repeats are only served within one task. - * As with the thread-safe {@code *ME} components, long-running environments such as application - * containers should call {@link #clearThreadLocalState()} when a pooled thread no longer uses - * this stemmer; otherwise the thread retains its delegate and cache until the instance is - * unreachable, which can pin the defining classloader on redeploys.

+ * and its own cache, so instances are safe to share regardless of whether the factory's stemmers + * are, and memory is bounded to {@code capacity} entries per thread that stems. Caching pays off + * on threads reused across many words, such as a fixed platform-thread pool; on a + * virtual-thread-per-task executor every task starts with an empty cache, so repeats are served + * only within one task. {@link #stemAll(CharSequence)} is forwarded to the delegate uncached.

* - *

{@link #stemAll(CharSequence)} is forwarded to the delegate uncached, so multi-output - * engines keep their full result list.

+ *

Long-running environments such as application containers should call + * {@link #clearThreadLocalState()} when a pooled thread no longer uses this stemmer.

*/ @ThreadSafe public final class CachingStemmer extends DelegatingStemmer { @@ -80,9 +69,8 @@ public CachingStemmer(StemmerFactory factory, int capacity) { } /** - * Validates the constructor arguments eagerly, then returns a supplier that mints one delegate - * and its per-thread cache. Validating here keeps a null factory or non-positive capacity a - * construction-time failure rather than a deferred first-use one. + * Validates the constructor arguments eagerly and returns a supplier that mints one delegate and + * its per-thread cache, so invalid arguments fail at construction rather than on first use. * * @param factory The factory that mints one delegate per thread. Must not be {@code null}. * @param capacity The maximum number of word-to-stem entries kept per thread; must be positive. @@ -98,6 +86,11 @@ private static Supplier threadStateSupplier(StemmerFactory factory, return () -> new ThreadState(factory.newStemmer(), capacity); } + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code word} is {@code null}. + */ @Override public CharSequence stem(CharSequence word) { if (word == null) { @@ -109,16 +102,23 @@ public CharSequence stem(CharSequence word) { if (cached != null) { return cached; } - // toString() detaches the result from any buffer the delegate may reuse internally. The - // result is deliberately NOT interned: the JVM-wide interner holds strong references - // forever, which would defeat this class's bounded-memory guarantee on open vocabularies. + // toString() detaches the result from any buffer the delegate reuses; the result is not + // interned, which would defeat the bounded-memory guarantee on open vocabularies. final String stemmed = ts.delegate.stem(key).toString(); ts.cache.put(key, stemmed); return stemmed; } + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code word} is {@code null}. + */ @Override public List stemAll(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } return state.get().delegate.stemAll(word); } @@ -137,12 +137,16 @@ static final class ThreadState { private final Stemmer delegate; private final LinkedHashMap cache; + /** + * Creates the per-thread delegate and its access-ordered LRU cache. + * + * @param delegate The stemmer minted for this thread. + * @param capacity The maximum number of entries retained before eviction. + */ private ThreadState(Stemmer delegate, int capacity) { this.delegate = delegate; - // Access-ordered so iteration order is least-recently-used first. The initial table size - // accounts for the 0.75 load factor so a cache filled to capacity never rehashes; the clamp - // keeps the eager allocation reasonable for very large capacities. Sized in double - // arithmetic: the int expression overflows to a negative capacity near Integer.MAX_VALUE. + // Access-ordered for LRU iteration; sized in double arithmetic against the 0.75 load factor + // so a full cache never rehashes, with the clamp bounding the eager allocation. this.cache = new LinkedHashMap<>((int) Math.min(capacity / 0.75d + 1.0d, 4096.0d), 0.75f, true) { @Override diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java index 7a6416449a..aac1c49c38 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/PorterStemmer.java @@ -595,8 +595,11 @@ public String stem(String s) { } /** - * Stem a word provided as a CharSequence. - * Returns the result as a CharSequence. + * Stem a word provided as a CharSequence. Returns the result as a CharSequence. + * + * @param word The word to stem. Must not be {@code null}. + * @return the stemmed word. + * @throws IllegalArgumentException if {@code word} is {@code null}. */ public CharSequence stem(CharSequence word) { if (word == null) { diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java index af3558f9b8..abcd21a1d9 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/SharingStemmer.java @@ -43,13 +43,29 @@ public SharingStemmer(StemmerFactory factory) { super(requireFactory(factory)::newStemmer, stemmer -> { }); } + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code word} is {@code null}. + */ @Override public CharSequence stem(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } return state.get().stem(word); } + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code word} is {@code null}. + */ @Override public List stemAll(CharSequence word) { + if (word == null) { + throw new IllegalArgumentException("word must not be null"); + } return state.get().stemAll(word); } } diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java index 08a45c5829..8d06cc2ad5 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java @@ -62,7 +62,13 @@ public SnowballStemmer(ALGORITHM algorithm) { this(algorithm, 1); } - // Shared with SnowballStemmerFactory, whose products embed one engine directly. + /** + * Returns a supplier of a fresh Snowball engine for the given algorithm, shared with + * {@link SnowballStemmerFactory} whose products embed one engine directly. + * + * @param algorithm The Snowball algorithm. + * @return a supplier that mints a new engine on each call. + */ static Supplier engineFor(ALGORITHM algorithm) { return switch (algorithm) { case ARABIC -> arabicStemmer::new; @@ -89,6 +95,9 @@ static Supplier engineFor(ALGORITHM algorithm) { }; } + /** + * {@inheritDoc} + */ @Override public CharSequence stem(CharSequence word) { return sharing.stem(word); diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java index 45aa7b25f5..393b676338 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java @@ -65,18 +65,28 @@ public Stemmer newStemmer() { return new ConfinedStemmer(SnowballStemmer.engineFor(algorithm).get(), repeat); } - // One engine in a plain field: the zero-indirection product for thread-confined use, per the - // StemmerFactory contract. Not thread-safe. + /** A plain, thread-confined stemmer that applies one Snowball engine directly; not thread-safe. */ private static final class ConfinedStemmer implements Stemmer { private final AbstractSnowballStemmer engine; private final int repeat; + /** + * Creates a thread-confined stemmer over the given engine. + * + * @param engine The Snowball engine to apply. + * @param repeat How many times to apply the engine per word. + */ private ConfinedStemmer(AbstractSnowballStemmer engine, int repeat) { this.engine = engine; this.repeat = repeat; } + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code word} is {@code null}. + */ @Override public CharSequence stem(CharSequence word) { if (word == null) { From e43fa46d844c60a14bf3b98c997300348bcd02be Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Mon, 13 Jul 2026 03:29:26 -0400 Subject: [PATCH 11/12] OPENNLP-1883: Use IllegalArgumentException for null arguments and trim commentary Aligns null-argument validation with the project convention and removes remaining rationale commentary. --- .../src/main/java/opennlp/tools/stemmer/CachingStemmer.java | 6 ++---- .../opennlp/tools/stemmer/snowball/SnowballStemmer.java | 1 - .../opennlp/tools/util/normalizer/NormalizationProfile.java | 5 ++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java index ec83114707..769b01c987 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java @@ -102,8 +102,7 @@ public CharSequence stem(CharSequence word) { if (cached != null) { return cached; } - // toString() detaches the result from any buffer the delegate reuses; the result is not - // interned, which would defeat the bounded-memory guarantee on open vocabularies. + // toString() copies the result out of any buffer the delegate reuses. final String stemmed = ts.delegate.stem(key).toString(); ts.cache.put(key, stemmed); return stemmed; @@ -145,8 +144,7 @@ static final class ThreadState { */ private ThreadState(Stemmer delegate, int capacity) { this.delegate = delegate; - // Access-ordered for LRU iteration; sized in double arithmetic against the 0.75 load factor - // so a full cache never rehashes, with the clamp bounding the eager allocation. + // Access-ordered for LRU eviction. this.cache = new LinkedHashMap<>((int) Math.min(capacity / 0.75d + 1.0d, 4096.0d), 0.75f, true) { @Override diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java index 8d06cc2ad5..21c1613ea8 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java @@ -38,7 +38,6 @@ @ThreadSafe public class SnowballStemmer implements Stemmer { - // All per-thread routing is delegated, so the pattern lives in exactly one class. private final SharingStemmer sharing; /** diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java index d66b402eb9..57f4ea27dc 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/normalizer/NormalizationProfile.java @@ -83,9 +83,8 @@ public Stemmer newStemmer() { * Returns a matching analyzer for this language: NFC, case folding, the language's * {@linkplain #accentFold() diacritic fold} when it has one, then stemming. The returned * analyzer is thread-safe, and repeated words resolve from a bounded per-thread stem cache - * instead of being re-stemmed (natural text is Zipf-distributed, so most tokens are cache - * hits). The cache is keyed to the thread, so build the analyzer once and reuse it from - * threads that live across many calls, such as a fixed platform-thread pool. + * instead of being re-stemmed. The cache is keyed to the thread, so build the analyzer once + * and reuse it from threads that live across many calls, such as a fixed platform-thread pool. * * @return the analyzer. */ From 7fadc1bc9993227626f10f1c085635deb5aaf4da Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Mon, 13 Jul 2026 04:05:52 -0400 Subject: [PATCH 12/12] OPENNLP-1883: Drop the patch-history framing from the benchmark notes --- opennlp-core/opennlp-runtime/BENCHMARKS.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/opennlp-core/opennlp-runtime/BENCHMARKS.md b/opennlp-core/opennlp-runtime/BENCHMARKS.md index a059fea896..21cb71a664 100644 --- a/opennlp-core/opennlp-runtime/BENCHMARKS.md +++ b/opennlp-core/opennlp-runtime/BENCHMARKS.md @@ -13,7 +13,7 @@ variance reporting. | `TokenizerMEBenchmark` | TokenizerME | 3 approaches | | `SentenceDetectorMEBenchmark` | SentenceDetectorME | 3 approaches | | `POSTaggerMEBenchmark` | POSTaggerME | 3 approaches x 2 cache configs | -| `SnowballStemmerBenchmark` | SnowballStemmer | 3 approaches (incl. pre-patch baseline) | +| `SnowballStemmerBenchmark` | SnowballStemmer | 3 approaches (incl. plain-field baseline) | | `CachingStemmerBenchmark` | CachingStemmer | cached vs uncached x 2 workloads | ### Approaches measured @@ -49,29 +49,29 @@ java -cp "$CP" org.openjdk.jmh.Main POSTaggerMEBenchmark ### Baseline comparison -Run the `newInstancePerCall` benchmark on both stock and patched -builds. The throughput numbers should be within JMH's error margin. +Run the `newInstancePerCall` benchmark on both `main` and this +branch. The throughput numbers should be within JMH's error margin. ```bash -# On stock (upstream/main): +# On main: # ... build and run as above, save output -# On patched (feature/thread-safe-me): +# On this branch: # ... build and run as above, compare ``` ### SnowballStemmer results (Linux, JDK 25, 32 cores, 2 forks x 10 iterations) `SnowballStemmerBenchmark` compares the thread-safe `SnowballStemmer` -(engine behind `OwnerOrPerThreadState`) against a replica of the -pre-patch implementation (engine in a plain field, not shareable). +(engine behind `OwnerOrPerThreadState`) against a baseline replica +that keeps the engine in a plain field (not shareable). One op = stemming 16 English words. | Strategy | 1 thread | 8 threads | 32 threads | |----------|---------:|----------:|-----------:| -| `sharedInstance` (patched, one shared stemmer) | 560k ± 3k ops/s | 1.55M ± 0.17M | 3.16M ± 0.34M | -| `instancePerThread` (patched, stemmer per thread) | 509k ± 26k ops/s | 1.60M ± 0.17M | 2.94M ± 0.11M | -| `legacyInstancePerThread` (pre-patch, stemmer per thread) | 544k ± 19k ops/s | 1.46M ± 0.16M | 4.77M ± 0.39M | +| `sharedInstance` (one shared stemmer) | 560k ± 3k ops/s | 1.55M ± 0.17M | 3.16M ± 0.34M | +| `instancePerThread` (stemmer per thread) | 509k ± 26k ops/s | 1.60M ± 0.17M | 2.94M ± 0.11M | +| `legacyInstancePerThread` (plain-field baseline, stemmer per thread) | 544k ± 19k ops/s | 1.46M ± 0.16M | 4.77M ± 0.39M | At 1 and 8 threads the three strategies are within (or nearly within) each other's error bars: the `OwnerOrPerThreadState` lookup is not