RFR: JEP 359-Records: hotspot runtime and serviceability code

Lois Foltan lois.foltan at oracle.com
Mon Oct 21 18:40:44 UTC 2019


On 10/18/2019 2:44 PM, Vicente Romero wrote:
> Hi,
>
> Please review the hotspot runtime and serviceability code for JEP 359 
> (Records).
>
> Thanks in advance for the feedback,
> Vicente
>
> PS, Thanks to Harold for the development
>
>
> [1] 
> http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/

Harold, Vicente,

Overall this looks good!  Some comments:

src/hotspot/share/classfile/classFileParser.cpp
- line #3226: I mentiond this below that I would like to see the comment 
from jvmtiClassFileReconstituter.cp ahead of
ClassFileParser::parse_classfile_record_attribute() so it is easy to 
follow the expected layout.
- line #3275: Ahead of the for loop it would be good to add a comment 
listing what the expected attributes are for Records.
- line #4732: The added check of "!major_gte_14" looks incorrect for 
final abstract classes.  That isn't relevant to Records, correct?

src/hotspot/share/prims/jvm.cpp
- line #1737 - #1739: use oopFactory::new_objArrayHandle() instead of 
breaking this accross 2 lines.
- line #1751: please add a comment that indicates the behavior when the 
InstanceKlass is not a record.  It seems like an empty array is returned?

src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp
- copyright update needed.
- line #427-438: I like the comment, can you add that ahead of 
ClassFileParser::parse_classfile_record_attribute().

src/hotspot/share/prims/jvmtiRedefineClasses.cpp
- line #839 - comment "of" should be "if"?

src/hotspot/share/oops/recordComponent.hpp
- line #95 - determination of TBD comment needed before committing.

I still need to review the tests.

Thanks,
Lois



More information about the amber-dev mailing list