On 2016-08-18 16:06, Aleksey Shipilev wrote:
On 08/18/2016 12:33 AM, Claes Redestad wrote:
Hi,

please review this change which adds pre-generation of simple
DelegatingMethodHandles corresponding to the DirectMethodHandles we
already pre-generate during linking.

webrev: http://cr.openjdk.java.net/~redestad/8164044/webrev.02/
*) This block:

   if (!dedupSet.contains(methodTypes[i])) {
      // do stuff
      dedupSet.add(methodTypes[i]);
   }

Is replaceable by:

   if (dedupSet.add(methodTypes[i])) {
      // do stuff
   }

Sure!


*) Can save instantiation here, by moving ptypes into else branch:

  333         Class<?>[] ptypes = new Class<?>[parameters.length()];
  334         if (ptypes.length == 0) {
  335             return MethodType.methodType(rtype);
  336         } else {
  337             for (int i = 0; i < ptypes.length; i++) {
  338                 ptypes[i] = simpleType(parameters.charAt(i));
  339             }
  340             return MethodType.methodType(rtype, ptypes);
  341         }
  342     }

Efficiency of code only called at link time isn't too important,
but it also looks cleaner: fixed.


*) Many whitespaces from this switch:

195         switch (which) {
  196         case LF_INVVIRTUAL:    linkerName = "linkToVirtual";   kind
= DIRECT_INVOKE_VIRTUAL;     break;
  197         case LF_INVSTATIC:     linkerName = "linkToStatic";    kind
= DIRECT_INVOKE_STATIC;      break;
  198         case LF_INVSTATIC_INIT:linkerName = "linkToStatic";    kind
= DIRECT_INVOKE_STATIC_INIT; break;
  199         case LF_INVSPECIAL:    linkerName = "linkToSpecial";   kind
= DIRECT_INVOKE_SPECIAL;     break;
  200         case LF_INVINTERFACE:  linkerName = "linkToInterface"; kind
= DIRECT_INVOKE_INTERFACE;   break;
  201         case LF_NEWINVSPECIAL: linkerName = "linkToSpecial";   kind
= DIRECT_NEW_INVOKE_SPECIAL; break;
  202         default:  throw new InternalError("which="+which);

...ran away to this one:

  154     private static Kind whichKind(int whichCache) {
  155         switch(whichCache) {
  156             case MethodTypeForm.LF_REBIND:            return
BOUND_REINVOKER;
  157             case MethodTypeForm.LF_DELEGATE:          return DELEGATE;
  158             default:                                  return REINVOKER;
  159         }
  160     }

I assume you want less whitespace?

http://cr.openjdk.java.net/~redestad/8164044/webrev.03/

Thanks!

/Claes


Thanks,
-Aleksey



Reply via email to