donnerpeter commented on a change in pull request #2207:
URL: https://github.com/apache/lucene-solr/pull/2207#discussion_r560195271
##########
File path:
lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SpellChecker.java
##########
@@ -24,22 +24,24 @@
*/
public class SpellChecker {
private final Dictionary dictionary;
- private final ThreadLocal<Stemmer> stemmer;
- private final ThreadLocal<BytesRef> scratch =
ThreadLocal.withInitial(BytesRef::new);
+ private final BytesRef scratch = new BytesRef();
- public SpellChecker(Dictionary dictionary) {
+ private SpellChecker(Dictionary dictionary) {
Review comment:
Yes, the threading bugs I've mentioned were related exactly to
morfologik :) I'd prefer to prevent them from happening here.
For data structures, I agree that one shouldn't expect them to be
thread-safe by default, but this isn't a data structure. It's a way to run
queries (a facade) over a thread-safe Dictionary, and I expect such things to
be thread-safe as well. It isn't, but that's an implementation detail, and I
don't see much evil in hiding it for now, given that hiding is cheap and
exposing can easily be done in future.
That said, I'm not totally committed to this approach. I'd estimate the
strength of my desire to hide thread-unsafety as 7 out of 10. How strongly do
you insist on your suggestion? :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
