markrmiller commented on a change in pull request #230:
URL: https://github.com/apache/solr/pull/230#discussion_r676398512
##########
File path: solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java
##########
@@ -605,80 +605,7 @@ public static UnInvertedField getUnInvertedField(String
field, SolrIndexSearcher
if (cache == null) {
return new UnInvertedField(field, searcher);
}
- AtomicReference<Throwable> throwableRef = new AtomicReference<>();
- UnInvertedField uif = cache.computeIfAbsent(field, f -> {
- UnInvertedField newUif;
- try {
- newUif = new UnInvertedField(field, searcher);
- } catch (Throwable t) {
- throwableRef.set(t);
- newUif = null;
- }
- return newUif;
- });
- if (throwableRef.get() != null) {
- rethrowAsSolrException(field, throwableRef.get());
- }
- return uif;
-
- // (ab) if my understanding is correct this whole block tried to mimic the
Review comment:
It was not likely trying to mimic ConcurrentHashMap#computeIfAbsent, as
it is detrimental to the concurrency of the map due to it's locking. Lot's of
other threads can be accessing the map, typically, also using computeIfAbsent.
That is why the docs say to make value creation short and sweet. Uninverting
fields doesn't really fit that definition. The old impl attempted to use a lock
for every single key, the place holder, so that the thread(s) creating a field
cache entry did not hamper unrelated map access the entire length of value
creation. People have run into ridiculously bad contention with this with
ConcurrentHashMap. The previous approach could be taken because the size of the
map is essentially tiny given it's key/values are large data structures based
on fields. ConcurrentHashMap was designed with requirements that it scale to
very large sizes. Once Caffeine became the cache impl, it's computeIfAbsent
impl has a mitigation that was added that is generally suitable fo
r caching - do a get first and only computeIfAbsent if that returns null.
--
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]
