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

[Leonid Mesnik]

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