JDK-6849716 : BitMap: performance regression introduced with G1
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: hs14
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2009-06-10
  • Updated: 2013-09-18
  • Resolved: 2009-06-30
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
6u18Fixed 7Fixed hs16Fixed
Related Reports
Relates :  
JDK-6711316 - Open source the Garbage-First garbage collector
Relates :  
JDK-6814552 - par compact - some compilers fail to optimize bitmap code
Description
The G1 integration (6711316: Open source the Garbage-First garbage collector) introduced a large performance regression in BitMap operations which are used heavily by parallel compaction.  A synthetic benchmark shows a 50% increase in full gc time w/parallel compaction after 6711316.
Comments
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/6e2afda171db
14-06-2009
SUGGESTED FIX diff -r 353ba4575581 -r 6e2afda171db src/share/vm/utilities/macros.hpp --- a/src/share/vm/utilities/macros.hpp Sun Jun 07 22:08:24 2009 -0700 +++ b/src/share/vm/utilities/macros.hpp Thu Jun 11 13:31:01 2009 -0700 @@ -106,11 +106,13 @@ #ifdef ASSERT #define DEBUG_ONLY(code) code #define NOT_DEBUG(code) +#define NOT_DEBUG_RETURN /*next token must be ;*/ // Historical. #define debug_only(code) code #else // ASSERT #define DEBUG_ONLY(code) #define NOT_DEBUG(code) code +#define NOT_DEBUG_RETURN {} #define debug_only(code) #endif // ASSERT diff -r 353ba4575581 -r 6e2afda171db src/share/vm/utilities/bitMap.hpp --- a/src/share/vm/utilities/bitMap.hpp Sun Jun 07 22:08:24 2009 -0700 +++ b/src/share/vm/utilities/bitMap.hpp Thu Jun 11 13:31:01 2009 -0700 @@ -93,10 +93,12 @@ // The index of the first full word in a range. idx_t word_index_round_up(idx_t bit) const; - // Verification, statistics. - void verify_index(idx_t index) const; - void verify_range(idx_t beg_index, idx_t end_index) const; + // Verification. + inline void verify_index(idx_t index) const NOT_DEBUG_RETURN; + inline void verify_range(idx_t beg_index, idx_t end_index) const + NOT_DEBUG_RETURN; + // Statistics. static idx_t* _pop_count_table; static void init_pop_count_table(); static idx_t num_set_bits(bm_word_t w); @@ -287,7 +289,6 @@ #endif }; - // Convenience class wrapping BitMap which provides multiple bits per slot. class BitMap2D VALUE_OBJ_CLASS_SPEC { public: diff -r 353ba4575581 -r 6e2afda171db src/share/vm/utilities/bitMap.inline.hpp --- a/src/share/vm/utilities/bitMap.inline.hpp Sun Jun 07 22:08:24 2009 -0700 +++ b/src/share/vm/utilities/bitMap.inline.hpp Thu Jun 11 13:31:01 2009 -0700 @@ -22,6 +22,17 @@ * */ +#ifdef ASSERT +inline void BitMap::verify_index(idx_t index) const { + assert(index < _size, "BitMap index out of bounds"); +} + +inline void BitMap::verify_range(idx_t beg_index, idx_t end_index) const { + assert(beg_index <= end_index, "BitMap range error"); + // Note that [0,0) and [size,size) are both valid ranges. + if (end_index != _size) verify_index(end_index); +} +#endif // #ifdef ASSERT inline void BitMap::set_bit(idx_t bit) { verify_index(bit); diff -r 353ba4575581 -r 6e2afda171db src/share/vm/utilities/bitMap.cpp --- a/src/share/vm/utilities/bitMap.cpp Sun Jun 07 22:08:24 2009 -0700 +++ b/src/share/vm/utilities/bitMap.cpp Thu Jun 11 13:31:01 2009 -0700 @@ -41,19 +41,6 @@ resize(size_in_bits, in_resource_area); } - -void BitMap::verify_index(idx_t index) const { - assert(index < _size, "BitMap index out of bounds"); -} - -void BitMap::verify_range(idx_t beg_index, idx_t end_index) const { -#ifdef ASSERT - assert(beg_index <= end_index, "BitMap range error"); - // Note that [0,0) and [size,size) are both valid ranges. - if (end_index != _size) verify_index(end_index); -#endif -} - void BitMap::resize(idx_t size_in_bits, bool in_resource_area) { assert(size_in_bits >= 0, "just checking"); idx_t old_size_in_words = size_in_words(); diff -r 353ba4575581 -r 6e2afda171db src/share/vm/includeDB_compiler1 --- a/src/share/vm/includeDB_compiler1 Sun Jun 07 22:08:24 2009 -0700 +++ b/src/share/vm/includeDB_compiler1 Thu Jun 11 13:31:01 2009 -0700 @@ -387,7 +387,7 @@ c1_ValueSet.cpp c1_ValueSet.hpp c1_ValueSet.hpp allocation.hpp -c1_ValueSet.hpp bitMap.hpp +c1_ValueSet.hpp bitMap.inline.hpp c1_ValueSet.hpp c1_Instruction.hpp c1_ValueStack.cpp c1_IR.hpp
10-06-2009
EVALUATION The BitMap functions verify_index() and verify_range(), which contain only asserts, were changed from inline to out-of-line and the definitions moved to BitMap.cpp. This forced the c++ compilers to insert calls to the (empty) functions in product builds. Previously, the empty inline bodies were optimized away.
10-06-2009
SUGGESTED FIX Use a new NOT_DEBUG_RETURN macro, similar to the existing PRODUCT_RETURN macro, for the BitMap verify_index() and verify_range() methods. Enclose the definitions in #ifdef ASSERT ... #endif.
10-06-2009