Review request: 8015428: Remove unused CDS support from StringTable

[Stefan Karlsson]

On 05/28/2013 10:58 AM, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2013-05-27 at 19:49 +0200, Stefan Karlsson wrote:
>> On 2013-05-27 16:15, Thomas Schatzl wrote:
>>
>>> Hi,
>>>
>>> On Mon, 2013-05-27 at 13:35 +0200, Stefan Karlsson wrote:
>>>> http://cr.openjdk.java.net/~stefank/8015428/webrev.00/
>>> Looks fine.
> (I had a look at http://cr.openjdk.java.net/~stefank/8015428/webrev.01/
> in the meantime. Looks good too btw)
>
>>> The new code for StringTable::oops_do() does not support that the
>>> closure unlinks the literal - apparently this has been removed because
>>> there is no closure that did that.
>> Yes. IMHO it doesn't make sense to have that kind of side-effect in a
>> oops_do function, unless you really need it.
>>
> Sure.
>
>>> Maybe it is useful to add an assert for that case anyway.
>> I can move down down the assert(entry->literal() != NULL,...) to after
>> the f->do_oop(...) if you want to.
>>
>> However, when thinking about this a bit more, I'd actually like to
>> remove that assert all together. There's no reason for
>> StringTable::oops_do to restrict the oops to be non-NULL. I think that
>> part can be handled by the verification code that's already present:
>>   776 void StringTable::verify() {
>>   777   for (int i = 0; i < the_table()->table_size(); ++i) {
>>   778     HashtableEntry<oop, mtSymbol>* p = the_table()->bucket(i);
>>   779     for ( ; p != NULL; p = p->next()) {
>>   780       oop s = p->literal();
>>   781       guarantee(s != NULL, "interned string is NULL");
> That's fine with me. You're right, StringTable::verify() and
> Universe::verify() which calls that are used if VerifyDuringGC is
> enabled.

Thanks.

StefanK

> Thomas
>