Peter,
First, just a nit. I think that in LambdaFormEditor:
289 private LambdaForm putInCache(Transform key, LambdaForm form) {
290 key = key.withResult(form);
291 for (int pass = 0; ; pass++) {
292 Object c = lambdaForm.transformCache;
293 if (c instanceof ConcurrentHashMap) {
294 @SuppressWarnings("unchecked")
295 ConcurrentHashMap<Transform,Transform> m =
(ConcurrentHashMap<Transform,Transform>) c;
296 Transform k = m.putIfAbsent(key, key);
297 if (k == null) return form;
298 LambdaForm result = k.get();
299 if (result != null) {
300 return result;
301 } else {
302 if (m.replace(key, k, key)) {
303 return form;
304 } else {
305 continue;
306 }
307 }
308 }
309 assert(pass == 0);
310 synchronized (lambdaForm) {
311 c = lambdaForm.transformCache;
312 if (c instanceof ConcurrentHashMap)
313 continue;
...
372 lambdaForm.transformCache = c = m;
^^^ put assignment to 'c'
back in
373 // The second iteration will update for this query,
concurrently.
374 continue;
...you could move the assignment to 'c' in line 292 out of for loop and
put it back in line 372, since once 'c' is instance of CHM,
lambdaForm.transformCache never changes again and if 'c' is not CHM yet,
it is re-assigned in lines 311 and 372 before next loop.
Am I right?
Yes, it's correct. I decided to keep the code as-is to avoid
complicating the code even more - while working on the fix I traced c
usages at least twice :-)
Now what scares me (might be that I don't have an intimacy with
LambdaForm class like you do). There is a situation where you publish
LambdaForm instances via data race.
One form of LambdaForm.transformCache is an array of Transform objects
(the other two forms are not problematic). Transform class has all
fields final except the 'referent' field of SoftReference, which holds a
LambdaForm instance. In the following line:
377 ta[idx] = key;
...you publish Transform object to an element of array with relaxed
write, and in the following lines:
271 } else {
272 Transform[] ta = (Transform[])c;
273 for (int i = 0; i < ta.length; i++) {
274 Transform t = ta[i];
275 if (t == null) break;
276 if (t.equals(key)) { k = t; break; }
277 }
278 }
279 assert(k == null || key.equals(k));
280 return (k != null) ? k.get() : null;
...you obtain the element of the array with no synchronization and a
relaxed read and might return a non-null referent (the LambdaForm) which
is then returned as an interned instance.
So can LambdaForm instances be published via data races without fear
that they would appear half-initialized?
That's what I didn't know when I used a lazySet coupled with volatile
get to access array elements in my version:
http://cr.openjdk.java.net/~plevart/misc/LambdaFormEditor.WeakCache/webrev.01/
As Paul already wrote, LambdaForms are safe to be published via a data
race, since it's structure is stored in final fields. LambdaForm cache
in MethodTypeForm is built on that property. For the reference, we had
discussed this aspect before (scattered in [1]).
Best regards,
Vladimir Ivanov
[1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-May/013902.html
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
