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

hotspot/src/share/vm/memory/compactingPermGenGen.cpp

@@ -352,15 +352,19 @@ void CompactingPermGenGen::post_compact() {
 }
 
 
+// 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());
   }

hotspot/src/share/vm/memory/compactingPermGenGen.hpp

@@ -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 @@ private:
   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.

hotspot/src/share/vm/memory/generation.cpp

@@ -606,6 +606,13 @@ void OneContigSpaceCardGeneration::shrink_by(size_t bytes) {
 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);
 }