United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: 6849716 BitMap: performance regression introduced with G1
6849716 : BitMap: performance regression introduced with G1

Details
Type:
Bug
Submit Date:
2009-06-10
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2009-06-30
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P2
Resolution:
Fixed
Affected Versions:
hs14
Fixed Versions:
hs16

Related Reports
Backport:
Backport:
Relates:
Relates:

Sub Tasks

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
                                     
2009-06-14
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.
                                     
2009-06-10
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.
                                     
2009-06-10
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
                                     
2009-06-10



Hardware and Software, Engineered to Work Together