On 07/22/2012 03:45 AM, John Rose wrote: > On Jul 18, 2012, at 1:43 AM, Aleksey Shipilev wrote: > Yes.
Thanks John. I'm having a glance over the fix in new webrev, and this
feels even worse:
+ private static SpeciesData get(String types) {
+ // Acquire cache lock for query.
+ SpeciesData d = lookupCache(types);
+ if (!d.isPlaceholder())
+ return d;
+ Class<? extends BoundMethodHandle> cbmh;
+ synchronized (d) {
+ // Use synch. on the placeholder to prevent multiple
instantiation of one species.
+ // Creating this class forces a recursive call to
getForClass.
+ cbmh = Factory.generateConcreteBMHClass(types);
+ }
+ // Reacquire cache lock.
+ d = lookupCache(types);
+ // Class loading must have upgraded the cache.
+ assert(d != null && !d.isPlaceholder());
+ return d;
+ }
+ static SpeciesData getForClass(String types, Class<? extends
BoundMethodHandle> clazz) {
+ // clazz is a new class which is initializing its
SPECIES_DATA field
+ return updateCache(types, new SpeciesData(types, clazz));
+ }
+ private static synchronized SpeciesData lookupCache(String types) {
+ SpeciesData d = CACHE.get(types);
+ if (d != null) return d;
+ d = new SpeciesData(types);
+ assert(d.isPlaceholder());
+ CACHE.put(types, d);
+ return d;
+ }
+ private static synchronized SpeciesData updateCache(String types,
SpeciesData d) {
+ SpeciesData d2;
+ assert((d2 = CACHE.get(types)) == null || d2.isPlaceholder());
+ assert(!d.isPlaceholder());
+ CACHE.put(types, d);
+ return d;
+ }
Global synchronization is the performance smell, and this looks to be
potential scalability bottleneck (it sends shivers down my spine every
time I see "static synchronized" in the same line. That is not to
mention synchronizing on placeholder looks suspicious and error-prone.
Do you need help rewriting this with CHM?
-Aleksey.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
