Review request: 8015428: Remove unused CDS support from StringTable

[Thomas Schatzl]

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.

Thomas