krickert commented on code in PR #1003:
URL: https://github.com/apache/opennlp/pull/1003#discussion_r3059023627
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java:
##########
@@ -21,62 +21,90 @@
import java.util.ArrayList;
import java.util.List;
+import opennlp.tools.commons.ThreadSafe;
import opennlp.tools.util.Cache;
/**
* Caches features of the aggregated {@link AdaptiveFeatureGenerator
generators}.
+ * <p>
+ * The cache is maintained per-thread via {@link ThreadLocal}, making this
class safe for
+ * concurrent use from multiple threads. Each thread gets its own independent
cache that is
+ * cleared when a new sentence (token array) is encountered.
+ * <p>
+ * <b>Note:</b> In container environments with classloader isolation (e.g.
Jakarta EE),
+ * {@link ThreadLocal} state may pin the classloader. Ensure instances do not
outlive
+ * the application's lifecycle, or call {@link ThreadLocal#remove()} on pooled
threads.
*
* @see Cache
*/
+@ThreadSafe
public class CachedFeatureGenerator implements AdaptiveFeatureGenerator {
+ /**
+ * System property to disable the feature cache globally.
+ * Set to {@code "true"} to bypass caching (useful for benchmarking).
+ */
+ public static final String DISABLE_CACHE_PROPERTY =
"opennlp.featuregen.cache.disabled";
+
private final AdaptiveFeatureGenerator generator;
+ private final boolean cacheEnabled;
- private String[] prevTokens;
+ private final ThreadLocal<CacheState> threadState =
+ ThreadLocal.withInitial(() -> new CacheState(new Cache<>(100)));
- private final Cache<Integer, List<String>> contextsCache;
+ private static final class CacheState {
+ private String[] prevTokens;
+ private final Cache<Integer, List<String>> cache;
- private long numberOfCacheHits;
- private long numberOfCacheMisses;
+ CacheState(Cache<Integer, List<String>> cache) {
+ this.cache = cache;
+ }
+ }
+ /**
+ * @deprecated Use {@link #CachedFeatureGenerator(AdaptiveFeatureGenerator)}
instead.
+ */
@Deprecated
public CachedFeatureGenerator(AdaptiveFeatureGenerator... generators) {
this.generator = new AggregatedFeatureGenerator(generators);
- contextsCache = new Cache<>(100);
+ this.cacheEnabled = !Boolean.getBoolean(DISABLE_CACHE_PROPERTY);
}
public CachedFeatureGenerator(AdaptiveFeatureGenerator generator) {
this.generator = generator;
- contextsCache = new Cache<>(100);
+ this.cacheEnabled = !Boolean.getBoolean(DISABLE_CACHE_PROPERTY);
}
@Override
public void createFeatures(List<String> features, String[] tokens, int index,
String[] previousOutcomes) {
+ if (!cacheEnabled) {
Review Comment:
Good call. The fast path (`!cacheEnabled`) is unchanged, and the cached path
is now in a proper `else` so the structure reads clearly. Landed in 3a43f71.
--
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]
