RFR 8029567: Clean up linkResolver code

Jiangli Zhou jiangli.zhou at oracle.com
Wed May 27 02:15:40 UTC 2015


Hi Coleen,

I like the refactor changes. The code looks much cleaner. Here are some of my comments.

LinkInfo:
ciEnv::lookup_method(), ciField::will_link() and ciMethod::resolve_vtable_index() create LinkInfo with ‘_check_access’ set to true. It might be better to make ‘_check_access’ default to ‘true’ in the LinkInfo constructor.

Bytecode_invoke::static_target(), bytecode.cpp:
Bytecode_invoke::static_target() originally returns a null handle instead of ‘NULL’ if the method is not resolved. Now that’s changed, however caller of Bytecode_invoke::static_target() still expects null handle instead of ’NULL’.

LinkResolver::lookup_method_in_klasses(), linkResolver.cpp:
Same as Bytecode_invoke::static_target(). This now might return ‘NULL’ instead of null handle.

The lookup_polymorphic_method() and resolve_method() have the similar issue. It probably is better to make all those return ‘null handle’ like they originally do.

Indentation:
loaderConstraints.hpp line 68
loaderConstraints.cpp line 74
linkResolver.cpp line 440 - 444
systemDictionary.cpp line 2238 - 2239

Thanks,
Jiangli

On May 14, 2015, at 4:16 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:

> Summary: Moved non-const reference return values to actual return values, refactored error handling code, pass CLD rather than class_loader oop, remove oop from Method* variable names.
> 
> Also modified the long parameter lists into a class LinkInfo to hold information from the constant pool that is always passed through several layers of functions together.  Also reformatted to split parameter list lines to some reasonable width.
> 
> The type methodHandle should be passed as const reference types to avoid copy construction, because it has a non-trivial destructor. This sort of change could be made in more places in the JVM, but I stopped with linkResolver.
> 
> Ran all hotspot jtreg tests, jck lang/vm/api/java_lang tests, internal testbase tests: vm.quick.testlist, vm.defmeth.testlist, proposed new selection-resolution tests, jdk/java/lang/invoke tests (see RFR).
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8029567/
> bug link https://bugs.openjdk.java.net/browse/JDK-8029567
> 
> Thanks,
> Coleen



More information about the hotspot-runtime-dev mailing list