RFR 8145628: hotspot metadata classes shouldn't use HeapWordSize or heap related macros like align_object_size

Jon Masamitsu jon.masamitsu at oracle.com
Thu Jan 28 20:41:49 UTC 2016


Changes look good.  Some minor points (for which I don't
need to see another webrev if you decide to make changes
based on these)

http://cr.openjdk.java.net/~coleenp/8145628.01/webrev/src/share/vm/oops/klassVtable.hpp.udiff.html

- static int size() { return sizeof(itableOffsetEntry) / HeapWordSize; } 
// size in words
+ static int size() { return sizeof(itableOffsetEntry)/wordSize; } // 
size in words

Surrounding code puts space around the operators.  In this case around 
the " / " seem
appropriate.


http://cr.openjdk.java.net/~coleenp/8145628.01/webrev/src/share/vm/utilities/globalDefinitions.cpp.frames.html

You added

88 assert(wordSize == BytesPerWord, "should be the same since they're 
used interchangeably");

Many of the changes were like the one above in klassVtable.hpp where 
HeapWordSize -> wordSize.
Do you want to assert "wordSize == HeapWordSize"?

Jon



On 01/27/2016 10:27 AM, Coleen Phillimore wrote:
> Summary: Use align_metadata_size, align_metadata_offset and 
> is_metadata_aligned for metadata rather
> than align_object_size, etc.  Use wordSize rather than HeapWordSize 
> for metadata.  Use align_ptr_up
> rather than align_pointer_up (all the related functions are ptr).
>
> Ran RBT quick tests on all platforms along with Chris's Plummers 
> change for 8143608, ran jtreg hotspot tests and nsk.sajdi.testlist 
> co-located tests because there are SA changes.   Reran subset of this 
> after merging.
>
> I have a script to update copyrights on commit. It's not a big change, 
> just mostly boring.  See the bug comments for more details about the 
> change.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8145628.01/
> bug link https://bugs.openjdk.java.net/browse/JDK-8145628
>
> thanks,
> Coleen
>
>



More information about the hotspot-dev mailing list