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

[Mikael Gerdin]

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