review request (S/M): 6962931 move interned strings out of perm gen

[John Coomes]

Tom Rodriguez (tom.rodriguez at oracle.com) wrote:
> On Feb 17, 2011, at 8:21 AM, Coleen Phillimore wrote:
> > The option ScavengeRootsInCode is a bit confusing since this is
> > moving all strings out of permgen so shouldn't depend on a flag
> > that has meaning to the compiler.  I'd like to see the code
> > conditional on this flag removed.
> 
> Currently ScavengeRootsInCode controls two things.  One is the GC
> machinery that supports embedding scavengeable oops into compiled
> code and the other is which kinds of oops the compiler considers to
> be embeddable.  The first part will become required behaviour by the
> end of 7 since invokedynamic requires it so at some point all tests
> which are comparing ScavengeRootsInCode with 0 should be simplified
> so that it's no longer optional.  We could do that now but it might
> be worth waiting a little while so that we can use that flag to
> narrow down problems by turning it off.
> 
> Anyway, I agree that controlling it through ScavengeRootsInCode is
> odd.  Maybe we need a separate temporary flag for putting Strings
> and Classes in perm?  Until perm goes away completely we can still
> allocate them there and things would continue to work fine.

Background for the list:  Tom's changes for 7017732 (move static
fields to Class) introduced globals.hpp var JavaObjectsInPerm, which
sets ScavengeRootsInCode to 1 if necessary.

After syncing with 7017732, I've updated the interned strings code to
use JavaObjectsInPerm; I also moved PSScavengeRootsClosure so we don't
need a second copy in psScavenge.cpp.  The updated webrev is

http://cr.openjdk.java.net/~jcoomes/6962931-intern/

For reviewers of the previous round, that page has a link to a
separate webrev with just the post-sync changes.

> > The other comment I have is based on ignorance.  Isn't "tenured'
> > the old generation?  So create_tenured_from_unicode() calls
> > basic_create() with a tenured flag and it either allocates the
> > string in permgen or eden, so it doesn't make sense if I have the
> > correct meaning of tenured.
> 
> That's a long time oddity.  I think it probably dates to the time
> when there was no perm gen and meta data was simply in tenured mixed
> with other data.  It will be corrected by the time we are done since
> it will simply go away.

I'll add a note to the perm gen removal project page so it doesn't get
forgotten.

-John

> > This would be fixed if the conditional code is removed also, if not, the names should be changed.  There is also a create_from_unicode() that is uncalled, presumably because someone added create_tenured_from_unicode().
> > 
> > Thanks,
> > Coleen
> > 
> > On 2/16/2011 2:05 PM, Tom Rodriguez wrote:
> >> I had assumed we'd be switching so that strings were always outside perm by default.  ScavengeRootsInCode must be != 0 once invokedynamic is enabled by default and since that's definitely going to be enabled in 7 don't we want to turn all this on?
> >> 
> >> tom
> >> 
> >> On Feb 16, 2011, at 10:10 AM, John Coomes wrote:
> >> 
> >>> This moves interned strings from the perm gen into the regular heap.
> >>> The code was written by Tom Rodriguez; I've added a minor fix and done
> >>> the testing.  More info in the webrev:
> >>> 
> >>> http://cr.openjdk.java.net/~jcoomes/6962931-intern/
> >>> 
> >>> Thanks for any feedback.
> >>> 
> >>> -John
> >>> 
> > 
>