RFR: 8009763 Add WB test for String.intern()

Zhengyu Gu zhengyu.gu at oracle.com
Wed Mar 27 12:49:42 PDT 2013


Hi Leonid,

symbolTable.cpp #687
   Why converts to "unsigned int"?  it appears that hash_to_index() 
returns "int", and the_table()->lookup() takes "int"

Other than that, good to me.

-Zhengyu

On 3/22/2013 3:07 AM, Leonid Mesnik wrote:
> Are there any other comments?
>
> Leonid
> On 03/20/2013 03:41 PM, Leonid Mesnik wrote:
>> Hi
>>
>> Here is next attempt:
>> http://cr.openjdk.java.net/~mgerdin/lmesnik/webrev.2/ 
>> <http://cr.openjdk.java.net/%7Emgerdin/lmesnik/webrev.2/>
>>
>> The StringTable::lookup methods were updated.
>>
>> Leonid
>> On 03/14/2013 06:38 PM, Mikael Gerdin wrote:
>>> Lenoid,
>>>
>>> On 2013-03-13 11:14, Leonid Mesnik wrote:
>>>> Hi
>>>>
>>>> werbrev was updated (thank you for this)
>>>> http://cr.openjdk.java.net/~mgerdin/lmesnik/webrev.1/
>>>> <http://cr.openjdk.java.net/%7Emgerdin/lmesnik/webrev.1/>
>>>
>>> Also, I don't know why you have "StringTabe::" inside these calls:
>>>
>>>    unsigned int hash = StringTable::hash_string(name, len);
>>>    unsigned int index = StringTable::the_table()->hash_to_index(hash);
>>>    return StringTable::the_table()->lookup(index, name, len, hash);
>>>
>>> Since you are in an instance method in StringTable you also 
>>> shouldn't get the_table() singleton, but rather use the current 
>>> instance.
>>>
>>>
>>> I think your new lookup() function could be static just as the other 
>>> public lookup() function.
>>>
>>> Another idea is to perhaps make these two lookup:s share the same 
>>> implementation:
>>>
>>> oop StringTable::lookup(Symbol* symbol) {
>>>    ResourceMark rm;
>>>    int length;
>>>    jchar* chars = symbol->as_unicode(length);
>>>    return lookup(chars, length);
>>> }
>>>
>>> oop StringTable::lookup(const jchar* chars, int length) {
>>>    unsigned int hashValue = hash_string(chars, length);
>>>    int index = the_table()->hash_to_index(hashValue);
>>>    return the_table()->lookup(index, chars, length, hashValue);
>>> }
>>>
>>> /Mikael
>>>
>>>>
>>>> JPRT passed with -rtest '*-*-*-runtime/interned'
>>>>
>>>> Leonid
>>>> On 03/12/2013 05:05 PM, Mikael Gerdin wrote:
>>>>> Hi Leonid,
>>>>>
>>>>> On 2013-03-12 08:25, Leonid Mesnik wrote:
>>>>>> Hi
>>>>>> I've added a small test which:
>>>>>> 1) creates java string and intern it
>>>>>> 2) verify that it presents in StringTable
>>>>>> 3) clear reference and call fullGC
>>>>>> 4) verify that there is no such string in StringTable
>>>>>>
>>>>>> Here is webrev:
>>>>>> http://cr.openjdk.java.net/~mgerdin/lmesnik/webrev.0/
>>>>>
>>>>> Overall this looks reasonable.
>>>>>
>>>>> I have some comments though:
>>>>> WB_IsInStringTable is defined to return a jboolean 
>>>>> (Ljava/lang/String;)Z
>>>>> but the actual function is defined to return an jint. This will
>>>>> probably work but you should fix this to make it consistent.
>>>>>
>>>>> The function WB_fullGC should have a capital F to be consistent with
>>>>> the naming of the other WB_* functions.
>>>>>
>>>>>>
>>>>>> Tested locally and by JPRT
>>>>>>
>>>>> did you run jprt with -retest '*-*-*-runtime/interned' as well?
>>>>>
>>>>> /Mikael
>>>>
>>>>
>>
>>
>
>



More information about the hotspot-runtime-dev mailing list