uschindler commented on PR #16381: URL: https://github.com/apache/lucene/pull/16381#issuecomment-4933800593
Another observation: It only uses a cache of "known" keys in the map variant. The LongSet is only used to figure out if the key exists (on `get()`), if the key does not exist it returns `null`, otherwise it does the lookup using the normal hash table. The idea behind this is that for StopFilter and similar use-cases have more often lookups of non-existent keys, so we only improve the non existing key (most tokens in a text are NOT stop-words, so most lookups return nothing). The downside of that approach is that looking up a key that exists may take longer. We have two possibilities: - When building the cache use a LongMap and store the value, too. Of course this have additional memory overhead. - Live with the limitation, but this would require users to know of the possible slowdown, so they should not call "freeze" (see above) after populating the map. - Maybe move the optimization only to CharArraySet only. In general I tend to disagree with the added complexity, but as we now have more possible uses of the code I have a slight preference to include it, although we have higher memory requirements (and complexity). Of course the API should be well documented and it should be an opt-in feature of CharArrayMap/Set. Freezing the maps is a good idea anyways to make sure they can't be modified after construction of the set. I don't know yet if I'd like it.... sorry need to think about it more! -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
