# HG changeset patch
# User ysr
# Date 1257295396 28800
# Node ID 4385d4e7497b712465de1fde2597504ec2c1666f
# Parent  c5388a988d5a5e4c7ddcdece5626ebc2beed47db
6801625: CDS: HeapDump tests crash with internal error in compactingPermGenGen.cpp
Summary: Allow iteration over the shared spaces when using CDS, repealing previous proscription. Deferred further required CDS-related cleanups of perm gen to CR 6897789.
Reviewed-by: phh, jmasa

diff -r c5388a988d5a -r 4385d4e7497b src/share/vm/memory/compactingPermGenGen.cpp
--- a/src/share/vm/memory/compactingPermGenGen.cpp	Wed Nov 04 07:07:14 2009 -0800
+++ b/src/share/vm/memory/compactingPermGenGen.cpp	Tue Nov 03 16:43:16 2009 -0800
@@ -352,15 +352,19 @@
 }
 
 
+// Do not use in time-critical operations due to the possibility of paging
+// in otherwise untouched or previously unread portions of the perm gen,
+// for instance, the shared spaces. NOTE: Because CompactingPermGenGen
+// derives from OneContigSpaceCardGeneration which is supposed to have a
+// single space, and does not override its object_iterate() method,
+// object iteration via that interface does not look at the objects in
+// the shared spaces when using CDS. This should be fixed; see CR 6897798.
 void CompactingPermGenGen::space_iterate(SpaceClosure* blk, bool usedOnly) {
   OneContigSpaceCardGeneration::space_iterate(blk, usedOnly);
   if (spec()->enable_shared_spaces()) {
-#ifdef PRODUCT
     // Making the rw_space walkable will page in the entire space, and
-    // is to be avoided. However, this is required for Verify options.
-    ShouldNotReachHere();
-#endif
-
+    // is to be avoided in the case of time-critical operations.
+    // However, this is required for Verify and heap dump operations.
     blk->do_space(ro_space());
     blk->do_space(rw_space());
   }
diff -r c5388a988d5a -r 4385d4e7497b src/share/vm/memory/compactingPermGenGen.hpp
--- a/src/share/vm/memory/compactingPermGenGen.hpp	Wed Nov 04 07:07:14 2009 -0800
+++ b/src/share/vm/memory/compactingPermGenGen.hpp	Tue Nov 03 16:43:16 2009 -0800
@@ -29,6 +29,9 @@
 class PermanentGenerationSpec;
 
 // This is the "generation" view of a CompactingPermGen.
+// NOTE: the shared spaces used for CDS are here handled in
+// a somewhat awkward and potentially buggy fashion, see CR 6801625.
+// This infelicity should be fixed, see CR 6897789.
 class CompactingPermGenGen: public OneContigSpaceCardGeneration {
   friend class VMStructs;
   // Abstractly, this is a subtype that gets access to protected fields.
@@ -47,7 +50,7 @@
   OffsetTableContigSpace* _ro_space;
   OffsetTableContigSpace* _rw_space;
 
-  // With shared spaces there is a dicotomy in the use of the
+  // With shared spaces there is a dichotomy in the use of the
   // _virtual_space of the generation.  There is a portion of the
   // _virtual_space that is used for the unshared part of the
   // permanent generation and a portion that is reserved for the shared part.
diff -r c5388a988d5a -r 4385d4e7497b src/share/vm/memory/generation.cpp
--- a/src/share/vm/memory/generation.cpp	Wed Nov 04 07:07:14 2009 -0800
+++ b/src/share/vm/memory/generation.cpp	Tue Nov 03 16:43:16 2009 -0800
@@ -606,6 +606,13 @@
 void OneContigSpaceCardGeneration::prepare_for_verify() {}
 
 
+// Override for a card-table generation with one contiguous
+// space. NOTE: For reasons that are lost in the fog of history,
+// this code is used when you iterate over perm gen objects,
+// even when one uses CDS, where the perm gen has a couple of
+// other spaces; this is because CompactingPermGenGen derives
+// from OneContigSpaceCardGeneration. This should be cleaned up,
+// see CR 6897789..
 void OneContigSpaceCardGeneration::object_iterate(ObjectClosure* blk) {
   _the_space->object_iterate(blk);
 }