Hi Claes,
Nice play with CHM and safe publication.
If findSpecies() is on a hot concurrent path, you might want to wrap the
preface:
176 S speciesData = cache.computeIfAbsent(key, new Function<>() {
177 @Override
178 public S apply(K key1) {
179 return newSpeciesData(key1);
180 }
181 });
with a simple:
S speciesData = cache.get(key);
if (speciesData == null) {
speciesData = cache.computeIfAbsent(....);
}
or even use putIfAbsent instead if redundant concurrent newSpeciesData
is OK.
This way you can avoid hot-path synchronization with locks that CHM
always performs in computeIfAbsent.
Regards, Peter
On 04/24/18 18:57, Claes Redestad wrote:
Hi,
the current implementation of ClassSpecializer.findSpecies may cause
excessive blocking due to a potentially expensive computeIfAbsent, and
we have reason to believe this might have been cause for a few very
rare bootstrap issues in tests that attach debuggers to VM in the middle
of this.
Breaking this apart so that code generation and linking is done outside
of the computeIfAbsent helps minimize the risk that a thread gets
interrupted by a debugger at a critical point in time, while also
removing
a few limitations on what we can do from a findSpecies, e.g., look up
other
SpeciesData.
Bug: https://bugs.openjdk.java.net/browse/JDK-8202184
Webrev: http://cr.openjdk.java.net/~redestad/8202184/open.00/
This implementation inserts a placeholder, unresolved SpeciesData,
and then replaces it with another instance that is linked together
fully before publishing it, which ensure safe publication. There might
be alternatives involving volatiles and/or careful fencing, but I've not
been able to measure any added startup cost from this anyway.
Thanks!
/Claes
