Request for reviews (M): 6924259: Remove String.count, String.offset fields

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri May 11 09:34:21 PDT 2012


Roland Westrelin wrote:
>> http://cr.openjdk.java.net/~kvn/6924259/webrev
>>
>> 6924259: Remove String.count/String.offset
>>
>> Allows a version of the String class that doesn't have count,offset 
>> fields.
>> These VM changes works with both versions of String class by checking the
>> presence of fields.
>>
>> Initial changes were done by Brian and Tom years ago. I addapted it 
>> for current
>> sources and tested it.
>>
>> Tested with NSK, regression tests, CTW, refworkload, CDS (class sharing).
> 
> Looks good to me (but I'm not a reviewer).

Why not? Even small comments and suggestions are considered as review. Tom 
reviewed these changes before but I look for other opinions also.

> 
> Maybe an assert(!java_lang_String::has_offset_field()) in 
> GraphKit::store_String_offset() and GraphKit::store_String_length()?

Accessors java_lang_String::count_offset_in_bytes() and 
java_lang_String::offset_offset_in_bytes() have that assert already.

> 
> Interestingly, it doesn't look like the c1 code is used anymore. I'm not 
> sure what the history is here.

I looked on history and the usage was removed when C1 switched to new linear RA. 
Anyway even if it is not used it should be correct.

Thanks,
Vladimir

> 
> Roland.


More information about the hotspot-dev mailing list