Review request: 8015428: Remove unused CDS support from StringTable

Stefan Karlsson stefan.karlsson at oracle.com
Mon May 27 10:49:45 PDT 2013


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.

Thanks.

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

> 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");


thanks,
StefanK

>
> Thanks,
> Thomas
>
>



More information about the hotspot-dev mailing list