RFR 8031820: NPG: Fix remaining references to metadata as oops in comments
Coleen Phillimore
coleen.phillimore at oracle.com
Fri Mar 21 15:57:23 UTC 2014
Thank you Stefan for reading the new comments!
On 3/21/14 5:02 AM, Stefan Karlsson wrote:
>
> 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.
You're right. This was an old comment moved. I reworked it a bit. See:
http://cr.openjdk.java.net/~coleenp/8031820_2
>
>
> 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?
>
I took it out.
>
> 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);
>
It's historical baggage when they were oops but probably
MetadataOnStackMark needs these to be handles, so I've left them in. I
think slowly we can turn them into instance member functions but not yet.
Thanks!
Coleen
>
> 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