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

2014-04-08 Thread Vladimir Ivanov

Chris, sorry for the late reply.

 Here's a version with the new naming scheme:

http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03.naming

I like existing naming scheme and OBJECT/VOID/LONG/etc names are quite popular(e.g. 
Wrapper  ASM (Opcodes) use them).


Of course they are popular because these are the type names.  There is no type 
L; it’s an object.  I don’t understand why we have to use different names just 
because they are used in other namespaces.  This is not a C define.

I see 2 problems with the naming scheme you propose.

(1) Wrapper, Opcodes  BasicType collide in some places. If element 
names are the same, static import doesn't work and all usages should be 
disambiguated.


(2) BasicType.I_TYPE corresponds to 5 primitive types. It's misleading 
to call it BasicType.INT (or BasicType.INTEGER) (consider Wrapper.INT vs 
BasicType.INT).


Current naming scheme makes correspondence with JVM basic types explicit.

Best regards,
Vladimir Ivanov




So, I'm in favor of leaving it as is.

Best regards,
Vladimir Ivanov

On 3/26/14 12:24 AM, Christian Thalinger wrote:

+ enum BasicType {
+ L_TYPE('L', Object.class, Wrapper.OBJECT),  // all reference types
+ I_TYPE('I', int.class,Wrapper.INT),
+ J_TYPE('J', long.class,   Wrapper.LONG),
+ F_TYPE('F', float.class,  Wrapper.FLOAT),
+ D_TYPE('D', double.class, Wrapper.DOUBLE),  // all primitive types
+ V_TYPE('V', void.class,   Wrapper.VOID);// not valid in all 
contexts

I would suggest to not name them X_TYPE but give them meaningful names like 
Int, Float, Void.  The enum BasicType already infers that it’s a type.  If you 
think it’s not clear that it’s a type just use BasicType.Double in that places.

On Mar 24, 2014, at 9:29 AM, 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

On 3/22/14 2:08 AM, Vladimir Ivanov wrote:

John, thanks for the feedback.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02

Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.

Best regards,
Vladimir Ivanov

On 3/21/14 10:54 PM, John Rose wrote:

On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
wrote:


Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/


That looks nice.  Strong typing; who woulda' thunk it.  :-)

The uses of .ordinal() are the extra cost relative to using just small
integers.  They seem totally reasonable in the code.

I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a common
name.

Don't make the enum class public.  And especially don't make the mutable
arrays public:

+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };

— John

P.S.  That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:

+public static final BasicType final[] ALL_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE };



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


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


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


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


___
mlvm-dev mailing list
mlvm-...@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

2014-04-08 Thread 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


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

2014-04-07 Thread Vladimir Ivanov

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.


Best regards,
Vladimir Ivanov

On 4/5/14 3:31 AM, Christian Thalinger wrote:


On Apr 3, 2014, at 9:44 PM, John Rose john.r.r...@oracle.com
mailto:john.r.r...@oracle.com wrote:


On Apr 3, 2014, at 6:33 PM, Christian Thalinger
christian.thalin...@oracle.com
mailto:christian.thalin...@oracle.com wrote:


Of course they are popular because these are the type names.  There
is no type L; it’s an object.  I don’t understand why we have to use
different names just because they are used in other namespaces.  This
is not a C define.


They stand for JVM signatures as well as basic types.  The letters are
signature letters.  Can we move on from this?


Sure.  Push it.



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




___
mlvm-dev mailing list
mlvm-...@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

2014-04-04 Thread Christian Thalinger

On Apr 3, 2014, at 9:44 PM, John Rose john.r.r...@oracle.com wrote:

 On Apr 3, 2014, at 6:33 PM, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 Of course they are popular because these are the type names.  There is no 
 type L; it’s an object.  I don’t understand why we have to use different 
 names just because they are used in other namespaces.  This is not a C 
 define.
 
 They stand for JVM signatures as well as basic types.  The letters are 
 signature letters.  Can we move on from this?

Sure.  Push it.

 
 — John
 ___
 mlvm-dev mailing list
 mlvm-...@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

2014-04-03 Thread Christian Thalinger

On Mar 26, 2014, at 8:01 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Here's a version with the new naming scheme:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03.naming
 
 I like existing naming scheme and OBJECT/VOID/LONG/etc names are quite 
 popular(e.g. Wrapper  ASM (Opcodes) use them).

Of course they are popular because these are the type names.  There is no type 
L; it’s an object.  I don’t understand why we have to use different names just 
because they are used in other namespaces.  This is not a C define.

 So, I'm in favor of leaving it as is.
 
 Best regards,
 Vladimir Ivanov
 
 On 3/26/14 12:24 AM, Christian Thalinger wrote:
 + enum BasicType {
 + L_TYPE('L', Object.class, Wrapper.OBJECT),  // all reference types
 + I_TYPE('I', int.class,Wrapper.INT),
 + J_TYPE('J', long.class,   Wrapper.LONG),
 + F_TYPE('F', float.class,  Wrapper.FLOAT),
 + D_TYPE('D', double.class, Wrapper.DOUBLE),  // all primitive types
 + V_TYPE('V', void.class,   Wrapper.VOID);// not valid in all 
 contexts
 
 I would suggest to not name them X_TYPE but give them meaningful names like 
 Int, Float, Void.  The enum BasicType already infers that it’s a type.  If 
 you think it’s not clear that it’s a type just use BasicType.Double in that 
 places.
 
 On Mar 24, 2014, at 9:29 AM, 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
 
 On 3/22/14 2:08 AM, Vladimir Ivanov wrote:
 John, thanks for the feedback.
 
 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02
 
 Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.
 
 Best regards,
 Vladimir Ivanov
 
 On 3/21/14 10:54 PM, John Rose wrote:
 On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
 vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
 wrote:
 
 Thanks for the feedback.
 
 What do you think about the following:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
 
 That looks nice.  Strong typing; who woulda' thunk it.  :-)
 
 The uses of .ordinal() are the extra cost relative to using just small
 integers.  They seem totally reasonable in the code.
 
 I suggest either wrapping ordinal as something like id or else
 changing temp names like id to ord, to reinforce the use of a common
 name.
 
 Don't make the enum class public.  And especially don't make the mutable
 arrays public:
 
 +public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
 J_TYPE, F_TYPE, D_TYPE, V_TYPE };
 +public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
 J_TYPE, F_TYPE, D_TYPE };
 
 — John
 
 P.S.  That would only be safe if we had (what we don't yet) a notion of
 frozen arrays like:
 
 +public static final BasicType final[] ALL_TYPES = { L_TYPE,
 I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
 +public static final BasicType final[] ARG_TYPES = { L_TYPE,
 I_TYPE, J_TYPE, F_TYPE, D_TYPE };
 
 
 
 ___
 mlvm-dev mailing list
 mlvm-...@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-...@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-...@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-...@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

2014-04-03 Thread John Rose
On Apr 3, 2014, at 6:33 PM, Christian Thalinger 
christian.thalin...@oracle.com wrote:

 Of course they are popular because these are the type names.  There is no 
 type L; it’s an object.  I don’t understand why we have to use different 
 names just because they are used in other namespaces.  This is not a C define.

They stand for JVM signatures as well as basic types.  The letters are 
signature letters.  Can we move on from this?

— John

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

2014-04-01 Thread 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



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

2014-04-01 Thread Vladimir Ivanov

Paul, thanks for review.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.04/

Best regards,
Vladimir Ivanov

On 4/1/14 1:44 PM, Paul Sandoz wrote:

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;

?

Done.




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));
 }

?

Done.




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()];
 }


Removed.




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(); }

Removed.




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.

Fixed.




Unused first parameter cbmhClass:

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

Removed.


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.

Changed as you suggest.




Unused?

 static int nfi = 0;

Removed.


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);
 }

Removed.

Best regards,
Vladimir Ivanov




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




___
mlvm-dev mailing list
mlvm-...@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

2014-04-01 Thread 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.


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

2014-04-01 Thread Vladimir Ivanov

Thank you, Paul.

Best regards,
Vladimir Ivanov

On 4/1/14 7:42 PM, Paul Sandoz wrote:


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.



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

2014-03-26 Thread Vladimir Ivanov

Here's a version with the new naming scheme:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03.naming

I like existing naming scheme and OBJECT/VOID/LONG/etc names are quite 
popular(e.g. Wrapper  ASM (Opcodes) use them). So, I'm in favor of 
leaving it as is.


Best regards,
Vladimir Ivanov

On 3/26/14 12:24 AM, Christian Thalinger wrote:

+ enum BasicType {
+ L_TYPE('L', Object.class, Wrapper.OBJECT),  // all reference types
+ I_TYPE('I', int.class,Wrapper.INT),
+ J_TYPE('J', long.class,   Wrapper.LONG),
+ F_TYPE('F', float.class,  Wrapper.FLOAT),
+ D_TYPE('D', double.class, Wrapper.DOUBLE),  // all primitive types
+ V_TYPE('V', void.class,   Wrapper.VOID);// not valid in all 
contexts

I would suggest to not name them X_TYPE but give them meaningful names like 
Int, Float, Void.  The enum BasicType already infers that it’s a type.  If you 
think it’s not clear that it’s a type just use BasicType.Double in that places.

On Mar 24, 2014, at 9:29 AM, 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

On 3/22/14 2:08 AM, Vladimir Ivanov wrote:

John, thanks for the feedback.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02

Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.

Best regards,
Vladimir Ivanov

On 3/21/14 10:54 PM, John Rose wrote:

On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
wrote:


Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/


That looks nice.  Strong typing; who woulda' thunk it.  :-)

The uses of .ordinal() are the extra cost relative to using just small
integers.  They seem totally reasonable in the code.

I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a common
name.

Don't make the enum class public.  And especially don't make the mutable
arrays public:

+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };

— John

P.S.  That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:

+public static final BasicType final[] ALL_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE };



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


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


___
mlvm-dev mailing list
mlvm-...@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

2014-03-25 Thread Christian Thalinger
+ enum BasicType {
+ L_TYPE('L', Object.class, Wrapper.OBJECT),  // all reference types
+ I_TYPE('I', int.class,Wrapper.INT),
+ J_TYPE('J', long.class,   Wrapper.LONG),
+ F_TYPE('F', float.class,  Wrapper.FLOAT),
+ D_TYPE('D', double.class, Wrapper.DOUBLE),  // all primitive types
+ V_TYPE('V', void.class,   Wrapper.VOID);// not valid in all 
contexts

I would suggest to not name them X_TYPE but give them meaningful names like 
Int, Float, Void.  The enum BasicType already infers that it’s a type.  If you 
think it’s not clear that it’s a type just use BasicType.Double in that places.

On Mar 24, 2014, at 9:29 AM, 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
 
 On 3/22/14 2:08 AM, Vladimir Ivanov wrote:
 John, thanks for the feedback.
 
 Updated webrev:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02
 
 Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.
 
 Best regards,
 Vladimir Ivanov
 
 On 3/21/14 10:54 PM, John Rose wrote:
 On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
 vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
 wrote:
 
 Thanks for the feedback.
 
 What do you think about the following:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
 
 That looks nice.  Strong typing; who woulda' thunk it.  :-)
 
 The uses of .ordinal() are the extra cost relative to using just small
 integers.  They seem totally reasonable in the code.
 
 I suggest either wrapping ordinal as something like id or else
 changing temp names like id to ord, to reinforce the use of a common
 name.
 
 Don't make the enum class public.  And especially don't make the mutable
 arrays public:
 
 +public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
 J_TYPE, F_TYPE, D_TYPE, V_TYPE };
 +public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
 J_TYPE, F_TYPE, D_TYPE };
 
 — John
 
 P.S.  That would only be safe if we had (what we don't yet) a notion of
 frozen arrays like:
 
 +public static final BasicType final[] ALL_TYPES = { L_TYPE,
 I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
 +public static final BasicType final[] ARG_TYPES = { L_TYPE,
 I_TYPE, J_TYPE, F_TYPE, D_TYPE };
 
 
 
 ___
 mlvm-dev mailing list
 mlvm-...@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-...@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

2014-03-24 Thread Vladimir Ivanov

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

On 3/22/14 2:08 AM, Vladimir Ivanov wrote:

John, thanks for the feedback.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02

Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.

Best regards,
Vladimir Ivanov

On 3/21/14 10:54 PM, John Rose wrote:

On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
wrote:


Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/


That looks nice.  Strong typing; who woulda' thunk it.  :-)

The uses of .ordinal() are the extra cost relative to using just small
integers.  They seem totally reasonable in the code.

I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a common
name.

Don't make the enum class public.  And especially don't make the mutable
arrays public:

+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };

— John

P.S.  That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:

+public static final BasicType final[] ALL_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE };



___
mlvm-dev mailing list
mlvm-...@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

2014-03-21 Thread Vladimir Ivanov

Chris,

Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/

Best regards,
Vladimir Ivanov

On 3/19/14 5:18 AM, Christian Thalinger wrote:


On Mar 18, 2014, at 2:35 PM, John Rose john.r.r...@oracle.com
mailto:john.r.r...@oracle.com wrote:


On Mar 18, 2014, at 1:36 PM, Christian Thalinger
christian.thalin...@oracle.com
mailto:christian.thalin...@oracle.com wrote:


Why are we not using an Enum instead of an untyped byte?


Byte is moderately typed, in the sense (which I rely on during
development) that you can't assign an int or char to a byte w/o a cast.
That's why it is not just a plain int.

But those values (L_TYPE etc.) are used a lot as numbers, and
specifically as low-level array indexes, and also comparisons (x 
V_TYPE).

To turn them into enums, we'd have to add lots of calls to
'.ordinal()' to turn them right back to numbers.  That dilutes
(completely IMO) the value they have as enums to raise the level of
the code.


But without being strongly typed we get bugs like this one:

  @Override
-MethodHandle bindArgument(int pos, char basicType, Object value) {
+MethodHandle bindArgument(int pos, byte basicType, Object value) {
  // If the member needs dispatching, do so.
  if (pos == 0  basicType == 'L') {

I’m just saying that for the sake of maintainability and correctness an
Enum would be better.



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




___
mlvm-dev mailing list
mlvm-...@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

2014-03-21 Thread John Rose
On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Thanks for the feedback.
 
 What do you think about the following:
 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/

That looks nice.  Strong typing; who woulda' thunk it.  :-)

The uses of .ordinal() are the extra cost relative to using just small 
integers.  They seem totally reasonable in the code.

I suggest either wrapping ordinal as something like id or else changing 
temp names like id to ord, to reinforce the use of a common name.

Don't make the enum class public.  And especially don't make the mutable arrays 
public:

+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE, J_TYPE, 
F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE, J_TYPE, 
F_TYPE, D_TYPE };

— John

P.S.  That would only be safe if we had (what we don't yet) a notion of frozen 
arrays like:

+public static final BasicType final[] ALL_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE };



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

2014-03-21 Thread Remi Forax

On 03/21/2014 07:54 PM, John Rose wrote:
On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov 
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com 
wrote:



Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/ 
http://cr.openjdk.java.net/%7Evlivanov/8037210/webrev.01/


That looks nice. Strong typing; who woulda' thunk it. :-)

The uses of .ordinal() are the extra cost relative to using just 
small integers. They seem totally reasonable in the code.


I suggest either wrapping ordinal as something like id or else 
changing temp names like id to ord, to reinforce the use of a 
common name.


Don't make the enum class public. And especially don't make the 
mutable arrays public:


+ public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+ public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE };


— John

P.S. That would only be safe if we had (what we don't yet) a notion of 
frozen arrays like:


+ public static final BasicType final[] ALL_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+ public static final BasicType final[] ARG_TYPES = { L_TYPE, I_TYPE, 
J_TYPE, F_TYPE, D_TYPE };


or
public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE, J_TYPE, 
F_TYPE, D_TYPE }.frozen();


because frozen() is a dynamic property.

Rémi



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

2014-03-21 Thread Vladimir Ivanov

John, thanks for the feedback.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02

Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.

Best regards,
Vladimir Ivanov

On 3/21/14 10:54 PM, John Rose wrote:

On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com wrote:


Thanks for the feedback.

What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/


That looks nice.  Strong typing; who woulda' thunk it.  :-)

The uses of .ordinal() are the extra cost relative to using just small
integers.  They seem totally reasonable in the code.

I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a common
name.

Don't make the enum class public.  And especially don't make the mutable
arrays public:

+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };

— John

P.S.  That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:

+public static final BasicType final[] ALL_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE };



___
mlvm-dev mailing list
mlvm-...@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

2014-03-18 Thread Christian Thalinger

On Mar 14, 2014, at 4:28 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8037210/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8037210
 953 lines changed: 425 ins; 217 del; 311 mod
 
 This is a massive cleanup of JSR292 code to replace char-based description of 
 basic types by numeric constants.

Why are we not using an Enum instead of an untyped byte?

 
 Contributed-by: john.r.r...@oracle.com
 
 Testing: jdk/java/{lang/invoke,util}, vm.mlvm.testlist, nashorn, jruby
 Flags: -ea -esa -Xverify:all -D...COMPILE_THRESHOLD={0,30}
 
 Best regards,
 Vladimir Ivanov
 ___
 mlvm-dev mailing list
 mlvm-...@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

2014-03-18 Thread John Rose
On Mar 18, 2014, at 1:36 PM, Christian Thalinger 
christian.thalin...@oracle.com wrote:

 Why are we not using an Enum instead of an untyped byte?

Byte is moderately typed, in the sense (which I rely on during development) 
that you can't assign an int or char to a byte w/o a cast.
That's why it is not just a plain int.

But those values (L_TYPE etc.) are used a lot as numbers, and specifically as 
low-level array indexes, and also comparisons (x  V_TYPE).

To turn them into enums, we'd have to add lots of calls to '.ordinal()' to turn 
them right back to numbers.  That dilutes (completely IMO) the value they have 
as enums to raise the level of the code.

— John

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

2014-03-18 Thread Christian Thalinger

On Mar 18, 2014, at 2:35 PM, John Rose john.r.r...@oracle.com wrote:

 On Mar 18, 2014, at 1:36 PM, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 Why are we not using an Enum instead of an untyped byte?
 
 Byte is moderately typed, in the sense (which I rely on during development) 
 that you can't assign an int or char to a byte w/o a cast.
 That's why it is not just a plain int.
 
 But those values (L_TYPE etc.) are used a lot as numbers, and specifically as 
 low-level array indexes, and also comparisons (x  V_TYPE).
 
 To turn them into enums, we'd have to add lots of calls to '.ordinal()' to 
 turn them right back to numbers.  That dilutes (completely IMO) the value 
 they have as enums to raise the level of the code.

But without being strongly typed we get bugs like this one:
 @Override
-MethodHandle bindArgument(int pos, char basicType, Object value) {
+MethodHandle bindArgument(int pos, byte basicType, Object value) {
 // If the member needs dispatching, do so.
 if (pos == 0  basicType == 'L') {
I’m just saying that for the sake of maintainability and correctness an Enum 
would be better.

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