krickert commented on code in PR #1163:
URL: https://github.com/apache/opennlp/pull/1163#discussion_r3564856669
##########
opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java:
##########
@@ -17,10 +17,37 @@
package opennlp.tools.stemmer;
+import java.util.List;
+
/**
- * The stemmer is reducing a word to its stem.
+ * Reduces a word to its root form.
+ *
+ * <p>Thread safety is implementation-specific: check the implementation's
documentation before
Review Comment:
Done
##########
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
Review Comment:
No, reverted.
##########
opennlp-api/src/main/java/opennlp/tools/stemmer/Stemmer.java:
##########
@@ -17,10 +17,37 @@
package opennlp.tools.stemmer;
+import java.util.List;
+
/**
- * The stemmer is reducing a word to its stem.
+ * Reduces a word to its root form.
+ *
+ * <p>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.</p>
*/
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
Review Comment:
Done, used your text verbatim.
##########
opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.
Review Comment:
Done.
##########
opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ *
+ * <p>Stateful stemming engines hold per-call mutable buffers and must not be
shared across
Review Comment:
Done.
##########
opennlp-api/src/main/java/opennlp/tools/stemmer/StemmerFactory.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ *
+ * <p>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.</p>
+ *
+ * <p>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.</p>
+ *
+ * <p>Implementations must be immutable and thread-safe after construction.</p>
+ */
+public interface StemmerFactory {
+
+ /**
+ * {@return a new {@link Stemmer} confined to the calling thread}
Implementations must return a
Review Comment:
Done.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmer.java:
##########
@@ -17,68 +17,85 @@
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.
Review Comment:
Done, here and on CachingStemmer, mirroring the *ME wording.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/snowball/SnowballStemmerFactory.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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 opennlp.tools.commons.ThreadSafe;
+import opennlp.tools.stemmer.Stemmer;
+import opennlp.tools.stemmer.StemmerFactory;
+
+/**
+ * A thread-safe factory that captures a Snowball stemmer configuration for
APIs that accept a
+ * {@link StemmerFactory}.
+ *
+ * <p>{@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.</p>
+ *
+ * @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 IllegalArgumentException Thrown if {@code algorithm} is {@code
null} or
+ * {@code repeat} is not positive.
+ */
+ public SnowballStemmerFactory {
+ if (algorithm == null) {
+ throw new IllegalArgumentException("algorithm must not be null");
+ }
+ if (repeat <= 0) {
+ throw new IllegalArgumentException("repeat must be positive, got " +
repeat);
+ }
+ }
+
+ @Override
+ 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.
+ 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());
Review Comment:
Done, guard added and tested.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/stemmer/CachingStemmer.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.
+ *
+ * <p>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.</p>
+ *
+ * <p>Each thread gets its own delegate stemmer (minted from the supplied
{@link StemmerFactory})
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
