krickert commented on code in PR #1003:
URL: https://github.com/apache/opennlp/pull/1003#discussion_r3059089227
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/postag/ThreadSafePOSTaggerME.java:
##########
@@ -42,7 +42,11 @@
* @see POSTagger
* @see POSTaggerME
* @see Probabilistic
+ *
+ * @deprecated As of OPENNLP-1816, {@link POSTaggerME} is
+ * itself thread-safe. Use it directly instead.
*/
+@Deprecated(since = "3.0.0")
@ThreadSafe
public class ThreadSafePOSTaggerME implements POSTagger, Probabilistic,
AutoCloseable {
Review Comment:
Thanks for the pointer — the deprecated wrapper already had `AutoCloseable`
for clearing its own `ThreadLocal<POSTaggerME>`.
With `POSTaggerME` / `BeamSearch` now thread-safe, the interesting bit is
the underlying `BeamSearch` instance: that one exposes `close()` as of 90123d6
so callers can `remove()` the beam’s ThreadLocal when they own the lifecycle
(pools, etc.).
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/AdditionalContextFeatureGenerator.java:
##########
Review Comment:
Same story as `CachedFeatureGenerator`: I didn’t add `AutoCloseable` here
because cleanup is really “call `clearAdaptiveData()` / `ThreadLocal#remove()`
on the right thread,” not a single owned handle. If we want API symmetry with
`BeamSearch`, we can sketch a follow-up, but I didn’t want to half-implement
`close()` without a clear contract for nested generators.
--
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]
