Re: Getting back into indy...need a better argument collector!

[Paul Sandoz]

Hi Charlie,

Thanks for the details. I quickly logged:

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

I don’t have time to dive into the details right now. Perhaps next week, or 
hopefully someone else can.

Paul.

> On Mar 25, 2021, at 9:25 PM, Charles Oliver Nutter  
> wrote:
> 
> JRuby branch with changes to use our own collector methods:
> https://github.com/jruby/jruby/pull/6630
> 
> InvokeBinder 1.2 added collect(index, type, collector) that calls
> MethodHandles.collectArguments:
> https://github.com/headius/invokebinder/commit/9650de07715c6e15a8ca4029c40ea5ede9d5c4c9
> 
> A build of JRuby from the branch (or from jruby-9.2 branch or master
> once it is merged) compared with JRuby 9.2.16.0 should show the issue.
> Benchmark included in the PR above.
> 
> On Thu, Mar 25, 2021 at 8:43 PM Charles Oliver Nutter
>  wrote:
>> 
>> After experimenting with MethodHandles.collectArguments (given a
>> hand-written collector function) versus my own logic (using folds and
>> permutes to call my collector), I can confirm that both are roughly
>> equivalent and better than MethodHandle.asCollector.
>> 
>> The benchmark linked below calls a lightweight core Ruby method
>> (Array#dig) that only accepts an IRubyObject[] (so all arities must
>> box). The performance of collectArguments is substantially better than
>> asCollector.
>> 
>> https://gist.github.com/headius/28343b8c393e76c717314af57089848d
>> 
>> I do not believe this should be so. The logic for asCollector should
>> be able to gather up Object subtypes into an Object[] subtype without
>> an intermediate array or extra copying.
>> 
>> On Thu, Mar 25, 2021 at 7:39 PM Charles Oliver Nutter
>>  wrote:
>>> 
>>> Well it only took me five years to circle back to this but I can
>>> confirm it is just as bad now as it ever was. And it is definitely due
>>> to collecting a single type.
>>> 
>>> I will provide whatever folks need to investigate but it is pretty
>>> straightforward. When asking for asCollector of a non-Object[] type,
>>> the implementation will first gather arguments into an Object[], and
>>> then create a copy of that array as the correct type. So two arrays
>>> are created, values are copied twice.
>>> 
>>> I can see this quite clearly in the assembly after letting things
>>> optimize. A new Object[] is created and populated, and then a second
>>> array of the correct type is created followed by an arraycopy
>>> operation.
>>> 
>>> I am once again backing off using asCollector directly to instead
>>> provide my own array-construction collector.
>>> 
>>> Should be easy to reproduce the perf issues simply by doing an
>>> asCollector that results in some subtype of Object[].
>>> 
>>> On Thu, Jan 14, 2016 at 8:18 PM Charles Oliver Nutter
>>>  wrote:
 
 Thanks Duncan. I will try to look under the covers this evening.
 
 - Charlie (mobile)
 
 On Jan 14, 2016 14:39, "MacGregor, Duncan (GE Energy Management)" 
  wrote:
> 
> On 11/01/2016, 11:27, "mlvm-dev on behalf of MacGregor, Duncan (GE Energy
> Management)"  duncan.macgre...@ge.com> wrote:
> 
>> On 11/01/2016, 03:16, "mlvm-dev on behalf of Charles Oliver Nutter"
>> 
>> wrote:
>> ...
>>> With asCollector: 16-17s per iteration
>>> 
>>> With hand-written array construction: 7-8s per iteration
>>> 
>>> A sampling profile only shows my Ruby code as the top items, and an
>>> allocation trace shows Object[] as the number one object being
>>> created...not IRubyObject[]. Could that be the reason it's slower?
>>> Some type trickery messing with optimization?
>>> 
>>> This is very unfortunate because there's no other general-purpose way
>>> to collect arguments in a handle chain.
>> 
>> I haven¹t done any comparative benchmarks in that area for a while, but
>> collecting a single argument is a pretty common pattern in the Magik 
>> code,
>> and I had not seen any substantial difference when we last touched that
>> area. However we are collecting to plain Object[] so it might be that is
>> the reason for the difference. If I¹ve got time later this week I¹ll do
>> some experimenting and check what the current situation is.
> 
> Okay, I’ve now had a chance to try this in with our language benchmarks
> and can’t see any significant difference between a hand crafted method and
> asCOllector, but we are dealing with Object and Object[], so it might be
> something to do with additional casting.
> 
> Duncan.
> 
> ___
> 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
> https://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net

Re: Performance of non-static method handles

[Paul Sandoz]

At some point in the future it may be possible, with the constant folding work, 
to express the declaration of a MH locally but it gets stuffed in the constant 
pool (see amber constant-folding) if what the MH is derived from is constant. 
e.g. think of a language compiler intrinsic for ldc. That may be improve some 
use-cases but if any input is not constant we are back to the slower path. 

Paul.

> On Feb 2, 2018, at 5:03 AM, Remi Forax  wrote:
> 
> Hi Charles,
> usually, it's because a non constant method handle is not inlined into the 
> callsite,
> so it's as fast as a function call or a method call when you ask to not 
> inline.
> 
> A way to improve the perf is to profile the method handles that can be seen 
> when doing an invokeExact,
> and inline them if they are few of them, making invokeExact acts as a 
> n-morphic inlining cache (with an identity check instanceof a class check).
> 
> Obviously, it's also easy to emulate think kind of cache with an 
> invokedynamic, i think Golo has such cache (Golo lambdas are plain method 
> handle),
> and if you want to go fully circular, you can simulate invokedynamic with an 
> invokeExact on a constant method handle :)   
> 
> see you tomorrow,
> Rémi
> 
> - Mail original -
>> De: "John Rose" 
>> À: "Da Vinci Machine Project" 
>> Envoyé: Vendredi 2 Février 2018 13:33:49
>> Objet: Re: Performance of non-static method handles
> 
>> Vladimir Ivanov did some work a few years ago on MH customization for hot MH
>> instances. It’s in the system. That should get better results than what you
>> show. I wonder why it isn’t kicking in. You are using invokeExact right?
>> 
>>> On Feb 2, 2018, at 1:26 PM, Charles Oliver Nutter  
>>> wrote:
>>> 
>>> Hey folks!
>>> 
>>> I'm running some simple benchmarks for my FOSDEM handles talk and wanted to
>>> reopen discussion about the performance of non-static-final method handles.
>>> 
>>> In my test, I just try to call a method that adds given argument to a static
>>> long. The numbers for reflection and static final handle are what I'd 
>>> expect,
>>> with the latter basically being equivalent to a direct call:
>>> 
>>> Direct: 0.05ns/call
>>> Reflected: 3ns/call
>>> static final Handle: 0.05ns/call
>>> 
>>> If the handle is coming from an instance field or local variable, however,
>>> performance is only slightly faster than reflection. I assume the only real
>>> improvement in this case is that it doesn't box the long value I pass in.
>>> 
>>> local var Handle: 2.7ns/call
>>> 
>>> What can we do to improve the performance of non-static method handle
>>> invocation?
>>> 
>>> - Charlie
>>> ___
>>> 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

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

Re: LinearProbeHashtable Re: ClassValue perf?

[Paul Sandoz]

Hi Peter,

> On 27 May 2016, at 12:41, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> Hi Paul,
> 
> On 05/26/2016 01:20 PM, Paul Sandoz wrote:
>> Hi Peter,
>> 
>> Opportunistically if your LinearProbeHashtable works out then i am wondering 
>> if we could replace the use of CHM within 
>> MethodType.ConcurrentWeakInternSet, which only uses get/putIfAbsent/remove.
>> 
>> Thereby CHM can use VarHandles without inducing a circular dependency.
>> 
>> Paul.
> 
> It could be used, yes. LinearProbeHashtable is not scalable to multiple 
> threads like CHM is for modifications as it is using a single lock for all 
> modification operations including rehashing, but it is lock-free for lookups, 
> so for usecases such as caching, where lookups dominate and modifications are 
> mostly performed in batches from single thread (when some subsystem 
> initializes), it could be a viable alternative to CHM.
> 

Or say expunging of stale entries?


> If it is moved to some jdk.internal subpackage and made public, I could add 
> missing Map methods, mostly to be able to include it in MOAT tests.
> 
> What do you think?
> 

I think moving to say jdk.internal.util.concurrent makes sense. Keeping it lean 
and focused to the purpose it currently serves seems appropriate, so 
implementing Map might an unwanted embellishment, i would be inclined to write 
some focused tests and also leverage the contextual testing via it’s use within 
ClassValue (and maybe MethodType).

I should investigate updating MethodType and running the Octane benchmark...

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR(L): 8139885: implement JEP 274: enhanced method handles

[Paul Sandoz]

> On 19 Nov 2015, at 22:16, Michael Haupt  wrote:
> 
> Hi John, Paul,
> 
> thanks for your reviews - they have helped me polish the code and 
> documentation some more and add some more tests. The result is at 
> http://cr.openjdk.java.net/~mhaupt/8139885/webrev.02 
> 
> 

I quickly browsed it. Looks ok.


> I noticed both of you emphasised how the Streams API helps the implementation 
> - I second that. Streams "made my day" there. :-)
> 

:-)


>> 3458  * The loop handle's result type is the same as the sole loop 
>> variable's, i.e., the result type of {@code init}.
>> 
>> s/variable’s/variable ?
> 
> No; the genitive is intentional (result type = loop variable type, rather 
> than result type = loop variable).
> 

Doh, yes, now that I read it again.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR(L): 8139885: implement JEP 274: enhanced method handles

[Paul Sandoz]

HI Michael,

Nice work, not easy this area!


It may be worth considering, as a separate issue, updating the example section 
in JavaDoc to be under @apiNote.

I have a mild preference to maintain the 80 char limit for JavaDoc, to me it’s 
easier to read. For code i don’t mind a 100 or more limit.

Add @since 9 to new public methods.


MethodType
—

 470 /** Replace the last arrayLength parameter types with the component 
type of arrayType.
 471  * @param arrayType any array type
 472  * @param arrayLength the number of parameter types to change
 473  * @return the resulting type
 474  */

 475 /*non-public*/ MethodType asSpreaderType(Class arrayType, int pos, 
int arrayLength) {

 476 assert(parameterCount() >= arrayLength);

 477 int spreadPos = pos;

Might as well adjust the JavaDoc for the new pos parameter (as you have done 
for asCollectorType).


MethodHandles
—

 898  * @param spreadArgPos the position (zero-based index) in the argument 
list at which spreading should start.
 899  * @param arrayType usually {@code Object[]}, the type of the array 
argument from which to extract the spread arguments
 900  * @param arrayLength the number of arguments to spread from an 
incoming array argument
 901  * @return a new method handle which spreads an array argument at a 
given position,
 902  * before calling the original method handle
 903  * @throws NullPointerException if {@code arrayType} is a null 
reference
 904  * @throws IllegalArgumentException if {@code arrayType} is not an 
array type,
 905  * or if target does not have at least
 906  * {@code arrayLength} parameter types,
 907  * or if {@code arrayLength} is negative,
 908  * or if the resulting method handle's type would have
 909  * too many parameters

For the new asSpreader method, what are the constraints for spreadArgPos? What 
happens if < 0 or >= arrayLength for example?

Same applies to asCollector.

This is not in your patch but the assert in the following method is redundant 
(as reported by the IDE, it’s really handy sometimes apply the patch and view 
via diffs in the IDE), as DirectMethodHandle is not a subtype of 
BoundMethodHandle:

MethodHandle viewAsType(MethodType newType, boolean strict) {
// No actual conversions, just a new view of the same method.
// Note that this operation must not produce a DirectMethodHandle,
// because retyped DMHs, like any transformed MHs,
// cannot be cracked into MethodHandleInfo.
assert viewAsTypeChecks(newType, strict);
BoundMethodHandle mh = rebind();
assert(!((MethodHandle)mh instanceof DirectMethodHandle));
return mh.copyWith(newType, mh.form);
}


 958 /**
 959  * Determines if a class can be accessed from the lookup context 
defined by this {@code Lookup} object. The
 960  * static initializer of the class is not run.
 961  *
 962  * @param targetClass the class to be accessed
 963  *
 964  * @return the class that has been accessed

Has it really been accessed by “accessClass” or just checked that it can be 
accessed?


 965  *
 966  * @throws IllegalAccessException if the class is not accessible 
from the lookup class, using the allowed access
 967  * modes.
 968  * @exception SecurityException if a security manager is present 
and it
 969  *  refuses access
 970  */
 971 public Class accessClass(Class targetClass) throws 
IllegalAccessException {



Great use of streams in MethodHandles.loop and good correlation with the 
specified steps in the JavaDoc to that in the code. Really clear and well 
specified.



3458  * The loop handle's result type is the same as the sole loop 
variable's, i.e., the result type of {@code init}.

s/variable’s/variable ?


3488  * // assume MH_initZip, MH_zipPred, and MH_zipStep are handles to the 
above methods
3489  * MethodHandle loop = MethodHandles.doWhileLoop(MH_initZip, 
MH_zipPred, MH_zipStep);
3490  * List a = Arrays.asList(new String[]{"a", "b", "c", "d"});
3491  * List b = Arrays.asList(new String[]{"e", "f", "g", "h"});
3492  * List zipped = Arrays.asList(new String[]{"a", "e", "b", 
"f", "c", "g", "d", "h"});
3493  * assertEquals(zipped, (List) loop.invoke(a.iterator(), 
b.iterator()));
3494  * }

You don’t need to create an explicit array and pass that into Arrays.asList. 
Same for other related methods.



MethodHandleImpl
—

1720 static MethodHandle makeLoop(Class tloop, List targs, 
List tvars, List init,
1721  List step, 
List pred, List fini) {
1722 MethodHandle[] ainit = 
toArrayArgs(init).toArray(MH_ARRAY_SENTINEL);
1723 MethodHandle[] astep = 
toArrayArgs(step).toArray(MH_ARRAY_SENTINEL);
1724 MethodHandle[] apred = 

Re: JFokus VM Tech Day 2016

[Paul Sandoz]

> On 12 Nov 2015, at 02:37, John Rose  wrote:
> 
> On Nov 11, 2015, at 12:35 AM, Marcus Lagergren  wrote:
>> 
>> bare silicone magic
> 
> That extra E moves the venue from Santa Clara to Las Vegas.

LOL :-)

"So now, less than five years later, you can go up on a steep hill in Las Vegas 
and look West, and with the right kind of eyes you can almost see the 
high-water mark—that place where the wave finally broke and rolled back.”

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR: JDK-8136893: Improve early java.lang.invoke infrastructure initialization

[Paul Sandoz]

> On 25 Sep 2015, at 17:19, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> Hi Paul,
> 
> Thanks for looking into this.
> 

Apologies for the late reply.


> On 09/25/2015 04:07 PM, Paul Sandoz wrote:
>> Hi,
>> 
>> This looks like a partial dup of 
>> https://bugs.openjdk.java.net/browse/JDK-8076596
> 
> Ah, sorry, I wasn't aware this has already been registered in JIRA. I have 
> linked the two issues together as DUPs.
> 

Thanks.


>> 
>> The changes look ok, but I am concerned post initialization there may be 
>> code paths taken that require the system class loader to be used but instead 
>> the boot stream class loader is used instead. Is that a legitimate concern?
> 
> It is legitimate, but I have inspected usages and:
> 
> - In java.lang.invoke.BoundMethodHandle.Factory#makeCbmhCtor, null is passed 
> explicitly and this method is used only from 
> java.lang.invoke.BoundMethodHandle.Factory#makeCtors which is used from 
> java.lang.invoke.BoundMethodHandle.SpeciesData#initForBootstrap and 
> java.lang.invoke.BoundMethodHandle.SpeciesData#SpeciesData(java.lang.String, 
> java.lang.Class). These usages 
> only deal with erased MH types (Object, long, int, double, float).
> 
> - In java.lang.invoke.MemberName#getMethodType, the result of 
> MemberName.getClassLoader() is passed to the method. This is the class loader 
> of the member's declaring class. Any types referenced from the member 
> declaration should be resolvable from this class loader. A member declared by 
> a bootstrap class (MemberName.getClassLoader() returns null) can only 
> reference types resolvable from bootstrap loader.
> 
> - In java.lang.invoke.MemberName#getFieldType, the result of 
> MemberName.getClassLoader() is passed to the method. Similar applies as above.
> 
> - In java.lang.invoke.MethodHandleNatives#fixMethodType(Class callerClass, 
> Object type), the callerClass.getClassLoader() is passed to the method. This 
> is invoked from:
> java.lang.invoke.MethodHandleNatives#linkMethodImpl(
> Class callerClass, int refKind,
> Class defc, String name, Object type,
> Object[] appendixResult)
> which is called from java.lang.invoke.MethodHandleNatives#linkMethod(same 
> args)
> 
> I suppose this is an upcall from VM to link a call to the 
> @PolymorphicSignature method and callerClass is the class in which the 
> invocation bytecodes are executed. Am I right?

Yes.

And before that there is an upcall to MethodHandleNatives.findMethodHandleType, 
see SystemDictionary::find_method_handle_invoker in 
src/share/vm/classfile/systemDictionary.cpp.

AFAICT the “type” argument passed to the linkMethod should always be of 
MethodType:

static MemberName linkMethod(Class callerClass, int refKind,
 Class defc, String name, Object type,
 Object[] appendixResult) {
if (!TRACE_METHOD_LINKAGE)
return linkMethodImpl(callerClass, refKind, defc, name, type, 
appendixResult);
return linkMethodTracing(callerClass, refKind, defc, name, type, 
appendixResult);
}
static MemberName linkMethodImpl(Class callerClass, int refKind,
 Class defc, String name, Object type,
 Object[] appendixResult) {
try {
if (defc == MethodHandle.class && refKind == REF_invokeVirtual) {
return Invokers.methodHandleInvokeLinkerMethod(name, 
fixMethodType(callerClass, type), appendixResult);
}
} catch (Throwable ex) {
if (ex instanceof LinkageError)
throw (LinkageError) ex;
else
throw new LinkageError(ex.getMessage(), ex);
}
throw new LinkageError("no such method "+defc.getName()+"."+name+type);
}
private static MethodType fixMethodType(Class callerClass, Object type) {
if (type instanceof MethodType)
return (MethodType) type;
else
return MethodType.fromMethodDescriptorString((String)type, 
callerClass.getClassLoader());
}

Thus it seems fixMethodType is not necessary. I think this is confirmed when 
looking at up calls to linkMethodHandleConstant and linkCallSite.


> The reasoning is as follows: if callerClass.getClassLoader() is sufficient 
> for resolving types that appear at the call site in a non-bootstrap 
> callerClass, then bootstrap classloader should be sufficient to resolve types 
> that appear at the call site in a bootstrap callerClass. Does anybody have 
> any other opinion?
> 
> - sun.invoke.util.BytecodeDescriptor#parseMethod(java.lang.String, 
> java.lang.ClassLoader) is only used from the newly introduced 
> java.lang.invoke.MethodType#fromDescriptor
> 

Ok, i t

Re: RFR: JDK-8136893: Improve early java.lang.invoke infrastructure initialization

[Paul Sandoz]

Hi,

This looks like a partial dup of 
https://bugs.openjdk.java.net/browse/JDK-8076596

The changes look ok, but I am concerned post initialization there may be code 
paths taken that require the system class loader to be used but instead the 
boot stream class loader is used instead. Is that a legitimate concern?

Paul.

On 25 Sep 2015, at 15:37, Michael Haupt  wrote:

> Hi Peter,
> 
> thanks for this changeset. Note I'm not a Reviewer (with a capital R); please 
> read this review in lower-case. ;-)
> 
> One question about MethodType: would you mind doing something about the 
> naming of the newly introduced fromDescriptor() method? Its name does not 
> suggest in any way that it chooses the class loader differently. The name is 
> subtly different from that of fromDescriptorString(), and the signature is 
> identical - it's probably really easy to confuse the two when working at the 
> core libs level. Unfortunately, the only proposal for a name I can make, 
> fromDescriptorStringBootCL(), is clunky. Maybe that's acceptable for a 
> low-level method only visible at package level.
> 
>> Am 25.09.2015 um 08:46 schrieb Peter Levart :
>> I have run the tests in java.lang.invoke and only have a problem with 1 test 
>> which seems to be related to the test or jtreg itself and happens also 
>> without my patch applied:
>> 
>> #Test Results (version 2)
>> #Tue Sep 22 09:48:38 CEST 2015
>> ...
>> #section:script_messages
>> --messages:(0/0)--
>> 
>> 
>> test result: Error. Parse Exception: `@library' must appear before first 
>> `@run'
> 
> Yes. The test is also marked as ignored until another issue is fixed, so that 
> can be ignored.
> 
> Other than the above remark/suggestion, this looks fine. I'll defer to an 
> upper-case Reviewer, though.
> 
> Best,
> 
> Michael
> 
> --
> 
> 
> Dr. Michael Haupt | Principal Member of Technical Staff
> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
> Oracle Java Platform Group | LangTools Team | Nashorn
> Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany
> Oracle is committed to developing 
> practices and products that help protect the environment
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (XS): 8059455: LambdaForm.prepare() does unnecessary work for cached LambdaForms

[Paul Sandoz]

On Apr 23, 2015, at 5:12 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 https://bugs.openjdk.java.net/browse/JDK-8059455
 http://cr.openjdk.java.net/~vlivanov/8059455/webrev.00/
 
 LambdaForm.compileToBytecode() does unnecessary work (constructs invokerType 
  check an assertion) before ensuring that LambdaForm hasn't been already 
 compiled. It happens very often for cached LambdaForms.
 
 The fix is to do the check first.
 

+1

Paul.

 Testing: failed VM tests (timeouts)
 
 Best regards,
 Vladimir Ivanov



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: Implementing VarHandle

[Paul Sandoz]

Hi Peter, 

We did consider supporting field and method literals in 9, leveraging the same 
syntax as for method references combined with target typing. But, we have 
currently concluded it would be best to punt it to post-9. 

As a result there is currently no compelling need to support VarHandles in the 
constant pool, which, while not particular hard AFAICT (famous last words!), is 
a welcome reduction in work.

Paul.

On Apr 20, 2015, at 10:41 AM, Peter Levart peter.lev...@gmail.com wrote:
 The thing that pushed us over the edge is that value types are coming.  With 
 value types, one can create type-safe, zero-cost, specialized wrappers for 
 {Static,Instance,Array,Native}VarHandleT that wrap an underlying VH; 
 because these wrappers can be values, they can provide type safety with no 
 indirection or footprint costs.  So it seemed better to provide a simple, 
 clean, low-level API now that doesn’t make any assumptions, let the early 
 adopters (mostly us) deal with the fact that type safety comes at runtime 
 (just as with MHs), and later provide a clean set of value wrappers on top 
 of it.  
 
 This seems like a good plan for post-JDK9 times. But I still miss one thing 
 in this picture - the syntax. If purely API approach is taken, then we will 
 still be using Strings to identify fields and do the caching of VarHandles 
 ourselves. Are there any plans for specifying syntax for constant 
 [Method|Var] handles in Java or is this being reserved for post-JDK9 times 
 where the syntax will be used to produce type-safe wrappers (similar to 
 approach taken with MethodHandles vs. Lambdas)?
 
 Regards, Peter
 


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: Implementing VarHandle

[Paul Sandoz]

On Apr 20, 2015, at 4:40 PM, Remi Forax fo...@univ-mlv.fr wrote:
 
 On 04/20/2015 11:06 AM, Paul Sandoz wrote:
 Hi Peter, 
 
 We did consider supporting field and method literals in 9, leveraging the 
 same syntax as for method references combined with target typing. But, we 
 have currently concluded it would be best to punt it to post-9. 
 
 As a result there is currently no compelling need to support VarHandles in 
 the constant pool, which, while not particular hard AFAICT (famous last 
 words!), is a welcome reduction in work.
 
 Paul.
 
 You don't even need to have VarHandle in the constant pool,
 once you have invokedynamic, you can create any constant you want at runtime,

Yes, good point (ignoring pesky bootstrap issues with CHM etc). 

When we were pondering field/method literals we also pondered about the 
translation to byte code, and naturally indy came up. This led onto whether 
constant dynamic (John's idea and term), a derived form of indy for 
constants, might be worth investigating.

Paul.


 MethodHandles are in the constant pool only because you need it to reference 
 the bootstrap method of an invokedynamic, you need it to bootstrap 
 invokedynamic if you prefer.
 
 That's not fully true because we may also want to have a way to represent 
 constant fields in annotation.
 This give me an idea, invokedynamic should be used to specify the constant 
 values in annotation.
 It will be extensible for the JDK and any (dynamic) languages that have a 
 more powerful annotation mechanism
 (Groovy anyone?) will be able to leverage that.
 
 At runtime, an annotation will still be a proxy but instead of using 
 java.lang.reflect.Proxy,
 it should use the Proxy2 API [1] :)
 
 Rémi
 [1] https://github.com/forax/proxy2
 
 
 
 On Apr 20, 2015, at 10:41 AM, Peter Levart peter.lev...@gmail.com wrote:
 The thing that pushed us over the edge is that value types are coming.  
 With value types, one can create type-safe, zero-cost, specialized 
 wrappers for {Static,Instance,Array,Native}VarHandleT that wrap an 
 underlying VH; because these wrappers can be values, they can provide type 
 safety with no indirection or footprint costs.  So it seemed better to 
 provide a simple, clean, low-level API now that doesn’t make any 
 assumptions, let the early adopters (mostly us) deal with the fact that 
 type safety comes at runtime (just as with MHs), and later provide a clean 
 set of value wrappers on top of it.  
 This seems like a good plan for post-JDK9 times. But I still miss one thing 
 in this picture - the syntax. If purely API approach is taken, then we will 
 still be using Strings to identify fields and do the caching of VarHandles 
 ourselves. Are there any plans for specifying syntax for constant 
 [Method|Var] handles in Java or is this being reserved for post-JDK9 times 
 where the syntax will be used to produce type-safe wrappers (similar to 
 approach taken with MethodHandles vs. Lambdas)?
 
 Regards, Peter
 
 
 
 ___
 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



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: Implementing VarHandle

[Paul Sandoz]

Hi Remi,

I am always impressed by the ability of MHs, and your ability to use them :-)

With the API you have defined in the VarHandle2 class located in j.l.invoke i 
strongly suspect you do not need to use method handles. The public methods 
could directly use casts/null-checks and Unsafe. Roland and Vladimir (K.) fixed 
issues with null-check droppings by making Class.cast an intrinsic (which 
iIIRC gave some benchmarks a little perf boost), so if the class is constant 
the cast should just fold away without leaving any trace.

It's statically type safe for the receiver but not for the value, a compromise 
to reduce the number of handle classes. Ideally what you want is VarHandle2T, 
V, but until valhalla arrives V is problematic. I tried push this as far as i 
could with a FieldHandleT, V where, say, Integer really meant int, but 
ultimately it was an awkward fit and there was a risk it would not be right 
when valhalla is ready so we decided not to pursue that direction.


A VarHandle (as currently designed) is a cross product of variable location, 
variable type and access mode.

The single class can support variable locations that are instance fields, 
static fields, array elements, and even off-heap array elements (perhaps 
indexed by long rather than int, and/or hooked up to direct byte buffers). For 
the former two, access modes work independently of whether the field is 
volatile or not (access is not possible if a field is final). I also want to 
support relaxed and volatile access for all primitive types (so the scope of 
supported variable types is the same as that for DHMs to fields).

I tried to design the implementation so there are minimal dependencies (as you 
point out in a later email CHM is problematic) and so that a minimal amount of 
work is performed by the runtime compiler, when say inlining accesses in 
complex j.u.c code. (It's feasible the implementation could use LambdaForms, or 
could change as possible advances are made to hotspot and the invoke classes.)

It's definitely not designed for every day developer use (as i think is also 
the case for MHs). It's focus is advanced developers currently using Unsafe.

You can find some live patches against hs-rt here:

  http://cr.openjdk.java.net/~psandoz/jdk9/varhandles/

Paul.

On Apr 12, 2015, at 4:54 PM, Remi Forax fo...@univ-mlv.fr wrote:

 Hi guys,
 I was about to write a blog post explaining why i don't like the way 
 VarHandle are currently implemented when it occurs to me that providing 
 another implementation may be a more efficient to discuss about 
 implementation.
 
 So my implementation is here,
  https://github.com/forax/varhandle2
 
 the API is typesafe, the implementation is runtime safe and i get mostly the 
 same assembly code using unsafe or this VarHandle API.
 
 The idea is that there is no need to add more polymorphic signature methods, 
 a good old Java method is enough if it using a method handle under the hood.
 All the logic is described using method handles and the only logic written in 
 Java is
 1) when to create such method handle (the first time a method is called),
it works like invokedynamic but the method handles are stable instead of 
 being constant.
 2) how to link all things together to do nullcheck, atomic operation and post 
 operation (like fence or sum).
 
 cheers,
 Rémi
 
 
 



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: The curious case of MHS.Lookup.unreflect on MethodHandle.invoke/invokeExact

[Paul Sandoz]

On Mar 17, 2015, at 8:24 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul,
 A call to the following:
 
   Object o = rmh.invokeExact((MethodHandle) null, new Object[]{});
 
 Will result in a:
 
   java.lang.UnsupportedOperationException: cannot reflectively invoke 
 MethodHandle
 
 However, the stack trace corresponds to the stack where the call to 
 unreflect was performed and not where the invocation occurs.
 The reason is that the preconstructed exception is thrown and not created on 
 every invocation:
  mh = mh.bindTo(new UnsupportedOperationException(cannot reflectively invoke 
 MethodHandle));
 

Yes.


 Further it does mh.withInternalMemberName(method, false), that i cannot 
 explain. Why do we need to re-associate the MH throwing the USO with the 
 member name corresponding to the MH.invokeExact/invoke method?
 I think the main reason is to keep direct method handle cracking API 
 (MethodHandles.revealDirect()) working for MethodHandle.invoke*.
 
 Actual method handle structure in this case is more complex than a simple 
 DMH, so additional trick with WrappedMember is needed to preserve an illusion 
 an ordinary direct method handle is returned.
 

Ah, i see, i had my suspicions in might be something to do with that.


 For such edge cases perhaps caching is not required.
 Agree, caching shouldn't be important for such cases.
 

Perhaps such method handles were originally cached to avoid an explosion 
(deliberate or otherwise) of class generation of LFs, but now there is more 
sophisticated LF caching in place this is not necessary.?

Thanks,
Paul.




signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (XS): 8073644: Assertion in LambdaFormEditor.bindArgumentType is too strict

[Paul Sandoz]

On Feb 26, 2015, at 7:14 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8073644/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8073644
 
 After JDK-8069591 [1] which introduced LambdaForm customization, the assert 
 in LambdaFormEditor.bindArgumentType became too strict.
 
 The fix is to relax it - compare uncustomized versions. And 
 LambdaFormEditor.lambdaForm is always uncustomized.
 
 Testing: java/lang/invoke, nashorn tests
 

+1

I needed to remind myself of the dual nature of LambdaForm.transformCache :-)
 
Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8067344: Adjust java/lang/invoke/LFCaching/LFGarbageCollectedTest.java for recent changes in java.lang.invoke

[Paul Sandoz]

On Jan 12, 2015, at 7:06 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 Paul,
 
 Thanks for the review!
 

Look good, +1,
Paul.

 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8067344/webrev.02
 
   70 TestMethods testCase = getTestMethod();
   71 if (testCase == TestMethods.EXACT_INVOKER || testCase == 
 TestMethods.INVOKER) {
   72 // Invokers aren't collected.
   73 return;
   74 }
 
 Can you just filter those test cases out in the main method within 
 EnumSet.complementOf?
 Good point! Done.
 
   82 mtype = adapter.type();
   83 if (mtype.parameterCount() == 0) {
   84 // Ignore identity_* LambdaForms.
   85 return;
   86 }
 
 Under what conditions does this arise? i guess it might be 
 non-determinisitic based on the randomly generated arity for the test case, 
 so could filter more tests than absolutely required?
 Some transformations can rarely degenerate into identity. I share your 
 concern, so I decided to check LambdaFor.debugName instead.
 
  - need to keep original test data for diagnostic purposes, since 
 getTestCaseData() produces new instance.
 
 
   78 adapter = getTestMethod().getTestCaseMH(data, 
 TestMethods.Kind.ONE);
 
 
 Could replace getTestMethod() with testCase.
 Done.
 




signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8067344: Adjust java/lang/invoke/LFCaching/LFGarbageCollectedTest.java for recent changes in java.lang.invoke

[Paul Sandoz]

Hi

  70 TestMethods testCase = getTestMethod();
  71 if (testCase == TestMethods.EXACT_INVOKER || testCase == 
TestMethods.INVOKER) {
  72 // Invokers aren't collected.
  73 return;
  74 }

Can you just filter those test cases out in the main method within 
EnumSet.complementOf?

On Dec 23, 2014, at 1:40 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Spotted some more problems:
  - need to skip identity operations (identity_* LambdaForms) in the test, 
 since corresponding LambdaForms reside in a permanent cache;
 

  82 mtype = adapter.type();
  83 if (mtype.parameterCount() == 0) {
  84 // Ignore identity_* LambdaForms.
  85 return;
  86 }

Under what conditions does this arise? i guess it might be non-determinisitic 
based on the randomly generated arity for the test case, so could filter more 
tests than absolutely required?


  - need to keep original test data for diagnostic purposes, since 
 getTestCaseData() produces new instance.
 

  78 adapter = getTestMethod().getTestCaseMH(data, 
TestMethods.Kind.ONE);


Could replace getTestMethod() with testCase.

Paul.

 Updated version:
 http://cr.openjdk.java.net/~vlivanov/8067344/webrev.01/
 
 Best regards,
 Vladimir Ivanov
 
 On 12/22/14 11:53 PM, Vladimir Ivanov wrote:
 http://cr.openjdk.java.net/~vlivanov/8067344/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8067344
 
 LFGarbageCollectedTest should be adjusted after JDK-8057020.
 
 There are a couple of problems with the test.
 
 (1) Existing logic to test that LambdaForm instance is collected isn't
 stable enough. Consequent System.GCs can hinder reference enqueueing.
 To speed up the test, I added -XX:SoftRefLRUPolicyMSPerMB=0 and limited
 the heap by -Xmx64m.
 
 (2) MethodType-based invoker caches are deliberately left strongly
 reachable. So, they should be skipped in the test.
 
 (3) Added additional diagnostic output to simplify failure analysis
 (test case details, method handle type and LambdaForm, heap dump
 (optional, -DHEAP_DUMP=true)).
 
 Testing: failing test.
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9, 8u40] RFR (XXS): 8066746: MHs.explicitCastArguments does incorrect type checks for VarargsCollector

[Paul Sandoz]

On Dec 9, 2014, at 12:47 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8066746/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8066746
 

Looks ok.

Curiously, is there a reason why you chose to use MH.invokeWithArguments rather 
than MH.invoke/invokeExact?

Paul.


 Recent changes (8057656 [1]) broke MHs.explicitCastArguments for 
 VarargsCollector case. It introduced an equivalence check between 
 MHs.explicitCastArguments and MethodHandle.asType() which doesn't work for 
 VarargsCollector case as expected.
 
 VarargsCollector has special asType() implementation, which supports 
 collecting any number of trailing positional arguments into an array 
 argument. It doesn't play well with MHs.explicitCastArguments, because the 
 latter is meant to be a pairwise argument and return type conversion.
 
 The fix is to ensure that adapted method handle has fixed arity.
 
 Testing: regression test, jck (api/java_lang/invoke), jdk/java/lang/invoke
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov
 
 [1] https://bugs.openjdk.java.net/browse/JDK-8057656
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

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

[Paul Sandoz]

On Dec 6, 2014, at 1:30 PM, Peter Levart peter.lev...@gmail.com wrote:
 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.
 

Transform still has final fields, thus when constructing a Transform using 
key.withResult(form)) i don't think hotspot will reorder the store of the 
referent, or dependent stores, so that they occur after publication of the key 
element into the Transform[] array.

So i think things are also are ok between putInCache and getInCache.

 So can LambdaForm instances be published via data races without fear that 
 they would appear half-initialized?
 

AFAICT yes. LambdaForm has final/stable fields and i presume it does not leak 
in the constructor (even when compiling on construction). See also 
MethodHandle.updateForm for rare cases where the lambda form of a MethodHandle 
is updated.

Paul.
 

 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/
 
 
 Regards, Peter



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

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

[Paul Sandoz]

On Dec 2, 2014, at 5:20 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Thanks, Paul!
 Updated webrev in place.

+1.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR(S) : 8039953 : [TESTBUG] Timeout java/lang/invoke/MethodHandles/CatchExceptionTest.java

[Paul Sandoz]

Hi Igor,

This looks ok. 

I like how you have factored out things into TimeLimitedRunner. Do you plan in 
a future patch to update lambda form test code that uses similar functionality?

Is the adjustment timeout *= 0.9 necessary? does reduction by 10% make a 
difference?

Paul.

On Nov 29, 2014, at 5:36 PM, Igor Ignatyev igor.ignat...@oracle.com wrote:

 http://cr.openjdk.java.net/~iignatyev/8039953/webrev.00
 98 lines changed: 93 ins; 3 del; 2 mod;
 
 Hi all,
 
 Please review the patch:
 
 Problem:
 on some configurations, 
 java/lang/invoke/MethodHandles/CatchExceptionTest.java can timeout before all 
 test cases are run
 
 Fix:
 interrupt test execution if it's not enough time to continue
 
 bug : https://bugs.openjdk.java.net/browse/JDK-8039953
 changes in testlibrary : https://bugs.openjdk.java.net/browse/JDK-8066191
 testing: locally
 
 -- 
 Igor



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9, 8u40] RFR (XXS): 8059880: Get rid of LambdaForm interpretation

[Paul Sandoz]

On Nov 19, 2014, at 11:24 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Aleksey, Duncan, thanks for the review and the confirmation that it doesn't 
 break stuff for you.
 
 Any Reviews, please? :-)
 

Looks good.

 Best regards,
 Vladimir Ivanov
 
 On 11/19/14, 2:23 PM, MacGregor, Duncan (GE Energy Management) wrote:
 On 18/11/2014 23:33, Aleksey Shipilev aleksey.shipi...@oracle.com
 wrote:
 On 11/19/2014 12:01 AM, Vladimir Ivanov wrote:
 http://cr.openjdk.java.net/~vlivanov/8059880/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8059880
 
 Yes, for the love of God, GO FOR IT.
 

Indeed!

Paul.

 Seconded. Startup of our stuff seems fine now with a compile threshold of
 zero, and it will make stacks so much easier to read in the debugger. :-)
 
 ___
 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



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9, 8u40] RFR (M): 8063135: Enable full LF sharing by default

[Paul Sandoz]

On Nov 19, 2014, at 10:30 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Hm, I remember I fixed that long time ago... Seems like I chose a stale 
 patch. Sorry for that. Updated webrev in place.
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: invoking a interface default method from within an InvocationHandler

[Paul Sandoz]

On Oct 18, 2014, at 6:59 PM, Peter Levart peter.lev...@gmail.com wrote:
 
 Hi Paul, Remi,
 
 The complete solution will require new API, but the java.lang.reflect.Proxy 
 API could be improved a bit too. With introduction of default interface 
 methods it is somehow broken now. Default interface methods enable interface 
 evolution, but break existing Proxy InvocationHandler(s) which are not 
 prepared to handle new methods. So perhaps a small improvement to Proxz API 
 could be to add new factory method that takes an additional a boolean flag 
 indicating whether generated proxy should override default methods and 
 forward their invocation to InvocationHandler or not. An all-or-nothing 
 approach. I know that even that is not trivial. Generated proxy class can 
 implement multiple interfaces and there can be conflicts. In that case, the 
 easiest solution is to bail out and refuse to generate a proxy class with 
 such combination of interfaces.

It would also be useful to investigate a safe mechanism so that an 
InvocationHandler implementation can call a default method, which then allows 
an impl to explicitly deal with conflicts.


 
 A starting point could be to re-implement current functionality using ASM and 
 then start experimenting with different approaches. What do you think?
 

Not sure replacing the bytecode generating implementation, in 
sun.misc.ProxyGenerator, with an ASM equivalent will really help all that much 
functionality wise, that may be nice from a unification perspective but does 
introduce some risk. A quicker route might be to work out how to filter out the 
default methods that are overridden in the existing code.

Paul. 


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: invoking a interface default method from within an InvocationHandler

[Paul Sandoz]

On Oct 16, 2014, at 12:43 PM, Remi Forax fo...@univ-mlv.fr wrote:

 
 On 10/15/2014 06:54 PM, Paul Sandoz wrote:
 Hi Remi,
 
 I did some brief evaluation of this area.
 
 MethodHandleProxies.asInterfaceInstance currently does not support proxying 
 to default methods. An InternalError will be thrown if a default method is 
 invoked. It should be possible to fix using a trusted internal IMPL_LOOKUP 
 to look up MHs that invokespecial. I believe this should be safe under the 
 context of proxying.
 
 Another solution is to do not rely on j.l.r.Proxy to implement 
 asInterfaceInstance but to generate a proxy using ASM,
 in that case, handling default methods is easy, just do nothing (i.e. do not 
 override a default method).
 It will also solve the fact that the proxies generated by asInterfaceInstance 
 are currently super slow
 because of the InvocationHandler API require boxing.

That's a good point, it's more involved and it would be nice to reuse 
implementing classes keyed off the functional interface, since a field can be 
defined for the proxying MH. Hmm... i wonder if we can leverage 
InnerClassLambdaMetafactory.


 
 The real solution in my opinion is to create yet another API,

Yes, i am thinking just in the 9 time-frame, beyond that, as you indicate 
below, there is much more scope to improve this area (via invoke or a class 
dynamic).

Paul.

 lets call it proxy 2.0 that takes an interface and a bootstrap method
 as parameter and generate a class that will call the BSM when calling a 
 method of the proxy class the first time
 (the BSM will be called with a 4th parameter which is a method handle 
 corresponding to the called method
 so one can use MHs.reflectAs to get the corresponding j.l.r.Method object if 
 necessary).
 For a default method, the 4th parameter will be a method handle created with 
 findSuper, so a user can choose
 to implement it or to delegate to this mh.
 And asInterfaceInstance is just a foldArguments between the method handle 
 taken as parameter and
 an exactInvoker (or a genericInvoker if parameter types mismatch).
 
 Compared to a j.l.r.Proxy, the proxy 2.0 API also need a way to store fields 
 (or at least values) inside the generated proxy class
 and a way to query that fields inside the BSM. This can be done exactly like 
 this is done by the lambda proxy,
 instead of returning a proxy class, the proxy 2.0 API will return a method 
 handle that acts as a factory for creating
 proxy instances of the proxy class (if the factory takes no arguments, as for 
 the lambda proxy, the same constant object can be returned).
 To get a mh getter from inside the BSM, because the BSM already have the 
 lookup object, all you need is a convention that says
 that the field names are something like arg0, arg1, etc. 
 
 The nice thing about this API is that it cleanly separate the information 
 that can be process from proxied interface(s)
 (by example, JavaEE annotations) and the ones, more dynamic, that are 
 specific to a proxy instance.
 It also removes one level of indirection compared to the InvocationHandler 
 proxy. 
 


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] [8u40] RFR (M): 8059877: GWT branch frequencies pollution due to LF sharing

[Paul Sandoz]

On Oct 14, 2014, at 8:54 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 Paul,
 
 Thanks for the feedback!
 
 Updated version:
  http://cr.openjdk.java.net/~vlivanov/8059877/webrev.01/
 
 http://cr.openjdk.java.net/~vlivanov/8059877/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8059877
 
 
 Generally looks ok.
 
 - MethodHandleImpl
 
  786  if (wrapper.unblock()) {
  787  // Reached invocation threshold. Replace 
 counting/blocking wrapper with a reinvoker.
  788  wrapper.isActivated = false;
 
 If unblock() returns true then isActivated is already false.
 Fixed.
 
 At the expense of another box you could use an AtomicBoolean with 
 compareAndSet if you want to really guarantee at most one unblocking, 
 although the box can be removed if the boolean is changed to 'int' and 
 Unsafe is used to CAS.
 I considered that, but I decided not to go that route. GuardWithTest is a 
 very common combinator, so footprint overhead could be noticeable. There's no 
 need to guarantee uniqueness of unblocking operation. The operation is 
 idempotent, so no problems performing it multiple times. What I try to 
 achieve with the flag is avoid pathological situation when some thread 
 continuously updates the form.

Ok.


 
 I dunno if strengthening the visibility of MethodHandle.form by stamping in 
 a memory fence after the following will help
 
  792  wrapper.updateForm(lform);
 
 e.g. using Unsafe.fullFence.
 I think the fence should be there. It won't help guarantee the update is 
 visible everywhere though. But it is expected.
 

I see you added it to MethodHandle.updateForm, i was incorrectly assuming you 
just wanted to add in the context of BlockInliningWrapper after the update 
call, so not all updates incur the full barrier cost for other calls to 
updateForm, but it makes sense in light of the ISSUE comment.

Actually i forgot MethodHandle.form is marked final!, i better understand some 
of your comments now, so visibility is even more restricted that i previously 
thought.

This is a good example to add to our stomping on a final field discussion, i 
think here it is definitely a very special case with a careful dance between 
updating and inlining (updateForm is also called by a direct method handle to a 
static method or field, such a handle initially holds a form with a check to 
initialize the class and after that occurs the handle is updated with a new 
form without the check).


 Perhaps isUnblocked is a better name than isActivated since there is no need 
 to set the initial value and it tracks the same value as that returned from 
 unblock?
 Done.
 

 765 isUnblocked = false;

Should be isUnblocked = true.

 756 MethodHandle newTarget = target.asType(newType);
 757 return asTypeCache = isUnblocked ? make(newTarget)
 758  : newTarget; // no need for a 
wrapper anymore

Should be return asTypeCache = !isUnblocked ?  ? make(newTarget) 

No need for another review round.


 Also while on the subject of naming perhaps consider changing 
 MethodTypeForm.LF_DELEGATE_COUNTING to ...LF_DELEGATE_BLOCK_INLINING?
 Done.
 
 When DONT_INLINE_THRESHOLD  0 and DONT_INLINE_THRESHOLD == 
 COMPILE_THRESHOLD will that trigger both compilation of a 
 BlockInliningWrapper's form and unblocking? I guess there is some fuzziness 
 depending on concurrent execution.
 No problems expected in this scenario - COMPILE_THRESHOLD updates LambdaForm 
 entry point and unblocking operates on a method handle.
 
 I am just wondering if there is any point compiling a BlockInliningWrapper's 
 form if DONT_INLINE_THRESHOLD is expected to be ~ the same as 
 COMPILE_THRESHOLD. Probably not terribly important especially if the 
 JIT-compiler control approach replaces it.
 I don't see any value in keeping BlockInliningWrapper interpreted.
 It would allow to avoid LambdaForm.forceInline flag, but there should be a 
 special case to skip compilation (in LambdaForm::checkInvocationCounter()). 
 Moreover, if we get rid of LambdaForm interpreter, we will need to precompile 
 BlockInliningWrapper again.
 

Ok.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] Review request : JDK-8058728: TEST_BUG: Make java/lang/invoke/LFCaching/LFGarbageCollectedTest.java skip arrayElementSetter and arrayElementGetter methods

[Paul Sandoz]

On Sep 19, 2014, at 11:54 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 Looks good.
 

Small typo in the comment:

  s/filed/field

Otherwise +1,
Paul.

 Best regards,
 Vladimir Ivanov
 
 On 9/19/14, 1:50 PM, Konstantin Shefov wrote:
 Hello,
 
 Please review the test bug fix
 https://bugs.openjdk.java.net/browse/JDK-8058728
 Webrev is http://cr.openjdk.java.net/~kshefov/8058728/webrev.00
 
 Thanks
 
 -Konstantin
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] Review request : JDK-8057719: Develop new tests for LambdaForm Reduction and Caching feature

[Paul Sandoz]

Hi,

On Sep 12, 2014, at 12:45 PM, Igor Ignatyev igor.ignat...@oracle.com wrote:

 Paul,
 
 thanks for your review. I'll take care about this change, since Konstantin is 
 on vacation.
 

Thanks.


 updated webrev: 
 http://cr.openjdk.java.net/~iignatyev/kshefov/8057719/webrev.00/
 
 please see inline answers.
 
 Igor
 
 On 09/11/2014 05:12 PM, Paul Sandoz wrote:
 
 On Sep 5, 2014, at 7:52 PM, Konstantin Shefov konstantin.she...@oracle.com 
 wrote:
 
 Hello,
 
 Please review the new tests for the feature Lambda Form Reduction and 
 Caching https://bugs.openjdk.java.net/browse/JDK-8046703
 
 JBS task: https://bugs.openjdk.java.net/browse/JDK-8057719
 
 Webrev: http://cr.openjdk.java.net/~kshefov/8057719/webrev.00/
 
 These tests also depend on testlibrary change: 
 https://bugs.openjdk.java.net/browse/JDK-8057707
 Webrev of the testlib change: 
 http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/
 
 
 Generally looks good. It would be interesting see code-coverage results.
 
 Are you sure the LFMultiThreadCachingTest is sufficiently testing the 
 thread-safety of j.l.invoke classes? It might be that more threads need to 
 be generated (== #cores), and the test repeatedly performed in a loop to 
 increase the chance of stuff stomping on each other. (I see you have 
 iterations in the outer loop of runTests, that might be sufficient, but you 
 might need a tighter loop specific to each test in LFMultiThreadCachingTest).
 
 good point, I updated LFMultiThreadCachingTest to use more threads, however I 
 doubt that we need to add a loop. Writing additional MT-stress tests, which 
 can provide more confidence on thread-safety, is out of the scope of 
 JDK-8057719.
 

It does not have to be of jcstress-test-like quality, but if one is trying to 
test some degree concurrent execution I think it worthwhile having a simple 
iteration for loop in LFMultiThreadCachingTest:

  for (int iter = 0; i  ITERS; i++) { ... }

balanced by default to not unduly make the test run too long. Plus often it is 
useful to define a sys prop for ITER so one can manually run it for longer 
periods.

Up to you, perhaps consider it has a possible enhancement, esp. if you wanna 
get this in quickly.


 
 A general comment, feel free to ignore. It's easy to use EnumSet to obtain a 
 collection of enum values:
 
   SetTestMethods tms = EnumSet.allOf(TestMethods.class)
 
 and filtered:
 
   SetTestMethods tms = 
 EnumSet.complementOf(EnumSet.of(TestMethods.IDENTITY, TestMethods.CONSTANT))
 
 (Quite concise with a static import.)
 
 Change LambdaFormTestCase.runTests to accept a CollectionTestMethods and 
 you are good to go :-)
 done

+1, ok to push.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] Review request : JDK-8057719: Develop new tests for LambdaForm Reduction and Caching feature

[Paul Sandoz]

On Sep 5, 2014, at 7:52 PM, Konstantin Shefov konstantin.she...@oracle.com 
wrote:

 Hello,
 
 Please review the new tests for the feature Lambda Form Reduction and 
 Caching https://bugs.openjdk.java.net/browse/JDK-8046703
 
 JBS task: https://bugs.openjdk.java.net/browse/JDK-8057719
 
 Webrev: http://cr.openjdk.java.net/~kshefov/8057719/webrev.00/
 
 These tests also depend on testlibrary change: 
 https://bugs.openjdk.java.net/browse/JDK-8057707
 Webrev of the testlib change: 
 http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/
 

Generally looks good. It would be interesting see code-coverage results.

Are you sure the LFMultiThreadCachingTest is sufficiently testing the 
thread-safety of j.l.invoke classes? It might be that more threads need to be 
generated (== #cores), and the test repeatedly performed in a loop to increase 
the chance of stuff stomping on each other. (I see you have iterations in the 
outer loop of runTests, that might be sufficient, but you might need a tighter 
loop specific to each test in LFMultiThreadCachingTest).


A general comment, feel free to ignore. It's easy to use EnumSet to obtain a 
collection of enum values:

  SetTestMethods tms = EnumSet.allOf(TestMethods.class)

and filtered:

  SetTestMethods tms = EnumSet.complementOf(EnumSet.of(TestMethods.IDENTITY, 
TestMethods.CONSTANT))

(Quite concise with a static import.)

Change LambdaFormTestCase.runTests to accept a CollectionTestMethods and you 
are good to go :-)

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S) 8057656: Improve MethodType.isCastableTo() MethodType.isConvertibleTo() checks

[Paul Sandoz]

On Sep 5, 2014, at 6:42 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
  http://cr.openjdk.java.net/~vlivanov/8057656/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8057656
 
 
  854 if (!canConvert(returnType(), newType.returnType()))
  855 return false;
  856 Class?[] srcTypes = newType.ptypes;
  857 Class?[] dstTypes = ptypes;
 
 Are the src and dst the wrong way around?
 
srcTypes = ptypes
dstTypes = newType.ptypes
 No, they are right. Parameters and return type conversions have opposite 
 directions.
 

Doh! of course, silly me.


  896 private static boolean canCast(Class? src, Class? dst) {
  897 if (src.isPrimitive()  !dst.isPrimitive()) {
  898 if (dst == Object.class || dst.isInterface())  return true;
 
 How come if the src is primitive and the dst is an interface it returns true 
 for any interface? I guess there are subtly different rules here for casting 
 and conversion.
 There are 2 types of converstions: MH.asType() and 
 MHs.explicitCastArguemnts() with more relaxed semantics.
 One of the differences is that interfaces are coerced to Object, since 
 verifier allows any interface to be treated as Object.
 
 Your questions reminded me about related changes waiting in the queue and I 
 decided to include then here.
 
 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8057656/webrev.01/
 
 Got rid of MT.isCastableTo(). MHs.explicitCastEquivalentToAsType() is used 
 instead. Also, it has detailed overview of differences between MT.asType() 
 and MHs.explicitCastArguments().
 

Much clearer IMO.

+1

Paul.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8057657: Annotate LambdaForm parameters with types

[Paul Sandoz]

On Sep 5, 2014, at 12:12 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8057657/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8057657
 

- BoundMethodHandle

  57 assert(speciesData() == speciesData(form));

I am missing some context here as to how that assert would pass for anything 
other than Species_L, namely for sub-classes of BoundMethodHandle generated by 
generateConcreteBMHClass how does that assert return true?


 153 public static SpeciesData speciesData(LambdaForm form) {

Minor point. Could be private (since only used by the assert in the 
constructor), or package private if the intention is for it to be used by other 
j.l.i classes in the future.

Paul.


 Add ability to annotate LambdaForm parameters with their types.
 Type info could be useful during LambdaForm compilation to produce better 
 bytecode.
 
 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.r...@oracle.com
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (L): Improve LambdaForm sharing by using LambdaFormEditor more extensively

[Paul Sandoz]

On Sep 9, 2014, at 12:51 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8057922/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8057922
 
 Introduce more sharing on LambdaForm level by rewriting most of the MH 
 combinators using LambdaFormEditor.
 
 The new code is guarded by USE_LAMBDA_FORM_EDITOR flag and turned off by 
 default because it introduces significant peak performance regression on 
 Octane benchmark. I'm working on the fix. Original implementation will be 
 removed once performance degradation is fixed.
 

Generally looks ok.


- LambdaFormEditor

 465 buf.endEdit();
 466 form = buf.lambdaForm();
 467 return putInCache(key, form);

A suggestion (feel free to ignore), that pattern repeats quite a bit. With some 
tweaks one could do:

  return putIntCache(but.endEdit()); // or buf.toLambdaForm()


- MethodHandles

2869 public static
2870 MethodHandle filterReturnValue(MethodHandle target, MethodHandle 
filter) {
2871 MethodType targetType = target.type();
2872 MethodType filterType = filter.type();
2873 filterReturnValueChecks(targetType, filterType);
2874 BoundMethodHandle result = target.rebind();
2875 BasicType rtype = BasicType.basicType(filterType.returnType());
2876 LambdaForm lform = result.editor().filterReturnForm(rtype, false);
2877 MethodType newType = 
targetType.changeReturnType(filterType.returnType());
2878 result = result.copyWithExtendL(newType, lform, filter);
2879 return result;
2880 }

Missing if (USE_LAMBDA_FORM_EDITOR).

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (L): Improve LambdaForm sharing by using LambdaFormEditor more extensively

[Paul Sandoz]

On Sep 9, 2014, at 3:39 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul, thanks for review!
 

+1

Paul.


 Updated webrev in place.
 
 http://cr.openjdk.java.net/~vlivanov/8057922/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8057922
 
 Introduce more sharing on LambdaForm level by rewriting most of the MH 
 combinators using LambdaFormEditor.
 
 The new code is guarded by USE_LAMBDA_FORM_EDITOR flag and turned off by 
 default because it introduces significant peak performance regression on 
 Octane benchmark. I'm working on the fix. Original implementation will be 
 removed once performance degradation is fixed.
 
 
 Generally looks ok.
 
 
 - LambdaFormEditor
 
  465 buf.endEdit();
  466 form = buf.lambdaForm();
  467 return putInCache(key, form);
 
 A suggestion (feel free to ignore), that pattern repeats quite a bit. With 
 some tweaks one could do:
 
   return putIntCache(but.endEdit()); // or buf.toLambdaForm()
 I decided to make LambdaFormBuffer.lambdaForm() private and return 
 constructed LambdaForm from LFB.endEdit(). I didn't combine endEdit()  
 putInCache() into a single statement, because I find current shape more 
 convenient for debugging.
 
 - MethodHandles
 
 2869 public static
 2870 MethodHandle filterReturnValue(MethodHandle target, MethodHandle 
 filter) {
 2871 MethodType targetType = target.type();
 2872 MethodType filterType = filter.type();
 2873 filterReturnValueChecks(targetType, filterType);
 2874 BoundMethodHandle result = target.rebind();
 2875 BasicType rtype = BasicType.basicType(filterType.returnType());
 2876 LambdaForm lform = result.editor().filterReturnForm(rtype, 
 false);
 2877 MethodType newType = 
 targetType.changeReturnType(filterType.returnType());
 2878 result = result.copyWithExtendL(newType, lform, filter);
 2879 return result;
 2880 }
 
 Missing if (USE_LAMBDA_FORM_EDITOR).
 Fixed.
 
 Best regards,
 Vladimir Ivanov
 
 Paul.
 



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8057657: Annotate LambdaForm parameters with types

[Paul Sandoz]

+1 to both.

On Sep 9, 2014, at 3:22 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8057657/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8057657
 
 
 - BoundMethodHandle
 
   57 assert(speciesData() == speciesData(form));
 
 I am missing some context here as to how that assert would pass for anything 
 other than Species_L, namely for sub-classes of BoundMethodHandle generated 
 by generateConcreteBMHClass how does that assert return true?
 Good catch, Paul. This assert should go with new bind implementation. Moved 
 the assert  speciesData(LambdaForm) from this change to 8057042 [1]. Updated 
 webrev in place.
 

Ok, i see how it wires up now, plus i suspected it might be some patch cross 
talk.

Paul.

  153 public static SpeciesData speciesData(LambdaForm form) {
 
 Minor point. Could be private (since only used by the assert in the 
 constructor), or package private if the intention is for it to be used by 
 other j.l.i classes in the future.
 Made the method package-private. It is also used in LambdaFormEditor ( see 
 oldSpeciesData() [1]).
 
 Best regards,
 Vladimir Ivanov
 
 [1] http://cr.openjdk.java.net/~vlivanov/8057042/webrev.00/



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

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

[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 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.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S) 8050173: Generalize BMH.copyWith API to all method handles

[Paul Sandoz]

On Sep 5, 2014, at 3:15 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 
 Looks good, just one comment.
 
 MethodHandles.restrictReceiver
 
 This method has:
 
 1578 private MethodHandle restrictReceiver(MemberName method, 
 MethodHandle mh, Class? caller) throws IllegalAccessException {
 ...
 1589 assert(mh instanceof DirectMethodHandle);  // 
 DirectMethodHandle.copyWith
 
 Why not make the second parameter be DirectMethodHandle mh ?
 Good point! While prototyping this I spotted uncovered corner case (restrict 
 a receiver on a MH with bound caller).
 
 Updated webrev:
  http://cr.openjdk.java.net/~vlivanov/8050173/webrev.01/
 Diff:
  http://cr.openjdk.java.net/~vlivanov/8050173/webrev.00.01/
 
 Reordered restrictReceiver and maybeBindCaller operations.
 

Looks good,
Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

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

[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 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.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

[Paul Sandoz]

On Sep 1, 2014, at 12:29 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Thanks, Paul.
 
 There's no need to add @ForceInline on selectAlternative.
 It is used only during LF interpretation. There's an intrinsic for GWT 
 combinator, which encodes it as a branch (see 
 InvokerBytecodeGenerator.emitSelectAlternative).
 

Ah yes, i forgot about those recently added intrinsics,
Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: DMH to fields, casts and type profiling was Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Aug 29, 2014, at 12:45 AM, John Rose john.r.r...@oracle.com wrote:

 On Aug 28, 2014, at 7:38 AM, Paul Sandoz paul.san...@oracle.com wrote:
 
 On Jul 8, 2014, at 9:09 PM, John Rose john.r.r...@oracle.com wrote:
 
 Regarding the extra cast in accessor logic that Paul picked up on:  That 
 may be a left-over from obsolete versions of the code, or it may cover for 
 some corner cases, or it could possibly be a re-assurance to the JIT.
 
 
 I had some enlightening discussions with Roland on this.
 
 It seems quite tricky to solve in general the removal of the null check due 
 to the aggressive nature in which the null branch is reduce to a trap, but 
 IIUC may be possible to turn Class.cast into an intrinsic to handle the 
 specific case (although that seems costly).
 
 I was labouring under the misapprehension that an explicit Class.cast was a 
 profiling point but now i realize it's only certain byte codes (like 
 checkcast/invokehandle). Nothing specific to the DHM access logic showed up 
 with regards to type profiling when analysing the MethodData output from 
 some simple examples [*]. Therefore i presume it's more likely to be the 
 first or third reason you state.
 
 So i propose to proceed with the experiment with a patch to replace the 
 casts with asserts in the accessor logic and run that through the usual 
 tests.
 
 Paul.
 
 
 [*] Also i have so far failed to concoct a simple example for VarHandles 
 where i can trigger profile pollution and failed inlining
 
 Here's something to try first:  Force a profile point before Class.cast, even 
 without Roland's enhancements.
 You should be able to type-profile x by inserting push x; checkcast 
 java/lang/Object; pop.
 See last line of https://wiki.openjdk.java.net/display/HotSpot/MethodData
 

Thanks, that's a neat trick. I will play around with that.

Some thoughts triggered (now i am a supposedly little wiser... perhaps :-)).

I could imagine things would get polluted fairly quickly within the compiled  
shared DHM LFs for field access (same for VHs), plus cast-wise only the value 
is operated on and not the receiver. In general presumably what matters most is 
the type profile from the call site that would flow down to the LF?

But... what would there be in compiled DHM LFs for field access that they would 
require profiling so that generated code would be different for accessing a 
field of Bar rather than a field of Foo? since it all boils down to Unsafe 
calls, or are there some subtle details hidden within the Unsafe intrinsics? or 
perhaps a lack of that can result in some odd effects?

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

DMH to fields, casts and type profiling was Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Jul 8, 2014, at 9:09 PM, John Rose john.r.r...@oracle.com wrote:

 Regarding the extra cast in accessor logic that Paul picked up on:  That may 
 be a left-over from obsolete versions of the code, or it may cover for some 
 corner cases, or it could possibly be a re-assurance to the JIT.
 

I had some enlightening discussions with Roland on this.

It seems quite tricky to solve in general the removal of the null check due to 
the aggressive nature in which the null branch is reduce to a trap, but IIUC 
may be possible to turn Class.cast into an intrinsic to handle the specific 
case (although that seems costly).

I was labouring under the misapprehension that an explicit Class.cast was a 
profiling point but now i realize it's only certain byte codes (like 
checkcast/invokehandle). Nothing specific to the DHM access logic showed up 
with regards to type profiling when analysing the MethodData output from some 
simple examples [*]. Therefore i presume it's more likely to be the first or 
third reason you state.

So i propose to proceed with the experiment with a patch to replace the casts 
with asserts in the accessor logic and run that through the usual tests.

Paul.


[*] Also i have so far failed to concoct a simple example for VarHandles where 
i can trigger profile pollution and failed inlining


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Redundant null checks due to casts was Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

I have logged the following issue for redundant null checks:

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

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050877: Improve code for pairwise argument conversions and value boxing/unboxing

[Paul Sandoz]

On Jul 18, 2014, at 4:02 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 ValueConversions
 
 I can see why an EnumMap is used for convenience mapping the Wrapper to MH. 
 IIUC it means the MH ref values are not @Stable? I guess it would be easy to 
 unpack into an explicit array and index from the wrapper ordinal, plus then 
 no additional runtime type checks on the key will be performed for get/put. 
 Dunno how important that is.
 
 Can UNBOX_CONVERSIONS be marked as @Stable? does that make any difference? 
 Same for BOX_CONVERSIONS etc.
 All 4 caches in ValueConversions can be marked as @Stable. But it's not 
 performance critical, because they are used only during MethodHandle 
 instantiation, which is expected to be expensive. Performance-wise it make 
 sense to reuse MethodHandles.
 

Ok.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050877: Improve code for pairwise argument conversions and value boxing/unboxing

[Paul Sandoz]

On Jul 16, 2014, at 6:28 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050877/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050877
 
 Improved MethodHandleImpl.makePairwiseConvert  ValueConversions.unbox and 
 small cleanups in related code.
 
 Also, improved method handle caching in ValueConversions.
 
 MethodHandleImpl.makePairwiseConvert:
 - * @param level which strength of conversion is allowed
 + * @param strict if true, only asType conversions are allowed; if false, 
 explicitCastArguments conversions allowed
 + * @param monobox if true, unboxing conversions are assumed to be 
 exactly typed (Integer to int only, not long or double)
 
 ValueConversions.unbox:
 -private static MethodHandle unbox(Wrapper wrap, boolean cast) {
 +private static MethodHandle unbox(Wrapper wrap, int kind) {
 +// kind 0 - strongly typed with NPE
 +// kind 1 - strongly typed but zero for null,
 +// kind 2 - asType rules: accept multiple box types but only 
 widening conversions with NPE
 +// kind 3 - explicitCastArguments rules: allow narrowing 
 conversions, zero for null
 +WrapperCache cache = UNBOX_CONVERSIONS[kind];
 
 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.r...@oracle.com
 

MethodHandleImpl

Merge the the 'if' into an 'else if':
 365 } else {
 366 if (dst.isPrimitive()) {

ValueConversions

I can see why an EnumMap is used for convenience mapping the Wrapper to MH. 
IIUC it means the MH ref values are not @Stable? I guess it would be easy to 
unpack into an explicit array and index from the wrapper ordinal, plus then no 
additional runtime type checks on the key will be performed for get/put. Dunno 
how important that is.

Can UNBOX_CONVERSIONS be marked as @Stable? does that make any difference? Same 
for BOX_CONVERSIONS etc.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050884: Intrinsify ValueConversions.identity() functions

[Paul Sandoz]

On Jul 16, 2014, at 6:44 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 http://cr.openjdk.java.net/~vlivanov/8050884/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050884
 
 Replace ValueConversions.identity() functions with intrinsics.
 
 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.r...@oracle.com
 

Looks good.

Same question as in previous email on @Stable for MethodHandles.IDENTITY_MHS.

FWIW for MethodHandles.IDENTITY_MHS the Wrapper.ordinal() is used as an index 
rather than using an EnumMap.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8050887: Intrinsify constants for default values

[Paul Sandoz]

On Jul 16, 2014, at 6:57 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 http://cr.openjdk.java.net/~vlivanov/8050887/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8050887
 
 Intrinsify MethodHandles.constant() for default values.
 
 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.r...@oracle.com
 

Looks good. Same question as before on the cached array.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050053: Improve caching of different invokers

[Paul Sandoz]

On Jul 15, 2014, at 3:48 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Update: http://cr.openjdk.java.net/~vlivanov/8050053/webrev.01
 Diff: http://cr.openjdk.java.net/~vlivanov/8050053/webrev.diff.00-01/
 
 Got rid of varargs  spread invokers.
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050052: Small cleanups in java.lang.invoke code

[Paul Sandoz]

On Jul 15, 2014, at 3:42 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Another update:
 http://cr.openjdk.java.net/~vlivanov/8050052//webrev.01/
 
 Tentative diff:
 http://cr.openjdk.java.net/~vlivanov/8050052/webrev.diff.01-02/
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050166: Get rid of some package-private methods on arguments in j.l.i.MethodHandle

[Paul Sandoz]

On Jul 14, 2014, at 4:04 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050166/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050166
 
 Get rid of the following methods in j.l.i.MethodHandle:
  * convertArguments(MethodType newType)
  * bindArgument(int pos, BasicType basicType, Object value)
  * bindReceiver(Object receiver)
  * dropArguments(MethodType srcType, int pos, int drops)
  * permuteArguments(MethodType newType, int[] reorder)
 
 Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea 
 -esa and COMPILE_THRESHOLD={0,30}.
 

Looks good. Minor points, take it or leave it:

MethodHandles. permuteArgumentChecks

Might be more preferable for permuteArgumentChecks to return true or false and 
do the throw newIllegalArgumentException on a false return. Then if would 
play better with it's use in the assert, since it can throw an assertion error.

  assert target == originalTarget;
  assert permuteArgumentChecks(reorder, newType, target.type()) : bad reorder 
array: +Arrays.toString(reorder)

MethodHandles.insertArgumentPrimitive

Might be more consistent to always use a return value in the switch:

2258 switch (w) {
2259 default:  intValue = ValueConversions.widenSubword(value);
break;
2260 case INT: intValue = (int)value;  
break;

default: return result.bindArgumentI(pos, ValueConversions.widenSubword(value));
case INT: return result.bindArgumentI(pos, (int)value);

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S) 8050173: Generalize BMH.copyWith API to all method handles

[Paul Sandoz]

On Jul 14, 2014, at 4:37 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050173/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050173
 
 Added j.l.i.MethodHandle.copyWith(MethodType, LambdaForm) and provided 
 implementation for all subclasses.
 
 Also, some cleanups:
  * rewrote MH.viewAsType on top of MH.copyWith;
  * extended MH.viewAsType to do strict checks w/ assertions turned on (new 
 parameter: boolean strict);
  * extended MT.isViewableAs to accept both interface preserving and interface 
 erasing conversions (new parameter: boolean keepInterfaces).
 
 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.r...@oracle.com
 

Looks good, just one comment.

MethodHandles.restrictReceiver

This method has:

1578 private MethodHandle restrictReceiver(MemberName method, 
MethodHandle mh, Class? caller) throws IllegalAccessException {
...
1589 assert(mh instanceof DirectMethodHandle);  // 
DirectMethodHandle.copyWith

Why not make the second parameter be DirectMethodHandle mh ?

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (XS): 8050174: Support overriding of isInvokeSpecial flag in WrappedMember

[Paul Sandoz]

On Jul 14, 2014, at 4:47 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050174/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050174
 
 Support overriding of isInvokeSpecial flag in WrappedMember.
 
 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.r...@oracle.com
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050057: Improve caching of MethodHandle reinvokers

[Paul Sandoz]

On Jul 14, 2014, at 5:17 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050057/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050057
 
 Cache MethodHandle reinvokers per basic type.
 For BoundMethodHandles, rebinding is no-op unless underlying LF is too 
 complex (see BMH::tooComplex() for details).
 
 Also, introduced DelegatingMethodHandle whose invocation behavior is 
 determined by a target MethodHandle. The delegating MH itself can hold extra 
 intentions beyond the simple behavior.
 
 AsVarargsCollector and WrappedMember are made subclasses of 
 DelegatingMethodHandle. Also, SimpleMethodHandle extends BoundMethodHandle 
 now.
 
 Rebinding and delegation share same logic and LF shape, but have different 
 caches (LF_REBIND vs LF_DELEGATE). The only difference is their name. They 
 could be consolidated in the future.
 
 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.r...@oracle.com
 

Looks good. Juste one comment.

BoundMethodHandle

The following fields can be made final:
132 private static int FIELD_COUNT_THRESHOLD = 12;  // largest convenient 
BMH field count
133 private static int FORM_EXPRESSION_THRESHOLD = 24;  // largest 
convenient BMH expression count

Paul.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050200: Make LambdaForm intrinsics detection more robust

[Paul Sandoz]

On Jul 14, 2014, at 7:10 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050200/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050200
 
 Replace pattern matching sequences of LambdaForm names during compilation 
 with explicit marks on MethodHandles. Intrinsic ID is associated with a 
 method handle using DelegatingMethodHandle.
 
 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.r...@oracle.com
 

Looks good.

I don't see any usages of the three arg MethodHandleImpl.makeIntrinsic, but i 
AFAICT such usages will occur further later patches.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050166: Get rid of some package-private methods on arguments in j.l.i.MethodHandle

[Paul Sandoz]

On Jul 16, 2014, at 12:53 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul, thanks for review.
 
 On 7/16/14 12:34 PM, Paul Sandoz wrote:
 
 On Jul 14, 2014, at 4:04 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
 wrote:
 
 http://cr.openjdk.java.net/~vlivanov/8050166/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8050166
 
 Get rid of the following methods in j.l.i.MethodHandle:
  * convertArguments(MethodType newType)
  * bindArgument(int pos, BasicType basicType, Object value)
  * bindReceiver(Object receiver)
  * dropArguments(MethodType srcType, int pos, int drops)
  * permuteArguments(MethodType newType, int[] reorder)
 
 Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ 
 -ea -esa and COMPILE_THRESHOLD={0,30}.
 
 
 Looks good. Minor points, take it or leave it:
 
 MethodHandles. permuteArgumentChecks
 
 Might be more preferable for permuteArgumentChecks to return true or false 
 and do the throw newIllegalArgumentException on a false return. Then if 
 would play better with it's use in the assert, since it can throw an 
 assertion error.
 
   assert target == originalTarget;
   assert permuteArgumentChecks(reorder, newType, target.type()) : bad 
 reorder array: +Arrays.toString(reorder)
 It's incorrect to split the assert. It's either target  reorder are 
 unchanged or new values pass all checks.

Doh! right it's || not .


 And if exception is thrown outside permuteArgumentChecks, we lose detailed 
 error message.
 

 So, I'd leave this code as is.
 

Fair enough, what i proposed only solves one possible source of 
IllegalArgumentException,i dunno how fastidious one should be about ensuring 
asserts throw AssertionError rather than another runtime exception or error 
that could potentially be handled differently, or be misconstrued as a bug in 
the assertion code itself.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050052: Small cleanups in java.lang.invoke code

[Paul Sandoz]

On Jul 11, 2014, at 6:18 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8050052/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8050052
 
 Numerous small code cleanups in java.lang.invoke package.
 
 Testing: jtreg, nashorn, octane w/ -ea -esa and COMPILE_THRESHOLD={0,30}.
 
 Reviewed-by: vlivanov, ?
 Contributed-by: john.r.r...@oracle.com
 

Looks ok, could not find anything obviously wrong.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8050053: Improve caching of different invokers

[Paul Sandoz]

On Jul 11, 2014, at 6:35 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 http://cr.openjdk.java.net/~vlivanov/8050053/webrev.00
 https://bugs.openjdk.java.net/browse/JDK-8050053
 
 Improve sharing of different invokers: basic, generic  exact invokers, 
 uninitialized call site invoker and NamedFunction invoker are changed.
 
 Testing: jdk/java/lang/invoke, nashorn, octane w/ -ea -esa.
 
 Reviewed-by: vlivanov, ?
 Contributed-by: john.r.r...@oracle.com
 

Looks ok,
Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Jul 8, 2014, at 9:09 PM, John Rose john.r.r...@oracle.com wrote:

 Regarding the extra cast in accessor logic that Paul picked up on:  That may 
 be a left-over from obsolete versions of the code, or it may cover for some 
 corner cases, or it could possibly be a re-assurance to the JIT.
 
 Generally speaking, we lean heavily on MH types to guarantee a priori 
 correctness of argument types.  Paul is right that the stored value type is 
 already validated and (except for corner cases we may be neglecting) does not 
 need re-validation via a cast.
 
 It might be an interesting experiment to replace the cast with an assert.
 

I did briefly look at that a few months ago, i would need to systematically 
revisit. My memory is hazy, I seem to recall removing casts perturbed the 
compilation process more than i expected.


 Sometimes corner cases bite you.  For example, an array store check is 
 necessary, if the type is an interface, because interfaces are weakly checked 
 all the way up to aastore or invokeinterface.
 
 Sometimes the JIT cannot see the type assurances implicit in a MH type, and 
 so (when choosing internal MH code shapes) we must choose between handing the 
 JIT code that is not locally verifiable, or adding reassurance casts to 
 repair the local verifiability of the code.  If the JIT thinks it sees 
 badly-typed code, it might bail out.Note that locality of verifiability is 
 a fluid concept, depending sometime on vagaries of inlining decisions.  This 
 is the reason for some awkward looking belt and suspenders MH internals, 
 such as the free use of casts in LF bytecode rendering.
 
 Usually, redundant type verifications (including casts and signatures of 
 incoming arguments) are eliminated, but they can cause (as noted) an extra 
 null check.  In theory, that should fold up also, if the null value is 
 replaced by another null, as (p == null ? null : identityFunction(p)).
 

I quickly verified the fold up does as you expect. Also, if i do the following 
the null check goes away:

public class A {

   volatile String a = A;
   volatile String snull = null;
   public volatile String b;

   static final MethodHandle b_setter;

   static {
   try {
   b_setter = MethodHandles.lookup().findSetter(A.class, b, 
String.class);
   }
   catch (Exception e) {
   throw new Error(e);
   }
   }

   public static void main(String[] args) {
   A a = new A();
   a.testLoop();
   }

   void testLoop() {
   for (int i = 0; i  100; i++) {
   testLoopOne(a);
   testLoopOne(snull);
   }
   }
   void testLoopOne(String s) {
   try {
 b_setter.invokeExact(this, s);
   } catch (Throwable t) {
   throw new Error(t);
   }
   }
}

I am probably obsessing too much over some micro/nano-benchmarks, but i am 
wondering if this could cause some unwanted de-opt/recompilation effects when 
all is good with no null values then suddenly a null triggers de-optimization.

Paul.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8038261: JSR292: cache and reuse typed array accessors

[Paul Sandoz]

On Jul 8, 2014, at 12:09 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 I'd like to revive review this review thread.
 
 Updated version:
 http://cr.openjdk.java.net/~vlivanov/8038261/webrev.03/
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8038261: JSR292: cache and reuse typed array accessors

[Paul Sandoz]

On Jul 8, 2014, at 12:40 PM, Paul Sandoz paul.san...@oracle.com wrote:

 
 On Jul 8, 2014, at 12:09 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
 wrote:
 
 I'd like to revive review this review thread.
 
 Updated version:
 http://cr.openjdk.java.net/~vlivanov/8038261/webrev.03/
 
 
 +1
 

A v. minor point. There is one newly added method 
InvokerBytecodeGenerator.match that is not used by this patch or the other one 
for 8037209. I dunno if it will be used later or not by a future patch.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Jul 8, 2014, at 12:09 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 I'd like to revive this review thread.
 
 Updated version:
 http://cr.openjdk.java.net/~vlivanov/8037209/webrev.04/
 

Seems ok. I don't claim to be knowledgable enough in the finer points of 
conversion/casting/verification but i could see anything obviously wrong with 
the code.


 Paul, I'd like to address your case (if possible) separately. Right now, I 
 don't see any way to avoid null check you see with your tests.
 

Ok, thanks. I was naively pondering whether it would be worth while replacing 
the cast with an intrinsic Unsafe.typeProfile(Class c, Object o, boolean 
isNullInteresting), then that could be called with (c, instance, false), 
although dunno if that would be any easier to solve.

Paul.




signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8032400: JSR292: invokeSpecial: InternalError attempting to lookup a method

[Paul Sandoz]

On Jun 6, 2014, at 1:17 AM, John Rose john.r.r...@oracle.com wrote:

 Reviewed.
 
 This is not a requirement, but I would prefer (in general) to see less test 
 logic in ASM-generated bytecode and more in Java.  I am guessing that the 
 invokeExact call could have been replaced by a simple weakly-typed invoke 
 call in the framing code, and likewise with most of the other invokes 
 (methodType, findSpecial) which are not caller-sensitive.

I was wondering that too, but don't wanna block the push. With some judicious 
squinting i can work out what is going on. Perhaps if there are other related 
code fixes in the pipe (e.g. for the FIXME) this test could then be updated if 
there is time.


 
 — John
 
 On Jun 5, 2014, at 3:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
 wrote:
 
 Thanks for review, Paul.
 
 
 Looks ok.
 
 The behaviour of MethodHandles.Lookup.findSpecial got me confused for a 
 while :-)
 
 Minor point: is it also worth exposing a T3.lookup() method and on the 
 returned Lookup calling findSpecial(T1.class, m, 
 MethodType.methodType(int.class), T3.class).invokeExact() to ensure the 
 programmatic path also works?
 Good point. Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8032400/webrev.01/
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (S): 8032400: JSR292: invokeSpecial: InternalError attempting to lookup a method

[Paul Sandoz]

On Jun 4, 2014, at 5:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 https://bugs.openjdk.java.net/browse/JDK-8032400
 http://cr.openjdk.java.net/~vlivanov/8032400/webrev.00/
 
 Consider the following hierarchy:
  class T1{int m() { return 1; }}
  class T2 extends T1 { static int m() { return 2; }}
  class T3 extends T2 {int m() { return 3; }}
 
 T3 has a method, which does the following using method handles:
   T3::test { invokespecial T1.m() T3 }
 
 T1.m lookup attempt from T3 ends up as T2.m lookup, but it fails since T2 has 
 static method with the same signature. Lookup.getDirectMethodCommon doesn't 
 expect such failure and throws InternalError.
 
 JVMS specification states the following:
  // JVMS 6.5: invokespecial
  // ... 2. Otherwise, if C is a class and has a superclass, a search
  // for a declaration of an instance method with the same name and
  // descriptor as the resolved method is performed, starting with the
  // direct superclass of C and continuing with the direct superclass
  // of that class, and so forth, until a match is found or no further
  // superclasses exist. If a match is found, then it is the method to
  // be invoked.
 
 The fix is to comply with the spec and search upper in the class hierarchy if 
 lookup attempt fails.
 
 Testing: regression test, jdk/test/java/lang/invoke, tests on access checks.
 

Looks ok.

The behaviour of MethodHandles.Lookup.findSpecial got me confused for a while 
:-)

Minor point: is it also worth exposing a T3.lookup() method and on the returned 
Lookup calling findSpecial(T1.class, m, MethodType.methodType(int.class), 
T3.class).invokeExact() to ensure the programmatic path also works?

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR: 8000975: (process) Merge UNIXProcess.java.bsd UNIXProcess.java.linux ( .solaris .aix)

[Paul Sandoz]

On Apr 25, 2014, at 12:04 PM, Peter Levart peter.lev...@gmail.com wrote:

 This is a ping for any Reviewer and also a question for Vladimir.
 
 Hello Vladimir,
 
 What do you think about the classloader issue in the resolving of classes in 
 MemberName.getMethodType() described below?
 

I looked a bit more closely and no longer think the one-liner will is 
sufficient, since it will break the behaviour of 
MethodType.fromMethodDescriptorString:

   public static MethodType fromMethodDescriptorString(String descriptor, 
ClassLoader loader)
   throws IllegalArgumentException, TypeNotPresentException

   /**
* Finds or creates an instance of a method type, given the spelling of its 
bytecode descriptor.
* Convenience method for {@link #methodType(java.lang.Class, 
java.lang.Class[]) methodType}.
* Any class or interface name embedded in the descriptor string
* will be resolved by calling {@link 
ClassLoader#loadClass(java.lang.String)}
* on the given loader (or if it is null, on the system class loader).


The observations do suggest there may be some unexpected future surprises in 
store for bootclasspath code in restricted packages when a SM is enabled. (The 
more code we can get off the boot classpath the better off we will be 
hopefully Jigsaw FTW!)

It is be better if the jtreg process test did not monkey around with the 
restricted package list, it's asking for a poke in the eye! 

IMHO we should prioritize fixing the test rather than fixing the lambda code to 
make that test work.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR: 8000975: (process) Merge UNIXProcess.java.bsd UNIXProcess.java.linux ( .solaris .aix)

[Paul Sandoz]

Hi Peter,

IMHO such security manager usage by the test is v. fragile and we should try 
and find a safer alternative if possible.

However, there may also be an issue with lambda form code. (About a month ago i 
too was looking, internally, at this kind of issue and thought there was a 
questionable use of the application/system class loader when initializing the 
LambdaForm class, but then i got distracted with other stuff and forgot about 
it.)

Your one-liner fix seems to do the trick, but I think we need Vladimir to 
confirm this is OK.

Paul.

On Apr 17, 2014, at 11:49 PM, Peter Levart peter.lev...@gmail.com wrote:

 Hi,
 
 I'm cross-posting this on the mlvm-dev mailing list, because I think it 
 concerns internal MHs implementation.
 
 While replacing some inner classes with lambdas in java.lang.UNIXProcess 
 class, a jtreg test started failing. This test is employing a security 
 manager with an unusual configuration. It defines java.util as a package 
 for which access should be checked against the set of permissions. The class 
 initialization of UNIXProcess class initializes some lambdas and their 
 initialization fails with the following stack-trace:
 
 
 java.lang.ExceptionInInitializerError
at 
 java.lang.invoke.DirectMethodHandle.makePreparedLambdaForm(DirectMethodHandle.java:256)
at 
 java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:221)
at 
 java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:210)
at java.lang.invoke.DirectMethodHandle.make(DirectMethodHandle.java:82)
at 
 java.lang.invoke.MethodHandles$Lookup.getDirectMethodCommon(MethodHandles.java:1638)
at 
 java.lang.invoke.MethodHandles$Lookup.getDirectMethodNoSecurityManager(MethodHandles.java:1602)
at 
 java.lang.invoke.MethodHandles$Lookup.getDirectMethodForConstant(MethodHandles.java:1778)
at 
 java.lang.invoke.MethodHandles$Lookup.linkMethodHandleConstant(MethodHandles.java:1727)
at 
 java.lang.invoke.MethodHandleNatives.linkMethodHandleConstant(MethodHandleNatives.java:442)
at java.lang.UNIXProcess$Platform.get(UNIXProcess.java:147)
at java.lang.UNIXProcess.clinit(UNIXProcess.java:160)
at java.lang.ProcessImpl.start(ProcessImpl.java:130)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1023)
at java.lang.Runtime.exec(Runtime.java:620)
at java.lang.Runtime.exec(Runtime.java:485)
at SecurityManagerClinit.main(SecurityManagerClinit.java:70)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
 sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
 sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:484)
at 
 com.sun.javatest.regtest.MainWrapper$MainThread.run(MainWrapper.java:94)
at java.lang.Thread.run(Thread.java:744)
 Caused by: java.security.AccessControlException: access denied 
 (java.lang.RuntimePermission accessClassInPackage.java.util)
at 
 java.security.AccessControlContext.checkPermission(AccessControlContext.java:457)
at 
 java.security.AccessController.checkPermission(AccessController.java:884)
at java.lang.SecurityManager.checkPermission(SecurityManager.java:541)
at 
 java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1481)
 *at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:305)*
at java.lang.ClassLoader.loadClass(ClassLoader.java:359)
at 
 sun.invoke.util.BytecodeDescriptor.parseSig(BytecodeDescriptor.java:83)
 *at 
 sun.invoke.util.BytecodeDescriptor.parseMethod(BytecodeDescriptor.java:54)*
at 
 sun.invoke.util.BytecodeDescriptor.parseMethod(BytecodeDescriptor.java:41)
at 
 java.lang.invoke.MethodType.fromMethodDescriptorString(MethodType.java:911)
at java.lang.invoke.MemberName.getMethodType(MemberName.java:144)
at 
 java.lang.invoke.LambdaForm.computeInitialPreparedForms(LambdaForm.java:477)
at java.lang.invoke.LambdaForm.clinit(LambdaForm.java:1641)
 
 
 I think I found the root cause of the problem. It's nothing wrong with the 
 test (although making java.util as an access-checked package is a little 
 unusual). The problem, I think, is in MH's LambdaForm implementation. 
 Although the UNIXProcess class is a system class (loaded by bootstrap class 
 loader), MHs created by evaluating lambda expressions in this class, trigger 
 loading a class in java.util package using AppClassLoader as the initiating 
 class loader, which involves package access checks. This should not happen, I 
 think, if only the system classes are involved in constructing MHs.
 
 I checked the code and there's a ClassLoader parameter passed to the 
 *BytecodeDescriptor.parseMethod *method. This ClassLoader is taken as the 
 defining class loader of the 

Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

[Paul Sandoz]

On Apr 8, 2014, at 1:53 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Thanks, Chris.
 
 I have to do one more iteration:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.05/
 
 I have to revert changes related to BMH::reinvokerTarget.
 
 Removal of reinvokerTarget in generated concrete BMH classes introduces 
 serious performance regression, since BMH::reinvokerTarget is much more 
 complex than an accessor and it disturbs inlining decisions in too many 
 places.
 

OK, IIUC it's just reintroducing some original code back into BMH, nothing else 
has changed. If so +1 , lets get this pushed :-)

I can now see why it might cause a perf issue if the following was used instead:

@Override MethodHandle reinvokerTarget() {
try {
return (MethodHandle) argL(0);
} catch (Throwable ex) {
throw newInternalError(ex);
}
}

Paul.

[1] http://www.diffnow.com/?report=biopj


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Apr 1, 2014, at 6:21 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 On 4/1/14 7:29 PM, Paul Sandoz wrote:
 
 On Apr 1, 2014, at 4:10 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
 wrote:
 
 Paul,
 
 Unfortunately, additional profiling doesn't work for Accessor.checkCast 
 case. The problem is Accessor.checkCast is called from multiple places, so 
 type profile is very likely to be polluted. And it kills the benefits.
 
 

Though... i wonder why the accessor cast is any more or less unique than casts 
performed for lambda form.


 So is there any point in doing such a cast in this case?
 
 The type performing the cast is the field type declared as a parameter in 
 the MethodType of the MethodHandle and also held by the Accessor.
 
 IIUC the Invokers.checkExactType should ensure no unsavoury instances of 
 the wrong type gets through? (the holder instance is only checked for null, 
 via checkBase).
 I don't see any point in doing profiling for this particular case. Such shape 
 should be well optimized by JIT if it sees that an instance of Accessor is a 
 constant. As I understand, it should be the case for most of the usage 
 scenarios.
 

Perhaps conservatively we could retain the existing casts if PROFILE  0. I can 
provide a patch if that helps?

Also, just double checking, i would presume the same would be applicable for MH 
setter/getters to arrays as per your patch improving those?


 Regarding redundant null check, do you have a test case so I can play with 
 it myself?
 
 
 I will send something to you later today or tomorrow.
 


Still plan to send you something today :-) but later on...

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Apr 2, 2014, at 4:16 PM, Paul Sandoz paul.san...@oracle.com wrote:
 
 Regarding redundant null check, do you have a test case so I can play with 
 it myself?
 
 
 I will send something to you later today or tomorrow.
 
 
 
 Still plan to send you something today :-) but later on...
 

See below for an inline trace, the assembler and the class that was executed 
with  -XX:-TieredCompilation using Java 8.

Paul.

Inlining _isInstance on constant Class java/lang/String
   !@ 9   MHFieldTest::testLoopOne (25 bytes)   inline 
(hot)
  @ 8   
java.lang.invoke.LambdaForm$MH/617901222::invokeExact_MT (15 bytes)   inline 
(hot)
@ 2   java.lang.invoke.Invokers::checkExactType 
(30 bytes)   inline (hot)
  @ 11   java.lang.invoke.MethodHandle::type (5 
bytes)   accessor
@ 11   
java.lang.invoke.LambdaForm$MH/523429237::putObjectFieldCast (32 bytes)   
inline (hot)
  @ 1   
java.lang.invoke.DirectMethodHandle::fieldOffset (9 bytes)   inline (hot)
  @ 6   
java.lang.invoke.DirectMethodHandle::checkBase (7 bytes)   inline (hot)
@ 1   java.lang.Object::getClass (0 bytes)  
 (intrinsic)
  @ 13   
java.lang.invoke.DirectMethodHandle::checkCast (9 bytes)   inline (hot)
@ 5   
java.lang.invoke.DirectMethodHandle$Accessor::checkCast (9 bytes)   inline (hot)
  @ 5   java.lang.Class::cast (27 bytes)   
inline (hot)
@ 6   java.lang.Class::isInstance (0 
bytes)   (intrinsic)
  @ 28   sun.misc.Unsafe::putObject (0 bytes)   
(intrinsic)

[Verified Entry Point]
  0x00010ccf0da0: mov%eax,-0x14000(%rsp)
  0x00010ccf0da7: push   %rbp
  0x00010ccf0da8: sub$0x20,%rsp ;*synchronization entry
; - MHFieldTest::testLoopOne@-1 
(line 57)

  0x00010ccf0dac: mov0xc(%rsi),%r10d;*getfield a
; - MHFieldTest::testLoopOne@5 
(line 57)

  0x00010ccf0db0: test   %r10d,%r10d
  0x00010ccf0db3: je 0x00010ccf0ddd  ;*ifnull
; - java.lang.Class::cast@1 
(line 3257)
; - 
java.lang.invoke.DirectMethodHandle$Accessor::checkCast@5 (line 441)
; - 
java.lang.invoke.DirectMethodHandle::checkCast@5 (line 510)
; - 
java.lang.invoke.LambdaForm$MH/640070680::putObjectFieldCast@13
; - 
java.lang.invoke.LambdaForm$MH/789451787::invokeExact_MT@11
; - MHFieldTest::testLoopOne@8 
(line 57)

  0x00010ccf0db5: add$0x10,%rsi
  0x00010ccf0db9: mov%r10d,(%rsi)
  0x00010ccf0dbc: mov%rsi,%r10
  0x00010ccf0dbf: shr$0x9,%r10
  0x00010ccf0dc3: mov$0x18f78,%r11
  0x00010ccf0dcd: mov%r12b,(%r11,%r10,1)  ;*getfield a
; - MHFieldTest::testLoopOne@5 
(line 57)

  0x00010ccf0dd1: add$0x20,%rsp
  0x00010ccf0dd5: pop%rbp
  0x00010ccf0dd6: test   %eax,-0x113ddc(%rip)# 0x00010cbdd000
;   {poll_return}
  0x00010ccf0ddc: retq   
  0x00010ccf0ddd: mov%rsi,%rbp
  0x00010ccf0de0: mov%r10d,0x4(%rsp)
  0x00010ccf0de5: mov$0xffad,%esi
  0x00010ccf0dea: nop
  0x00010ccf0deb: callq  0x00010ccbc120  ; OopMap{rbp=Oop [4]=NarrowOop 
off=112}
;*ifnull
; - java.lang.Class::cast@1 
(line 3257)
; - 
java.lang.invoke.DirectMethodHandle$Accessor::checkCast@5 (line 441)
; - 
java.lang.invoke.DirectMethodHandle::checkCast@5 (line 510)
; - 
java.lang.invoke.LambdaForm$MH/640070680::putObjectFieldCast@13
; - 
java.lang.invoke.LambdaForm$MH/789451787::invokeExact_MT@11
; - MHFieldTest::testLoopOne@8 
(line 57)
;   {runtime_call}
  0x00010ccf0df0: callq  0x00010c07ace4  ;*ifnull
; - java.lang.Class::cast@1 
(line 3257)
; - 
java.lang.invoke.DirectMethodHandle$Accessor::checkCast@5 (line 441

Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

[Paul Sandoz]

Hi Vladimir,

This looks good. Minor stuff below.

I too prefer *_TYPE instead of Int/Float/Void etc as those are not v. friendly 
for static imports.

Paul.

LambaForm:
--

private static int fixResult(int result, Name[] names) {
if (result == LAST_RESULT)
result = names.length - 1;  // might still be void
if (result = 0  names[result].type == V_TYPE)
result = -1;
return result;
}

change to result = -1 to:

  result = VOID_RESULT;

?


Change:

LambdaForm addArguments(int pos, ListClass? types) {
BasicType[] basicTypes = new BasicType[types.size()];
for (int i = 0; i  basicTypes.length; i++)
basicTypes[i] = basicType(types.get(i));
return addArguments(pos, basicTypes);
}

to:

LambdaForm addArguments(int pos, ListClass? types) {
return addArguments(pos, basicTypes(types));
}

?


Methods not used, i cannot tell which may be there for future code or are 
referenced indirectly:

String basicTypeSignature() {
//return LambdaForm.basicTypeSignature(resolvedHandle.type());
return LambdaForm.basicTypeSignature(methodType());
}

void resolve() {
for (Name n : names) n.resolve();
}

static LambdaForm identityForm(BasicType type) {
return LF_identityForm[type.ordinal()];
}
static LambdaForm zeroForm(BasicType type) {
return LF_zeroForm[type.ordinal()];
}



BoundMethodHandle:
--

Methods not used:

SpeciesData extendWith(Class? type) {
return extendWith(basicType(type));
}

SpeciesData extendWithChar(char type) {
return extendWith(basicType(type));
}

static void initStatics() {}
static { SpeciesData.initStatics(); }


Deprecated method in ASM (there are also a few others):

mv.visitMethodInsn(INVOKESPECIAL, className, init, 
makeSignature(types, true));

I think you need to append false as the last parameter.


Unused first parameter cbmhClass:

static NamedFunction[] makeNominalGetters(Class? cbmhClass, String 
types, NamedFunction[] nfs, MethodHandle[] getters) {


InvokerByteCodeGenerator
--

private int loadInsnOpcode(BasicType type) throws InternalError {
int opcode;
switch (type) {
case I_TYPE:  opcode = Opcodes.ILOAD;  break;
case J_TYPE:  opcode = Opcodes.LLOAD;  break;
case F_TYPE:  opcode = Opcodes.FLOAD;  break;
case D_TYPE:  opcode = Opcodes.DLOAD;  break;
case L_TYPE:  opcode = Opcodes.ALOAD;  break;
default:
throw new InternalError(unknown type:  + type);
}
return opcode;
}

Could just do:

  case I_TYPE:  return Opcodes.ILOAD;

etc. Same for storeInsnOpcode method.


Unused?

static int nfi = 0;


MethodHandle
--

Unused?

/*non-public*/
MethodHandle bindImmediate(int pos, BasicType basicType, Object value) {
// Bind an immediate value to a position in the arguments.
// This means, elide the respective argument,
// and replace all references to it in NamedFunction args with the 
specified value.

// CURRENT RESTRICTIONS
// * only for pos 0 and UNSAFE (position is adjusted in MHImpl to make 
API usable for others)
assert pos == 0  basicType == L_TYPE  value instanceof Unsafe;
MethodType type2 = type.dropParameterTypes(pos, pos + 1); // 
adjustment: ignore receiver!
LambdaForm form2 = form.bindImmediate(pos + 1, basicType, value); // 
adjust pos to form-relative pos
return copyWith(type2, form2);
}


On Mar 24, 2014, at 5:29 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Updated version:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03/
 
 - changed the way how arrays of types are created:
static final BasicType[] ALL_TYPES = BasicType.values();
static final BasicType[] ARG_TYPES = Arrays.copyOf(ALL_TYPES, 
 ALL_TYPES.length-1);
 
 - added a test for BasicType (LambdaFormTest.testBasicType).
 
 Best regards,
 Vladimir Ivanov



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Mar 14, 2014, at 5:36 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:
 Doh! crossed webrevs, thanks.
 
 Just had a quick look, this looks like a really nice improvement to the 
 array setter/getter support, definitely simplified. IIUC the mh.viewAsType 
 will now handle the appropriate casting. I believe it might reduce the 
 ceremony for array setter/getter MHs [1].
 
 I see there is a PROFILE_LEVEL option, by default set to 0, that results in 
 casts not being emitted:
 
 +if (VerifyType.isNullConversion(Object.class, pclass, 
 false)) {
 +if (PROFILE_LEVEL  0)
 +emitReferenceCast(Object.class, arg);
 +return;
 +}
 ...
 +mv.visitLdcInsn(constantPlaceholder(cls));
 +mv.visitTypeInsn(Opcodes.CHECKCAST, CLS);
 +mv.visitInsn(Opcodes.SWAP);
 +mv.visitMethodInsn(Opcodes.INVOKESTATIC, MHI, castReference, 
 CLL_SIG, false);
 +if (Object[].class.isAssignableFrom(cls))
 +mv.visitTypeInsn(Opcodes.CHECKCAST, OBJARY);
 +else if (PROFILE_LEVEL  0)
 +mv.visitTypeInsn(Opcodes.CHECKCAST, OBJ);
 
 Can you explain a bit the rational for that?
 These casts are redundant - they aren't required for bytecode correctness. 
 The idea behind PROFILE_LEVEL is to provide more type information to 
 JIT-compiler. Right now, type profiling occurs on every checkcast 
 instruction. So, having these additional instructions we can feed C2 with 
 more accurate information about types.
 
 Consider this as a hack to overcome some of the limitations of current 
 profiling implementation in VM.
 

Apologies for the late reply this dropped off my radar...

Ah! i may have just had a minor epiphany :-)

So that is why in DirectMethodHandle there are casts for fields, via say 
Accessor.checkCast? 

@Override Object checkCast(Object obj) {
return fieldType.cast(obj);
}

if so could PROFILE_LEVEL be supported in that code too?

Perhaps the JIT could derive some profile information from the MethodType of 
the MethodHandle?

I notice that in my experiments for enhanced access to instances of fields that 
casts are almost optimized away but a null-check is left [*], which is also 
seems redundant and could impact performance get/set of null values.

Paul.

[*]

 0x00010d050f70: test   %r10d,%r10d
 0x00010d050f73: je 0x00010d050f9d
...
 0x00010d050f9d: mov%rsi,%rbp
 0x00010d050fa0: mov%r10d,0x4(%rsp)
 0x00010d050fa5: mov$0xffad,%esi
 0x00010d050faa: nop
 0x00010d050fab: callq  0x00010d0163e0  ; OopMap{rbp=Oop [4]=NarrowOop 
off=112}
   ;*ifnull
   ; - java.lang.Class::cast@1 
(line 3253)
   ; - 
java.lang.invoke.InstanceFieldHandle::checkCast@2 (line 133)
   ; - 
java.lang.invoke.InstanceFieldHandle::set@19 (line 153)
   ; - 
java.lang.invoke.VarHandle::set@21 (line 127)
   ; - VarHandleTest::testLoopOne@8 
(line 157)
   ;   {runtime_call}
 0x00010d050fb0: callq  0x00010c39d330  ;*ifnull
   ; - java.lang.Class::cast@1 
(line 3253)
   ; - 
java.lang.invoke.InstanceFieldHandle::checkCast@2 (line 133)
   ; - 
java.lang.invoke.InstanceFieldHandle::set@19 (line 153)
   ; - 
java.lang.invoke.VarHandle::set@21 (line 127)
   ; - VarHandleTest::testLoopOne@8 
(line 157)
   ;   {runtime_call}



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Apr 1, 2014, at 4:10 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul,
 
 Unfortunately, additional profiling doesn't work for Accessor.checkCast case. 
 The problem is Accessor.checkCast is called from multiple places, so type 
 profile is very likely to be polluted. And it kills the benefits.
 

So is there any point in doing such a cast in this case? 

The type performing the cast is the field type declared as a parameter in the 
MethodType of the MethodHandle and also held by the Accessor. 

IIUC the Invokers.checkExactType should ensure no unsavoury instances of the 
wrong type gets through? (the holder instance is only checked for null, via 
checkBase).


 I don't think MethodType helps with profiling in any way. It represents type 
 info which is necessary for correctness checks. Profiling collects more 
 fine-grained information (e.g. exact types, values).
 

OK.


 Regarding redundant null check, do you have a test case so I can play with it 
 myself?
 

I will send something to you later today or tomorrow.

Paul.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

[Paul Sandoz]

On Apr 1, 2014, at 3:57 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul, thanks for review.
 
 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.04/
 

+1

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Mar 14, 2014, at 1:19 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 FYI, this change isn't limited to only bytecode assembly improvements, but 
 also contains caching of lambda forms for setters/getter of typed arrays.
 

Do you mean for MethodHandles.arrayElementGetter/Setter? If so i don't see 
relevant changes in:

http://cr.openjdk.java.net/~vlivanov/8037210/webrev.00/src/share/classes/java/lang/invoke/MethodHandleImpl.java.sdiff.html

to say MethodHandleImpl.ArrayAccessor:

static final class ArrayAccessor {
/// Support for array element access
static final HashMapClass?, MethodHandle GETTER_CACHE = new 
HashMap();  // TODO use it
static final HashMapClass?, MethodHandle SETTER_CACHE = new 
HashMap();  // TODO use it

Paul.

 If there are any objections, I can back the caching logic out and include it 
 into one of upcoming changes.
 
 Best regards,
 Vladimir Ivanov
 
 On 3/14/14 3:45 PM, Vladimir Ivanov wrote:
 http://cr.openjdk.java.net/~vlivanov/8037209/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8037209
 440 lines changed: 313 ins; 67 del; 60 mod
 
 This is a cleanup of JSR292 code to improve bytecode assembly code for
 lambda forms.
 
 Contributed-by: john.r.r...@oracle.com
 
 Testing: jdk/java/{lang/invoke,util}, vm.mlvm.testlist, nashorn, jruby
 
 Configs: -ea -esa -Xverify:all -D...COMPILE_THRESHOLD={0,30}
 -D...PROFILE_LEVEL={0,1}
 
 Best regards,
 Vladimir Ivanov
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8027827: Improve performance of catchException combinator

[Paul Sandoz]

On Feb 20, 2014, at 6:57 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul,
 
 Thanks for the feedback! See my answers inline.
 
 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.01/
 
 I finally figured out how to make caching work. This webrev contains these 
 changes.
 

+1


 I changed LF representation a bit and added 2 auxiliary method handles - 
 argument boxing and wrapping into Object[] and result unboxing. These 
 operations depend on actual type and can't be shared among arbitrary 
 combinators with the same basic type. They are used only during LF 
 interpretation and are completely ignored in compiled LFs.
 


 src/share/classes/java/lang/invoke/MethodHandleImpl.java
 
  634 // t_{i+2}:L=ValueConversions.unbox(t_{i+1}) OR 
 ValueConversions.identity(t_{i+1})
  635 if (type.returnType().isPrimitive()) {
  636 names[UNBOX_RESULT] = new 
 Name(ValueConversions.unbox(type.returnType()),
  637   names[TRY_CATCH]);
  638 } else {
  639 names[UNBOX_RESULT] = new Name(ValueConversions.identity(),
  640   names[TRY_CATCH]);
  641 }
 
 
 You could create the form without the identity transform for the 
 non-primitive case?
 
   final int UNBOX_RESULT = type.returnType().isPrimitive() ? nameCursor++ : 
 0;
 ...
 
   if (UNBOX_RESULT  0) { ...
   names[UNBOX_RESULT] = new 
 Name(ValueConversions.unbox(type.returnType()), names[TRY_CATCH]);
   }
 I can, but it complicates matching and compiling the pattern in 
 InvokerBytecodeGenerator. I decided to keep the shape uniform for all cases.
 

Ah, yes i see now, the code is simpler being kept uniform.

Paul.


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [9] RFR (M): 8027827: Improve performance of catchException combinator

[Paul Sandoz]

Hi Vladimir,

I know just enough about this area to be dangerous


src/share/classes/java/lang/invoke/BoundMethodHandle.java

 865 private static final SpeciesData[] SPECIES_DATA_CACHE = new 
SpeciesData[4];


Only 3 elements are stored in the above array?



src/share/classes/java/lang/invoke/MethodHandleImpl.java

 634 // t_{i+2}:L=ValueConversions.unbox(t_{i+1}) OR 
ValueConversions.identity(t_{i+1})
 635 if (type.returnType().isPrimitive()) {
 636 names[UNBOX_RESULT] = new 
Name(ValueConversions.unbox(type.returnType()),
 637   names[TRY_CATCH]);
 638 } else {
 639 names[UNBOX_RESULT] = new Name(ValueConversions.identity(),
 640   names[TRY_CATCH]);
 641 }


You could create the form without the identity transform for the non-primitive 
case?

  final int UNBOX_RESULT = type.returnType().isPrimitive() ? nameCursor++ : 0;
...

  if (UNBOX_RESULT  0) { ...
  names[UNBOX_RESULT] = new Name(ValueConversions.unbox(type.returnType()), 
names[TRY_CATCH]);
  }



 699 try {
 700 GUARD_WITH_CATCH
 701 = IMPL_LOOKUP.findStatic(MethodHandleImpl.class, 
guardWithCatch,
 702 MethodType.methodType(Object.class, 
MethodHandle.class, Class.class, MethodHandle.class, Object[].class));
 703 } catch (ReflectiveOperationException ex) {
 704 throw new RuntimeException(ex);
 705 }
 706 return GUARD_WITH_CATCH;


Should #704 be:

  throw newInternalError(ex);

?


test/java/lang/invoke/MethodHandles/TestCatchException.java

 100 Object o = new Object();
 101 Object[] obj1 = new Object[] { str };
 102 
 103 Object r1 = target.invokeExact(o, o, o, o, o, o, o, o, obj1);
 104 Object r2 = gwc.invokeExact(o, o, o, o, o, o, o, o, obj1);
 105 assertTrue(r1 != null);
 106 assertTrue(r1.equals(r2));

To be on the safe side should probably assert as follows:

  assertEquals(r1, obj);
  assertEquals(r2, obj);

Paul.


On Feb 19, 2014, at 10:46 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.00
 https://jbs.oracle.com/bugs/browse/JDK-8027827
 354 lines changed: 193 ins; 91 del; 70 mod
 
 OVERVIEW
 
 MethodHandles.catchException combinator implementation is based on generic 
 invokers (MethodHandleImpl$GuardWithCatch.invoke_*). It is significantly 
 slower than a Java equivalent (try-catch).
 
 Future Nashorn performance improvements require catchException combinator 
 speed to be on par with try-catch in Java.
 
 So, it should be represented in a more efficient form.
 
 I chose the following lambda form representation:
 
  t_{i}:L=ValueConversions.array(a1:L,...,a_{k}:L);
 t_{i+1}:L=MethodHandleImpl.guardWithCatch(t_{p1}, t_{p2}, t_{p3}, t_{i}:L);
 t_{i+2}:I=ValueConversions.unbox(t7:L);
 OR :L=ValueConversions.identity(t_{n+1})
 OR :V=ValueConversions.ignore(t_{n+1})
 
 where:
  a1, ..., a_{k} - arguments
  t_{p1}, t_{p2}, t_{p3} - target method handle, exception class, catcher 
 method handle respectively; passed as bounded parameters;
 
 During lambda form compilation it is converted into bytecode equivalent of 
 the following Java code:
  try {
  return target.invokeBasic(...);
  } catch(Throwable e) {
  if (!exClass.isInstance(e)) throw e;
  return catcher.invokeBasic(e, ...);
  }
 
 There's a set of microbenchmarks (attached to the bug) I wrote to verify 
 performance characteristics of new implementation.
 
 FURTHER WORK
 
 What is missing is lambda form caching. The plan is to have a single lambda 
 form per basic type, but it needs more work - current representation is 
 suitable for sharing on bytecode level, but lambda form interpretation 
 doesn't work well (arguments boxing + result unboxing are problematic).
 
 TESTING
 
 Tests: microbenchmarks, jdk/java/lang/invoke/, nashorn with optimistic types 
 (unit tests, octane).
 
 Tested in 2 modes:
  * COMPILE_THRESHOLD=30
  * COMPILE_THRESHOLD=0 -Xverify:all
 
 OTHER
 
 1) Update of cached name and member in LF compilation loop (see 
 InvokerBytecodeGenerator.generateCustomizedCodeBytes) fixes a bug during 
 compilation of selectAlternative when running with COMPILE_THRESHOLD=0.
 
 2) As part of this change, I fix existing bug [1], so I add regression test 
 for it.
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov
 
 [1] https://bugs.openjdk.java.net/browse/JDK-8034120



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: [8] RFR (S): 8033278: Missed access checks for Lookup.unreflect* after 8032585

[Paul Sandoz]

On Jan 31, 2014, at 3:21 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Paul,
 
 The transformation you suggest is equivalent, but I reluctant to apply it. 
 IMO, it doesn't add much value and current version is easier to read.

OK, i guess we will just have to agree to disagree on that small point as i 
think the opposite :-) but I don't wanna block the review.

Paul.

 Considering the current level of complexity in VA.isMemberAccessible I don't 
 want to increase it even further :-)
 
 Best regards,
 Vladimir Ivanov
 
 PS: thanks for looking into the fix.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev