[9, 8u40] RFR (M): 8057020: LambdaForm caches should support eviction

[Vladimir Ivanov]

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