On Jul 18, 2012, at 1:43 AM, Aleksey Shipilev wrote: > This feels outright wrong: > > + static Map<String, Data> CACHE = new IdentityHashMap<>(); > I couple of questions pops out in my mind: > 1. Is this code supposed to be thread-safe? It is then wrong to use > unguarded IdentityHashMap without external synchronization. > 2. Do we really need a second .get() check, if code is not thread-safe > anyway? This code allows two threads to call put() on the same key anyway. > 3. Do the $types *need* to be interned? Or is this the premature > optimization (gone wrong, btw)?
You are right on all points. Fixed. > I would rather like to see CHM.putIfAbsent()-based cache on plain > non-interned strings there. Even if interned strings are required, we > could intern them on successful map update, not every time we look up > the key. Yes. Using a IdentityHashMap here is an anti-pattern; sorry to propagate the bad example. Thanks, — John _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
