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

[Harold Seigel]

Thanks Coleen!

Please see comments inline.

On 10/21/2019 6:19 PM, coleen.phillimore at oracle.com wrote:
>
> http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/src/hotspot/share/oops/instanceKlass.cpp.udiff.html 
>
>
> +void InstanceKlass::deallocate_record_components(ClassLoaderData* 
> loader_data,
> + Array<RecordComponent*>* record_components) {
> + if (record_components != NULL && !record_components->is_shared()) {
> + for (int i = 0; i < record_components->length(); i++) {
> + RecordComponent* record_component = record_components->at(i);
> + if (record_component == NULL) continue; // maybe null if error 
> processing
> + MetadataFactory::free_metadata(loader_data, record_component);
> + }
> + MetadataFactory::free_array<RecordComponent*>(loader_data, 
> record_components);
> + }
> +}
> +
>
> free_metadata<> already has a NULL check so you don't need the line 
> with the continue.
Removed.
>
> http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/src/hotspot/share/classfile/javaClasses.cpp.udiff.html 
>
>
> Can you add a HandleMark hm(THREAD); to the beginning of 
> java_lang_RecordComponent::create, since it creates Handles.
Done.
>
> http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/hotspot/jtreg/runtime/records/recordAttribute.jcod.html 
>
>
> This is a nice test but could you split the jcod files into one per 
> class with a short comment of the result you expect when loaded?
Done.
>
> http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang/instrument/RedefineRecordAttrGenericSig/TestRecordAttrGenericSig.java.html 
>
>
> The bytesForHostClass(String) variant should be added to 
> NamedBuffer.java too and replace the one in 
> RedefineNestmateAttr/TestNestmateAttr.java.  This other variant of 
> this function is there so this should be there too.
I plan to delay doing this change because bytesForHostClass(String) 
calls compile(String) and compile(String) differs between the 
NestmateAttr and RecordAttr tests.
>
> This is really nice work, Harold!
Thanks!
Harold
>
> Coleen
>
>
> On 10/18/19 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/
>