Hi, This looks good. Took me a while to understand the interactions: you need to replace not update otherwise there is a race on isResolved (which currently queries multiple state, there is no singular guard here). We could push isResolved into the synchronized block and simplify but every findSpecies call may take a small hit (or there are potentially other ways looking more closely at how ClassSpecializer.Factory initializes state, i.e. a fenced static field write, but we go further down the rabbit hole)
I think this might fix some rare and intermittent recursive exceptions from ConcurrentHashMap cache we have been observing, where a collision occurs on keys for recursive updates (rather than for the same key). Paul. > On Apr 24, 2018, at 9:57 AM, Claes Redestad <claes.redes...@oracle.com> 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
