JDK-6765804 : GC "dead ratios" should be unsigned
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 1.3.0
  • Priority: P5
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2008-10-30
  • Updated: 2010-04-02
  • Resolved: 2008-11-25
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7 Other
6u14Fixed 7Fixed hs14Fixed
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
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.") \ \
12-11-2008

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

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