jaisonbi commented on PR #816: URL: https://github.com/apache/lucene/pull/816#issuecomment-1110760113
> Actually the implementation does not look wrong. I mostly like that it removes the `ThreadLocal<WeakReference<T>>`. > > The problem now is that every call goes through the WeakHashMap with a lock around. The original implementation was always first trying the native thread local (almost lock-free) and only if the value disappeared it was checking the hard references (with a lock). So in most cases getting the value was fast (because the value is still on the thread local). > > It would be better to use a ConcurrentHashMap to store the values, but there exists now weak version of it, so the keys (threads) in the map do not disappear automatically once thread has died. We considered "CloseableThreadLocal#get" and "CloseableThreadLocal#set" as low-frequency calls, so choose WeakHashMap. Ya, ConcurrentHashMap would be a better approach, we can trigger "purge" more frequently to remove the died threads. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
