Request for review 8000662: NPG: nashorn ant clean test262 out-of-memory with Java heap

[Christian Thalinger]

On Nov 28, 2012, at 4:43 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:

> 
> Please review updated changes to fix the nashorn OOMs from using jsr 292 anonymous classes.   The changes to fix bug 8003720 NPG: Method in interpreter stack frame can be deallocated <https://jbs.oracle.com/bugs/browse/JDK-8003720>  have been removed from this changeset in favor of the change that Stefan checked in on Monday.   Note - there is no target-specific code in this change now.
> 
> Other changes since last webrev are several cleanups from internal discussions.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8000662_3/
> bug link at http://bugs.sun.com/view_bug.do?bug_id=8000662_3
> 
> Retesting with NSK vm tests.   I'll try to describe things better below in reply to David's questions.

src/share/vm/prims/unsafe.cpp:

+   // this point.   The mirror any any instances of this class have to keep

Typo.

Otherwise this looks good.  Thanks for fixing it.

-- Chris

> 
> On 11/11/2012 11:32 PM, David Holmes wrote:
>> Hi Coleen,
>> 
>> :)
>> 
>> Can you explain the old and new relationships between these various objects please. As I understand it JSR-292 introduced this special kind of class - the anonymous class - and they differ from regular classes in important ways (mainly that they can be collected before their classloader), and pre-NPG this all "just worked". The NPG changes added the ClassLoaderData metadata objects and changed some of the associations that anonymous classes actually relied upon and as a result they stopped getting GC'd in some cases, and were prematurely GC'd in others (I hope I got that right). This changeset addresses these problems.
> 
> This is correct.  I don't think I could have written this better. Yes, in the old Permgen world, when the MethodHandles that contained the mirror to the anonymous class was unreferenced, the klass and all the metadata associated with it would be collected.   Now with ClassLoaderData objects, collection of metadata is tied to these objects.  The initial naive implementation just added the anonymous classes to the host_klass's ClassLoaderData object, which before b63 (I think) was the boot class loader.  So the anonymous classes were never collected.
> 
> This changeset gives each anonymous class it's own ClassLoaderData object and uses it's java_mirror to determine whether the anonymous class is still live and whether the ClassLoaderData object can be unloaded.
> 
> We have been doing some tuning to determine how large the metaspace should be for these special cases.   That is ongoing.
> 
>> 
>> I've been trying to follow this from the beginning and still don't have a clear understanding on what the relationships between ClassLoader, Class, "anonymous class" and ClassLoaderData objects are. So a clear picture of what relationships exist (particularly this new 1:N association) would be appreciated, and help make the code changes more understandable to me.
> 
> I can't draw pictures without a whiteboard!  But you may have several CLD objects that correspond to one class_loader oop.  Each CLD object points to the class_loader, but the class_loader only points to the primary CLD object.   The additional CLD objects are for anonymous classes, which are kept live by their mirrors, not the class_loader.  That's the source of many of the changes in this changeset.
> 
> Thanks,
> Coleen
> 
>> 
>> Thanks,
>> David
>> 
>> On 12/11/2012 1:51 PM, Coleen Phillimore wrote:
>>> 
>>> This change creates a ClassLoaderData object for each JSR 292 anonymous
>>> class, so that the metadata and mirror object for the anonymous class
>>> can be reclaimed when the anonymous class is no longer referenced. The
>>> java_lang_ClassLoader object for the anonymous class is the same as its
>>> host_class. Most of this change is to break the assumption that there's
>>> a 1-1 relationship between java_lang_ClassLoader Java objects and
>>> ClassLoaderData metadata objects in the VM. Also, nmethods and other
>>> things that were strong roots for java_lang_ClassLoader objects have to
>>> also be strong roots for java_lang_Class objects for anonymous classes.
>>> 
>>> There were also changes to move the dependencies out of the
>>> java_lang_ClassLoader object to the ClassLoaderData type. This type is
>>> preallocated so that CMS will have card marks to track additions to the
>>> dependencies. Please check this, Stefan!
>>> 
>>> Also, in this change is the addition of mirrors to the interpreter frame
>>> and a test case that shows why this is necessary. An interpreter frame
>>> can be running while it's class loader is unloaded in some special
>>> circumstances. It is easier to do this with JSR292 static MethodHandle
>>> code. Some people are looking for a platform independent way to do this,
>>> by changing code in GC. While this target-dependent interpreter code is
>>> unfortunate, the concept is simple. If the latter effort succeeds, we
>>> can remove the mirror from the interpreter frame later. A note to
>>> openjdk developers - I added this mirror to zero but not to shark. More
>>> testing is necessary.
>>> 
>>> Please review the following change:
>>> 
>>> Summary: Add ClassLoaderData object for each anonymous class with
>>> metaspaces to allocate in. Add mirror interpreter frame.
>>> 
>>> http://cr.openjdk.java.net/~coleenp/8000662/
>>> http://bugs.sun.com/view_bug.do?bug_id=8000662
>>> 
>>> Tested with Nashorn tests, NSK full testlist, dacapo with CMS and G1.
>>> 
>>> Thanks,
>>> Coleen
>