JDK-8029567 : Clean up linkResolver code
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: hs25
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2013-12-04
  • Updated: 2023-01-24
  • Resolved: 2015-05-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b69Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Description
Recent changes for to meet the jdk8 deadline have left some cleanup items.  These are cosmetic but because the code is so complex now, the cosmetic problems make this code even harder to understand and more likely to create bugs.   See referenced bugs.

The specific things that need to be done is to fix the parameter lists in linkResolver to 1. not wrap multiple lines and 2. not pass non-const reference parameters because at the call site you have no idea that these are output parameters.

Other things in linkResolver, there are similar but different functions: lookup_instance_method_in_klasses vs. lookup_method_in_klasses.   The former skips statics.  It would be better to have an InstanceKlass::uncached_lookup_instance_method() which skips statics and that could be used instead of the following code in klassVtable.cpp also.

      Method* mo = InstanceKlass::cast(super)->lookup_method(name, signature);
+      while (mo != NULL && mo->access_flags().is_static()
+             && mo->method_holder() != NULL
+             && mo->method_holder()->super() != NULL)
+      {
+         mo = mo->method_holder()->super()->uncached_lookup_method(name, signature);
+      }

A cleanup pass of link resolution code should be done before the suggested complete rewrite so that it's clearer what it does.   This is technical debt.
Comments
URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/6b9feb52df5d User: lana Date: 2015-06-17 03:53:04 +0000
17-06-2015

URL: http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/6b9feb52df5d User: coleenp Date: 2015-05-29 21:16:13 +0000
29-05-2015

In linkResolver, there's Method* called result_oop which is a bad name since methods aren't oops anymore. These should be renamed. ! void LinkResolver::lookup_method_in_klasses(methodHandle& result, KlassHandle klass, Symbol* name, Symbol* signature, bool checkpolymorphism, bool in_imethod_resolve, TRAPS) { Method* result_oop = klass->uncached_lookup_method(name, signature); + + // JDK 8, JVMS 5.4.3.4: Interface method resolution should + // ignore static and non-public methods of java.lang.Object, + // like clone, finalize, registerNatives. + if (in_imethod_resolve && + result_oop != NULL && + klass->is_interface() && + (result_oop->is_static() || !result_oop->is_public()) && + result_oop->method_holder() == SystemDictionary::Object_klass()) { + result_oop = NULL; + } +
10-12-2013

ILW: MLH => P4 I: M, affects maintainability of the code L: L, no direct way to reproduce W: H
06-12-2013

+ // As of the fix for 4486457 we disable verification for all of the + // dynamically-generated bytecodes associated with the 1.4 + // reflection implementation, not just those associated with + // sun/reflect/SerializationConstructorAccessor. + bool is_reflect = JDK_Version::is_gte_jdk14x_version() && + UseNewReflection && + klass_to_check->is_subclass_of( + SystemDictionary::reflect_MagicAccessorImpl_klass()); Make this a function. With hsx gone, we don't need the is_gte_jdk14x check anymore.
04-12-2013