Default methods for jdk8: request for code review

[Coleen Phillimore]

bytecodeAssembler.hpp

Can you add to the top comment about why someone would use this class, 
and why you are using it.  I think if we have other generated methods, 
this is a useful class for people to add to rather than write their own 
in a different way.

BytecodeCPEntry is a VALUE_OBJ_CLASS_SPEC  (currently)

defaultMethods.cpp

line 775 - I think this  is the ResourceMark that's keeping everything 
alive right.  I think you should have a comment here that everything is 
resource allocated and not to add ResourceMark's in odd places.

KeepAliveRegistrar - I think this is a good solution to the problem of 
redefining classes while you are processing them.  It usually isn't a 
problem for the class you are finding default methods for because it's 
not in the system dictionary yet.  If a super class is redefined while 
you are processing the methods can it add an implementation for default 
or abstract method?   I think this isn't possible today until extended 
class redefinition.

783 - I think you want a ResourcMark() in here and 657.   These names 
printed might take up enough space for you to worry about running out 
while this is code is executed.

647 - if m is an overpass, why do you lookup the method again and why 
can it get a different answer.   A comment why would help because I 
don't know.

81 - seems like super_of() belongs in instanceKlass.hpp - just change 
the type of super() to return InstanceKlass.   Isn't _super always 
InstanceKlass (we can add this as another cleanup, but if you move your 
function we'll get it too in the cleanup).

48 - a nice comment about what these PseudoScopeMarks are for would be 
useful.   It's so you don't iterate recursively I think.

86 - print_slot and print_method seem like they should be under ifndef 
PRODUCT    And PrintHierarchy should be #ifndef PRODUCT rather than 
ASSERT.   Not that there's a good solid rule around these two 
preprocessor directives, but make sure optimized builds.

I am looking at HiearchyVisitor::run now.  I like how it perfectly fits 
on a page for me.   This looks really good so far.

Thanks,
Coleen

On 10/19/2012 7:24 AM, 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