RFR 8031820: NPG: Fix remaining references to metadata as oops in comments

Stefan Karlsson stefan.karlsson at oracle.com
Fri Mar 21 09:02:02 UTC 2014


On 2014-03-20 20:39, Coleen Phillimore wrote:
> Also fixed:
> 8012125: Comments for ConstantPoolCache should reflect the addition of 
> resolved_references in ConstantPool
> Summary: Updated comments in metadata header files, and renamed 
> this_oop variables to this_cp or this_k when referring to constant 
> pool or classes.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8031820/

Looks good. Thanks for cleaning this up!


Small comments:

http://cr.openjdk.java.net/~coleenp/8031820/src/share/vm/oops/constMethod.hpp.frames.html

  42 // critical at all.  Accessing the checked exceptions table is used 
by reflection,
  43 // so we put that last to make access to it fast.

It doesn't look like we put the checked exceptions table at the end of 
the constMethod.


http://cr.openjdk.java.net/~coleenp/8031820/src/share/vm/oops/constantPool.hpp.udiff.html

    friend class VMStructs;
-  friend class BytecodeInterpreter;  // Directly extracts an oop in the 
pool for fast instanceof/checkcast
+  friend class BytecodeInterpreter;  // Directly extracts an klass in 
the pool for fast instanceof/checkcast
+
    friend class Universe;             // For null constructor

Do we need/want this extra blank line?


http://cr.openjdk.java.net/~coleenp/8031820/src/share/vm/oops/cpCache.hpp.udiff.html

I haven't verified that this comment is correct.


Not related to this patch, but is there a reason why all these functions 
are static and take a constantPoolHandle as the first parameter, instead 
of being instance member functions? My guess is that it's just to make 
sure MetadataOnStackMark finds this_cp.

+  static Method*          method_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static bool       has_appendix_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static oop            appendix_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static bool    has_method_type_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static oop         method_type_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static Klass*            klass_at_if_loaded      (constantPoolHandle this_cp, int which);
+  static Klass*        klass_ref_at_if_loaded      (constantPoolHandle this_cp, int which);


thanks,
StefanK

> bug link https://bugs.openjdk.java.net/browse/JDK-8031820
>
> Tested with jck, all jtreg hotspot tests and nsk vm.quick.testlist.
>
> Thanks,
> Coleen



More information about the hotspot-runtime-dev mailing list