On 09/02/2014 03:59 PM, Vladimir Ivanov wrote:

Webrev: http://cr.openjdk.java.net/~vlivanov/8057042/webrev.00

Best regards, Vladimir Ivanov

Hi Vladimir, In LambdaFormEditor, where Transform[] is promoted into ConcurrentHashMap<Transform, Transform>: 339 ConcurrentHashMap<Transform, Transform> m = new ConcurrentHashMap<>(MAX_CACHE_ARRAY_SIZE * 2); 340 for (Transform k : ta) { 341 if (k != null) continue; 342 m.put(k, k); 343 } 344 lambdaForm.transformCache = m; 345 // The second iteration will update for this query, concurrently. 346 continue; I think line 341 is wrong. It should be: if (k == null) break; shouldn't it? Regards, Peter

On 9/2/14, 5:57 PM, Vladimir Ivanov wrote:

...

https://bugs.openjdk.java.net/browse/JDK-8057042

LambdaFormEditor provides an API to transform LambdaForms. Deriving new LambdaForms from a base one allows to cache and reuse results of repeated transformations.

BMH binding is rewritten to use LambdaFormEditor.

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}.

Reviewed-by: vlivanov, ? Contributed-by: john.r.rose@oracle.com

Thanks!

Best regards, Vladimir Ivanov _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

---

mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

On 09/03/2014 03:25 PM, Vladimir Ivanov wrote:

Peter,

Thanks for the feedback.

...

In LambdaFormEditor, where Transform[] is promoted into ConcurrentHashMap<Transform, Transform>:

339 ConcurrentHashMap<Transform, Transform> m = new ConcurrentHashMap<>(MAX_CACHE_ARRAY_SIZE * 2); 340 for (Transform k : ta) { 341 if (k != null) continue; 342 m.put(k, k); 343 } 344 lambdaForm.transformCache = m; 345 // The second iteration will update for this query, concurrently. 346 continue;

I think line 341 is wrong. It should be:

if (k == null) break;

shouldn't it? Good catch! Fixed.

I think it can even be removed or replaced with something like: assert k != null; ...since null entry in array is not possible in this situation - promotion to CHM happens only when array is full. Regards, Peter

Best regards, Vladimir Ivanov

...

Regards, Peter

...

On 9/2/14, 5:57 PM, Vladimir Ivanov wrote:

...

https://bugs.openjdk.java.net/browse/JDK-8057042

LambdaFormEditor provides an API to transform LambdaForms. Deriving new LambdaForms from a base one allows to cache and reuse results of repeated transformations.

BMH binding is rewritten to use LambdaFormEditor.

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}.

Reviewed-by: vlivanov, ? Contributed-by: john.r.rose@oracle.com

Thanks!

Best regards, Vladimir Ivanov _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

---

mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

_______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Hi Vladimir, I'm sure you and John have thought about it through, but I'll ask anyway. Are cached LambdaForms going to stay around forever? What about using a WeakReference<LambdaForm> (LambdaFormEditor.Transform could extend WeakReference). This way unused LambdaForms would get GCed. Regards, Peter On 09/02/2014 03:59 PM, Vladimir Ivanov wrote:

Webrev: http://cr.openjdk.java.net/~vlivanov/8057042/webrev.00

Best regards, Vladimir Ivanov

On 9/2/14, 5:57 PM, Vladimir Ivanov wrote:

...

https://bugs.openjdk.java.net/browse/JDK-8057042

LambdaFormEditor provides an API to transform LambdaForms. Deriving new LambdaForms from a base one allows to cache and reuse results of repeated transformations.

BMH binding is rewritten to use LambdaFormEditor.

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}.

Reviewed-by: vlivanov, ? Contributed-by: john.r.rose@oracle.com

Thanks!

Best regards, Vladimir Ivanov _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

---

mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

On 09/03/2014 03:29 PM, Vladimir Ivanov wrote:

Peter,

Yes, it's a known problem [1]. There are other caches (in MethodTypeForm, for example), which shouldn't retain their elements.

Hi Vladimir, I was tempted to see what it would take to use weakly referenced LambdaForms from cache entries (LambdaFormEditor.Transform objects). This is what I came up with: http://cr.openjdk.java.net/~plevart/misc/LambdaFormEditor.WeakCache/webrev.0... In this modification on top of your patch, a reference to LambdaForm from Transform is not a final field any more (WeakReference has a normal field), so I had to change LambdaForm.transformCacheto be volatile field to enable safe publication of Transform objects and transiently LambdaForm objects. I also used ordered writes with volatile reads for Transform[] array elements where necessary. CHM already does what's necessary. If LambdaForm objects are unsafe-publication-tolerable, this can be simplified. I have made a little effort to re-use slots occupied by cleared Transform objects, but nothing fancy like using ReferenceQueue(s) or such, since there would have to be a queue per LambdaForm and this would bring some overhead with it. Transform objects are very compact (even when they extend a WeakReference) and majority of heap is released by free-ing weakly reachable LambdaForm objects which can be quite big and deep sometimes. A cache forms a tree of LambdaForm objects. Child LFs are derived from base (parent) LFs. parent -> child references are weak, but I added child -> parent references which are strong. If there is a strong reference to some cached LF from the application, the whole path leading from the root LF to the cached LF is kept alive this way, so that any code that wishes to follow that path can get to the cached LF. So do you think that cached LambdaForm(s) are so general that they are better cached for the life of JVM even in long-running application servers that re-deploy apps on the fly and using WeakReference(s) is not necessary? Regards, Peter

Best regards, Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8057020

On 9/3/14, 3:43 PM, Peter Levart wrote:

...

Hi Vladimir,

I'm sure you and John have thought about it through, but I'll ask anyway. Are cached LambdaForms going to stay around forever? What about using a WeakReference<LambdaForm> (LambdaFormEditor.Transform could extend WeakReference). This way unused LambdaForms would get GCed.

Regards, Peter

On 09/02/2014 03:59 PM, Vladimir Ivanov wrote:

...

Webrev: http://cr.openjdk.java.net/~vlivanov/8057042/webrev.00

Best regards, Vladimir Ivanov

On 9/2/14, 5:57 PM, Vladimir Ivanov wrote:

...

https://bugs.openjdk.java.net/browse/JDK-8057042

LambdaFormEditor provides an API to transform LambdaForms. Deriving new LambdaForms from a base one allows to cache and reuse results of repeated transformations.

BMH binding is rewritten to use LambdaFormEditor.

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}.

Reviewed-by: vlivanov, ? Contributed-by: john.r.rose@oracle.com

Thanks!

Best regards, Vladimir Ivanov _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

---

mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

On Sep 2, 2014, at 3:59 PM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:

Webrev: http://cr.openjdk.java.net/~vlivanov/8057042/webrev.00

Generally looks good (re: Peter's observation of continue/break). - LambdaFormEditor 61 private static final class Transform { 62 final long packedBytes; 63 final byte[] fullBytes; 64 final LambdaForm result; // result of transform, or null, if there is none available 65 66 private enum Kind { More an oddity really, Transform.Kind is private but exposed via package private methods. 187 static Transform of(Kind k, int b1, byte[] b234) { It might be worthwhile investing in a unit test for LambdaFormEditor to exercise the transform types and transitions, otherwise edge cases might bite us later. I was thinking Transform could be greatly simplified if one just uses byte[], but IIUC (via JOL) that increases the instance size by 16 bytes, although that may be small compared to the LambdaForm result and if say a WekRef is also used to hold that result. Plus if that is the case the transformCache could be simplified by just supporting Transform[] and ConcurrentHashMap. I guess the underlying question i am asking here is do these space savings really give us much over some less complicated code? Paul.

On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:

Paul, thanks for review.

...

Generally looks good (re: Peter's observation of continue/break).

- LambdaFormEditor

61 private static final class Transform { 62 final long packedBytes; 63 final byte[] fullBytes; 64 final LambdaForm result; // result of transform, or null, if there is none available 65 66 private enum Kind {

More an oddity really, Transform.Kind is private but exposed via package private methods. Good point. I'll fix that.

+1 on review.

...

187 static Transform of(Kind k, int b1, byte[] b234) {

It might be worthwhile investing in a unit test for LambdaFormEditor to exercise the transform types and transitions, otherwise edge cases might bite us later.

Tests for JEP 210 (SQE is working on them) should cover that.

Ok.

...

I was thinking Transform could be greatly simplified if one just uses byte[], but IIUC (via JOL) that increases the instance size by 16 bytes, although that may be small compared to the LambdaForm result and if say a WekRef is also used to hold that result. Plus if that is the case the transformCache could be simplified by just supporting Transform[] and ConcurrentHashMap. I guess the underlying question i am asking here is do these space savings really give us much over some less complicated code? All these specializations can be considered overoptimizations, but I'd prefer to leave them. Complexity is manageable, encapsulated, and localized ([1],[2]).

And these specializations are for the common case. The numbers I got for Octane shows that: (1) large transforms are very rare: 98-99% of Transforms fit into packed representation; (2) for LF caches (a) 70-80% are single-element; (b) 20-30% fit into array (<16 elements) (c) CHM is needed very rarely (<<1%)

OK, good to see some numbers on this, quite reassuring. Paul.