Default methods for jdk8: request for code review
coleen.phillimore at oracle.com
Fri Oct 19 10:23:04 PDT 2012
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)
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
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
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.
On 10/19/2012 7:24 AM, Keith McGuigan wrote:
> On 10/10/2012 1:12 PM, Keith McGuigan wrote:
>> 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:
>> And the code is here:
>> Any review (even partial) would be appreciated. Thanks!
>> - Keith
More information about the lambda-dev