Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-16 Thread Vladimir Ivanov
Peter, thank you very much for experimenting with that! It looks really promising. I do believe we need to purge LambdaForm cache, until there's no upper limit on a possible number of LambdaForm instance. I'll gather some data how efficient a cache with weak keys is and get back to you.

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-10 Thread Peter Levart
On 09/03/2014 03:25 PM, Vladimir Ivanov wrote: Peter, Thanks for the feedback. In LambdaFormEditor, where Transform[] is promoted into ConcurrentHashMapTransform, Transform: 339 ConcurrentHashMapTransform, Transform m = new ConcurrentHashMap(MAX_CACHE_ARRAY_SIZE * 2);

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-10 Thread Vladimir Ivanov
Peter, 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

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-10 Thread Peter Levart
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

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Remi Forax
On 09/03/2014 07:46 PM, John Rose wrote: On Sep 3, 2014, at 10:35 AM, Mark Roos mr...@roos.com wrote: From Morris All that assert laden code is nice to see. I just finished watching a video from Doug Lea where he mentioned that having asserts can inhibit inlining due to the

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Paul Sandoz
On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov vladimir.x.iva...@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

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Thomas Wuerthinger
This is why Graal’s inlining heuristics are not based on the number of bytecodes, but the complexity of the compiler graph after applying canonicalisation. Adding asserts to the bytecodes should not influence peak performance when they are disabled. Same for expressing the same logic with a

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Vladimir Ivanov
Paul, Peter, Morris, thanks for review. Best regards, Vladimir Ivanov On 9/5/14, 3:51 PM, Paul Sandoz wrote: On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: Paul, thanks for review. Generally looks good (re: Peter's observation of continue/break). -

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Marcus Lagergren
Totally agree on that bytecode based metrics are evil. On 05 Sep 2014, at 14:32, Thomas Wuerthinger thomas.wuerthin...@oracle.com wrote: This is why Graal’s inlining heuristics are not based on the number of bytecodes, but the complexity of the compiler graph after applying

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Morris Meyer
Remi, That assert in a static method could be pulled out to a static block. Regarding the asserts in the LambdaForms code, my feeling is that in code that is still in the process of being refactored, that they are critical to maintain integrity. After the dust settles, creating a debugging

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-05 Thread Remi Forax
On 09/05/2014 04:07 PM, Morris Meyer wrote: Remi, That assert in a static method could be pulled out to a static block. Regarding the asserts in the LambdaForms code, my feeling is that in code that is still in the process of being refactored, that they are critical to maintain integrity.

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-04 Thread Paul Sandoz
On Sep 2, 2014, at 3:59 PM, Vladimir Ivanov vladimir.x.iva...@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

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Peter Levart
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 ConcurrentHashMapTransform, Transform: 339

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Peter Levart
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 WeakReferenceLambdaForm (LambdaFormEditor.Transform could extend WeakReference). This way unused LambdaForms would get GCed.

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Vladimir Ivanov
Peter, Thanks for the feedback. In LambdaFormEditor, where Transform[] is promoted into ConcurrentHashMapTransform, Transform: 339 ConcurrentHashMapTransform, Transform m = new ConcurrentHashMap(MAX_CACHE_ARRAY_SIZE * 2); 340 for (Transform k : ta)

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Vladimir Ivanov
Peter, Yes, it's a known problem [1]. There are other caches (in MethodTypeForm, for example), which shouldn't retain their elements. 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

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Morris Meyer
src/share/classes/java/lang/invoke/BoundMethodHandle.java Could we keep /* */ comment style consistent throughout? @Override // there is a default binder in the super class, for 'L' types only /*non-public*/ src/share/classes/java/lang/invoke/LambdaForm.java ! Object

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread Mark Roos
From Morris All that assert laden code is nice to see. I just finished watching a video from Doug Lea where he mentioned that having asserts can inhibit inlining due to the additional byte codes. So he sadly does not use them due to performance issues. Does anyone have any insights

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread John Rose
On Sep 3, 2014, at 10:35 AM, Mark Roos mr...@roos.com wrote: From Morris All that assert laden code is nice to see. I just finished watching a video from Doug Lea where he mentioned that having asserts can inhibit inlining due to the additional byte codes. So he sadly does

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-03 Thread John Rose
On Sep 3, 2014, at 8:46 AM, Morris Meyer morris.me...@oracle.com wrote: src/share/classes/java/lang/invoke/BoundMethodHandle.java Could we keep /* */ comment style consistent throughout? @Override // there is a default binder in the super class, for 'L' types only

[9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-02 Thread Vladimir Ivanov
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:

Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF

2014-09-02 Thread Vladimir Ivanov
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