United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6765804 GC "dead ratios" should be unsigned
JDK-6765804 : GC "dead ratios" should be unsigned

Details
Type:
Bug
Submit Date:
2008-10-30
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2008-11-25
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P5
Resolution:
Fixed
Affected Versions:
1.3.0
Fixed Versions:
hs14 (b08)

Related Reports
Backport:
Backport:

Sub Tasks

Description
The variables MarkSweepDeadRatio and PermMarkSweepDeadRatio represent a percentage of the generation that can be left unreclaimed during a full gc, so valid values are 0-100.  Yet they are signed types, simply because of historical (bad) practice.

                                    

Comments
EVALUATION

Change the declarations in globals.hpp to use unsigned types and update the code that uses these values.
                                     
2008-10-30
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/122d10c82f3f
                                     
2008-11-12
SUGGESTED FIX

# HG changeset patch
# User jcoomes
# Date 1225287002 25200
# Node ID 122d10c82f3fe08aa323dd01703793c0869330ad
# Parent  348be627a148a69f62f2033da6bf60c66367891f
6765804: GC "dead ratios" should be unsigned
Reviewed-by: ysr, tonyp

diff --git a/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp b/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp
--- a/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp
+++ b/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp
@@ -90,10 +90,10 @@
    */
   bool skip_dead = ((PSMarkSweep::total_invocations() % MarkSweepAlwaysCompactCount) != 0);
 
-  ssize_t allowed_deadspace = 0;
+  size_t allowed_deadspace = 0;
   if (skip_dead) {
-    int ratio = allowed_dead_ratio();
-    allowed_deadspace = (space()->capacity_in_bytes() * ratio / 100) / HeapWordSize;
+    const size_t ratio = allowed_dead_ratio();
+    allowed_deadspace = space()->capacity_in_words() * ratio / 100;
   }
 
   // Fetch the current destination decorator
@@ -271,10 +271,10 @@
   dest->set_compaction_top(compact_top);
 }
 
-bool PSMarkSweepDecorator::insert_deadspace(ssize_t& allowed_deadspace_words,
-                                       HeapWord* q, size_t deadlength) {
-  allowed_deadspace_words -= deadlength;
-  if (allowed_deadspace_words >= 0) {
+bool PSMarkSweepDecorator::insert_deadspace(size_t& allowed_deadspace_words,
+                                            HeapWord* q, size_t deadlength) {
+  if (allowed_deadspace_words >= deadlength) {
+    allowed_deadspace_words -= deadlength;
     oop(q)->set_mark(markOopDesc::prototype()->set_marked());
     const size_t aligned_min_int_array_size =
       align_object_size(typeArrayOopDesc::header_size(T_INT));
diff --git a/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.hpp b/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.hpp
--- a/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.hpp
+++ b/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.hpp
@@ -39,14 +39,16 @@
   HeapWord* _first_dead;
   HeapWord* _end_of_live;
   HeapWord* _compaction_top;
-  unsigned int _allowed_dead_ratio;
+  size_t _allowed_dead_ratio;
 
-  bool insert_deadspace(ssize_t& allowed_deadspace_words, HeapWord* q, size_t word_len);
+  bool insert_deadspace(size_t& allowed_deadspace_words, HeapWord* q,
+                        size_t word_len);
 
  public:
   PSMarkSweepDecorator(MutableSpace* space, ObjectStartArray* start_array,
-                       unsigned int allowed_dead_ratio) :
-    _space(space), _start_array(start_array), _allowed_dead_ratio(allowed_dead_ratio) { }
+                       size_t allowed_dead_ratio) :
+    _space(space), _start_array(start_array),
+    _allowed_dead_ratio(allowed_dead_ratio) { }
 
   // During a compacting collection, we need to collapse objects into
   // spaces in a given order. We want to fill space A, space B, and so
@@ -57,14 +59,14 @@
   static PSMarkSweepDecorator* destination_decorator();
 
   // Accessors
-  MutableSpace* space()                           { return _space; }
-  ObjectStartArray* start_array()                 { return _start_array; }
+  MutableSpace* space()                     { return _space; }
+  ObjectStartArray* start_array()           { return _start_array; }
 
-  HeapWord* compaction_top()                      { return _compaction_top; }
-  void set_compaction_top(HeapWord* value)        { _compaction_top = value; }
+  HeapWord* compaction_top()                { return _compaction_top; }
+  void set_compaction_top(HeapWord* value)  { _compaction_top = value; }
 
-  unsigned int allowed_dead_ratio()               { return _allowed_dead_ratio; }
-  void set_allowed_dead_ratio(unsigned int value) { _allowed_dead_ratio = value; }
+  size_t allowed_dead_ratio()               { return _allowed_dead_ratio; }
+  void set_allowed_dead_ratio(size_t value) { _allowed_dead_ratio = value; }
 
   // Work methods
   void adjust_pointers();
diff --git a/src/share/vm/memory/space.cpp b/src/share/vm/memory/space.cpp
--- a/src/share/vm/memory/space.cpp
+++ b/src/share/vm/memory/space.cpp
@@ -997,11 +997,11 @@
 }
 
 
-int TenuredSpace::allowed_dead_ratio() const {
+size_t TenuredSpace::allowed_dead_ratio() const {
   return MarkSweepDeadRatio;
 }
 
 
-int ContigPermSpace::allowed_dead_ratio() const {
+size_t ContigPermSpace::allowed_dead_ratio() const {
   return PermMarkSweepDeadRatio;
 }
diff --git a/src/share/vm/memory/space.hpp b/src/share/vm/memory/space.hpp
--- a/src/share/vm/memory/space.hpp
+++ b/src/share/vm/memory/space.hpp
@@ -421,7 +421,7 @@
 
   // The maximum percentage of objects that can be dead in the compacted
   // live part of a compacted space ("deadwood" support.)
-  virtual int allowed_dead_ratio() const { return 0; };
+  virtual size_t allowed_dead_ratio() const { return 0; };
 
   // Some contiguous spaces may maintain some data structures that should
   // be updated whenever an allocation crosses a boundary.  This function
@@ -507,7 +507,7 @@
                                                                              \
   size_t allowed_deadspace = 0;                                              \
   if (skip_dead) {                                                           \
-    int ratio = allowed_dead_ratio();                                        \
+    const size_t ratio = allowed_dead_ratio();                               \
     allowed_deadspace = (capacity() * ratio / 100) / HeapWordSize;           \
   }                                                                          \
                                                                              \
@@ -1079,7 +1079,7 @@
   friend class VMStructs;
  protected:
   // Mark sweep support
-  int allowed_dead_ratio() const;
+  size_t allowed_dead_ratio() const;
  public:
   // Constructor
   TenuredSpace(BlockOffsetSharedArray* sharedOffsetArray,
@@ -1094,7 +1094,7 @@
   friend class VMStructs;
  protected:
   // Mark sweep support
-  int allowed_dead_ratio() const;
+  size_t allowed_dead_ratio() const;
  public:
   // Constructor
   ContigPermSpace(BlockOffsetSharedArray* sharedOffsetArray, MemRegion mr) :
diff --git a/src/share/vm/memory/tenuredGeneration.hpp b/src/share/vm/memory/tenuredGeneration.hpp
--- a/src/share/vm/memory/tenuredGeneration.hpp
+++ b/src/share/vm/memory/tenuredGeneration.hpp
@@ -73,7 +73,6 @@
 
   // Mark sweep support
   void compute_new_size();
-  int allowed_dead_ratio() const;
 
   virtual void gc_prologue(bool full);
   virtual void gc_epilogue(bool full);
diff --git a/src/share/vm/runtime/globals.hpp b/src/share/vm/runtime/globals.hpp
--- a/src/share/vm/runtime/globals.hpp
+++ b/src/share/vm/runtime/globals.hpp
@@ -2792,7 +2792,7 @@
   product(intx, TargetSurvivorRatio,    50,                                 \
           "Desired percentage of survivor space used after scavenge")       \
                                                                             \
-  product(intx, MarkSweepDeadRatio,     5,                                  \
+  product(uintx, MarkSweepDeadRatio,     5,                                 \
           "Percentage (0-100) of the old gen allowed as dead wood."         \
           "Serial mark sweep treats this as both the min and max value."    \
           "CMS uses this value only if it falls back to mark sweep."        \
@@ -2801,7 +2801,7 @@
           "either completely full or completely empty.  Par compact also"   \
           "has a smaller default value; see arguments.cpp.")                \
                                                                             \
-  product(intx, PermMarkSweepDeadRatio,    20,                              \
+  product(uintx, PermMarkSweepDeadRatio,    20,                             \
           "Percentage (0-100) of the perm gen allowed as dead wood."        \
           "See MarkSweepDeadRatio for collector-specific comments.")        \
                                                                             \
                                     
2008-11-12



Hardware and Software, Engineered to Work Together