Default methods for jdk8: request for code review

[David Holmes]

Hi Keith,

I'm afraid that is far too extensive a set of changes to provide an 
actual Review on. I've read the design doc and flicked through the code, 
but that's as far as I can go.

One concern I do have:

src/share/vm/utilities/growableArray.hpp

You changed the at(i) access methods to return an object reference 
rather than (I presume) a copy. But is that a safe change to make?

Other general comments:

src/share/vm/runtime/reflection.cpp
src/share/vm/classfile/systemDictionary.hpp
src/share/vm/classfile/vmSymbols.hpp

The changes here doesn't seem to be related to default methods.

---

src/share/vm/oops/method.hpp

Would it be worthwhile defining "MethodType methodType()" directly 
rather than is_overpass() and avoid the bool to MethodType conversion 
when calling allocate() ?

---

src/share/vm/classfile/classFileParser.cpp

Some of your line reformatting is making things more obscure and 
inconsistent in my view. Eg I find this:

   klassVtable::compute_vtable_size_and_num_mirandas(vtable_size,
                                                     num_miranda_methods,
                                                     super_klass(),
                                                     methods,
                                                     access_flags,
                                                     class_loader,
                                                     class_name,
                                                     local_interfaces,
                                                     CHECK_(nullHandle));

much clearer than:

   klassVtable::compute_vtable_size_and_num_mirandas(
       &vtable_size, &num_miranda_methods, &all_mirandas, super_klass(), 
methods,
       access_flags, class_loader, class_name, local_interfaces,
                                                     CHECK_(nullHandle));

Ditto for changes elsewhere eg src/share/vm/oops/klassVtable.cpp

---

src/share/vm/classfile/classFileParser.hpp

+                                 bool* has_default_methods,
+                                 bool* has_default_method,

why the inconsistency in the plurality of the name?

---

src/share/vm/oops/instanceKlass.cpp

Interfaces are still stateless so what initialization is required? Are 
these synthetic initialization routines inserted by javac?

David
-----

On 19/10/2012 9:24 PM, Keith McGuigan wrote:
>
> Anyone?
>
> On 10/10/2012 1:12 PM, Keith McGuigan wrote:
>> Hello,
>>
>> I'd like any review of the code which implements default methods in the
>> JVM.  This is destined for jdk8 as part of JSR 335 (Lambdas), and
>> tracked by CR 7200776.
>>
>> The design and implementation is described in this short document:
>> http://cr.openjdk.java.net/~kamg/default_methods_in_hotspot.txt
>>
>> And the code is here:
>> http://cr.openjdk.java.net/~kamg/default_methods/
>>
>> Any review (even partial) would be appreciated.  Thanks!
>>
>> --
>> - Keith
>