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

[Leonid Mesnik]

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
>>>
>>>
>
>


-- 
Leonid Mesnik
JVM SQE