Closed pomadchin closed 5 years ago
Since you aren't using a bounding, the speedup compared to ConcurrentHashMap
must be due to lock contention. Caffeine is built on top of CHM, but performs an optimistic lookup on a computeIfAbsent
(which Cache.get(key, function)
delegates to).
In Java 8 the computeIfAbsent
is pessimistic by always taking the hashbin lock, even if the entry is present. That can be really bad at scale (see compute benchmark). Based on this, JDK9 switched to a lock-free check of the hashbin's first entry. If that is for the requested key then it returns, otherwise it locks to evaluate the entries. I haven't benchmarked, but I'm sure it improved it partially.
The hesitation for a full prescreen, like Caffeine, is because at 32+ cores the performance diverges to be a benefit. Doug theorized it is because of GC safepoints and biased locking, which are general problems the JVM is addressing. Since the cache strives to have hot entries (else a poor hit rate) on server-class machines, we can make a much simpler choice rather than think through everyone's map scenarios on various machine sizes.
So if you didn't want the dependency you could simply mimic it as,
var value = map.get(key);
if (value != null) {
return value;
}
return map.computeIfAbsent(key, mappingFunction);
@ben-manes thanks for the insights, I didn't realize that. I think this will work out good for our needs. Tbh I don't think, that there is a need in a separate Caching library, since there would be not a lot of data, and even in the worst case it would not be bad.
If interested, here is Doug's reply on this optimization. The remaining discussion in that thread was wrt a different issue, recursive computations, which was also improved in JDK9.
@ben-manes I double checked the behavior on bounded and unbounded Caffeine
and on the ConcurrentHashMap
in c9b25e9:
[info] Benchmark Mode Cnt Score Error Units
[info] CRSBench.getEpsgCodeTest avgt 3 0.599 ± 7.173 us/op
[info] CRSBench.logicalEqualsFalse avgt 3 1.175 ± 9.221 us/op
[info] CRSBench.logicalEqualsTrue avgt 3 2.537 ± 25.793 us/op
[info] CRSBench.logicalVarEqualsFalse avgt 3 0.940 ± 4.540 us/op
[info] CRSBench.logicalVarEqualsTrue avgt 3 1.133 ± 14.138 us/op
[info] CRSBench.resolveCRS avgt 3 0.893 ± 2.728 us/op
I would say that I'll rollback this commit. Also at this point we can't use JDK 9/10/11 everywhere. However thanks a lot for this extra helpful information.
oh interesting! yeah, rollback. I really don't know why Caffeine is better but I'm never unhappy when that's true :)
@ben-manes woooot I looked into the old terminal window; look at these results, you were right and I'm super happy that it aligns with the information you wrote:
[info] Benchmark Mode Cnt Score Error Units
[info] CRSBench.getEpsgCodeTest avgt 3 0.064 ± 0.161 us/op
[info] CRSBench.logicalEqualsFalse avgt 3 0.039 ± 0.009 us/op
[info] CRSBench.logicalEqualsTrue avgt 3 0.035 ± 0.020 us/op
[info] CRSBench.logicalVarEqualsFalse avgt 3 0.040 ± 0.068 us/op
[info] CRSBench.logicalVarEqualsTrue avgt 3 0.037 ± 0.034 us/op
[info] CRSBench.resolveCRS avgt 3 0.034 ± 0.008 us/op
So definitely c9b25e9 would be the thing. Thanks again! :+1:
haha, no problem. Not too often I pipe in to recommend not using my library 😉
@ben-manes maestro 💯 :D
This PR addresses https://github.com/locationtech/geotrellis/issues/3022 and https://github.com/locationtech/geotrellis/issues/2890
Notes
Benchmarking results: