Project proposal: Remove the Permanent Generation

[David Schlosnagle]

On Tue, Mar 20, 2012 at 10:40 AM, Jon Masamitsu
<jon.masamitsu at oracle.com> wrote:
> You may have noticed that this project proposal never got
> any farther then this mail.  Somewhat due to indecision on
> our part, this work is not going to become its own project
> but will be integrated through the hsx/hotspot-gc/hotspot
> repository.  We wanted to provide a preview of the work
> so prepared this webrev from an recent merge of the perm gen
> removal work with hotspot-gc.  We're still working on it
> but thought this intermediate webrev would be of interest.
>
> http://cr.openjdk.java.net/~coleenp/metadata2

Jon,

I am very glad to see progress on the removal of perm gen from HotSpot
and the move to dynamic allocation of these metadata objects. I will
be very happy once I can remove the XX:MaxPermSize from all of our
deployment configurations, without switching everything to JRockit or
J9 :)

I took a look through some, but not all of the changes in this webrev,
and I had a few comments and questions.


agent/src/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java
  Minor spacing nit on line 416:
    int length = ( innerClassList == null)? 0 : (int) innerClassList.length();
                  ^                       ^

  There are a few places (lines 542-543, 572-574, 587-588, 679-681)
that could use the getAllFieldsCount() method instead of separate
calls to getFields() and length(), for example replacing:
    ShortArray fields = getFields();
    int length = (int) fields.length();
  with:
    int length = getAllFieldsCount();

  On line 589, the variable cp is never referenced, is this needed?
    ConstantPool cp = getConstants();

  On line 886, the variable fields is never referenced, is this needed?
    ShortArray fields = getFields();

agent/src/share/classes/sun/jvm/hotspot/tools/jcore/ClassWriter.java
  On lines 383-384, it could use InstanceKlass.getAllFieldsCount()

agent/src/share/classes/sun/jvm/hotspot/tools/soql/SOQL.java
  On lines 153-154, it could use InstanceKlass.getAllFieldsCount()

agent/src/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java
  On lines 1120-1121 and 1819-1820, it could use
InstanceKlass.getAllFieldsCount()

src/cpu/sparc/vm/assembler_sparc.cpp
  The following comment seems odd, should this be "The array of super
classes is compressed in permgen."?
    3185   // The array of super classes is compressed it is in permgen.

src/cpu/x86/vm/c1_CodeStubs_x86.cpp
  Is it worthwhile to try to keep the various architectures
implementations in sync? For example c1_CodeStubs_sparc.cpp lines
268-270 and 283-285 has address start = __ pc(); in an #ifdef ASSERT,
while the c1_CodeStubs_x86.cpp lines 287 and 300 always includes
address start = __ pc();

src/cpu/sparc/vm/frame_sparc.cpp
src/cpu/x86/vm/frame_x86.cpp
  There seem to be some slight differences in the changes to
frame::is_interpreted_frame_valid that I wonder if they were
intentional, or if these should really be more similar/identical,
maybe something along the lines of:
    constantPoolCacheOop cp = *interpreter_frame_cache_addr();
    if (cp == NULL ||
        !Space::is_aligned(cp) ||
        !cp->is_metadata()) return false;
    assert(!Universe::heap()->is_in_reserved(cp),
      "Should not be in heap");

src/cpu/sparc/vm/frame_sparc.cpp lines 669-673:
 669   constantPoolCacheOop cp = *interpreter_frame_cache_addr();
 670
 671   if (cp == NULL ||
 672       !cp->is_metadata()) return false;
 673

src/cpu/x86/vm/frame_x86.cpp lines 556-561:
 556   constantPoolCacheOop cp = *interpreter_frame_cache_addr();
 557
 558   if (cp == NULL ||
 559       !Space::is_aligned(cp)) return false;
 560   assert(!Universe::heap()->is_in_reserved(cp),
 561     "Should not be in heap");

src/share/vm/classfile/classFileParser.cpp
  On line 1225, will the old fields Array only be freed when the
classloader is unloaded? Why not free the old fields Array immediately
before reassigning the new_fields?
1222       // This loses the original array for "fields" but since it is
1223       // in the class loader arena, it will be freed.  Question is
1224       // if this is avoidable waste.


Thanks,
Dave