8221540: ZGC: Reduce width of zForwardingEntry::from_index field
authorpliden
Thu, 28 Mar 2019 19:43:59 +0100
changeset 54330 69e80a82db9a
parent 54329 ddd60ad787d4
child 54331 f0fec71d2fff
8221540: ZGC: Reduce width of zForwardingEntry::from_index field Reviewed-by: stefank, eosterlund
src/hotspot/share/gc/z/zForwarding.cpp
src/hotspot/share/gc/z/zForwarding.inline.hpp
src/hotspot/share/gc/z/zForwardingEntry.hpp
src/hotspot/share/gc/z/zRelocate.cpp
test/hotspot/gtest/gc/z/test_zForwarding.cpp
--- a/src/hotspot/share/gc/z/zForwarding.cpp	Thu Mar 28 19:43:59 2019 +0100
+++ b/src/hotspot/share/gc/z/zForwarding.cpp	Thu Mar 28 19:43:59 2019 +0100
@@ -58,7 +58,7 @@
 
   for (ZForwardingCursor i = 0; i < _entries.length(); i++) {
     const ZForwardingEntry entry = at(&i);
-    if (entry.is_empty()) {
+    if (!entry.populated()) {
       // Skip empty entries
       continue;
     }
--- a/src/hotspot/share/gc/z/zForwarding.inline.hpp	Thu Mar 28 19:43:59 2019 +0100
+++ b/src/hotspot/share/gc/z/zForwarding.inline.hpp	Thu Mar 28 19:43:59 2019 +0100
@@ -119,9 +119,9 @@
 inline ZForwardingEntry ZForwarding::find(uintptr_t from_index, ZForwardingCursor* cursor) const {
   // Reading entries in the table races with the atomic CAS done for
   // insertion into the table. This is safe because each entry is at
-  // most updated once (from -1 to something else).
+  // most updated once (from zero to something else).
   ZForwardingEntry entry = first(from_index, cursor);
-  while (!entry.is_empty()) {
+  while (entry.populated()) {
     if (entry.from_index() == from_index) {
       // Match found, return matching entry
       return entry;
@@ -140,14 +140,14 @@
 
   for (;;) {
     const ZForwardingEntry prev_entry = Atomic::cmpxchg(new_entry, entries() + *cursor, old_entry);
-    if (prev_entry.is_empty()) {
+    if (!prev_entry.populated()) {
       // Success
       return to_offset;
     }
 
     // Find next empty or matching entry
     ZForwardingEntry entry = at(cursor);
-    while (!entry.is_empty()) {
+    while (entry.populated()) {
       if (entry.from_index() == from_index) {
         // Match found, return already inserted address
         return entry.to_offset();
--- a/src/hotspot/share/gc/z/zForwardingEntry.hpp	Thu Mar 28 19:43:59 2019 +0100
+++ b/src/hotspot/share/gc/z/zForwardingEntry.hpp	Thu Mar 28 19:43:59 2019 +0100
@@ -32,40 +32,40 @@
 // Forwarding entry layout
 // -----------------------
 //
-//   6                      4 4                                             0
-//   3                      2 1                                             0
-//  +------------------------+-----------------------------------------------+
-//  |11111111 11111111 111111|11 11111111 11111111 11111111 11111111 11111111|
-//  +------------------------+-----------------------------------------------+
-//  |                        |
-//  |                        * 41-0 To Object Offset (42-bits)
+//   6                  4 4
+//   3                  6 5                                                1 0
+//  +--------------------+--------------------------------------------------+-+
+//  |11111111 11111111 11|111111 11111111 11111111 11111111 11111111 1111111|1|
+//  +--------------------+--------------------------------------------------+-+
+//  |                    |                                                  |
+//  |                    |                      0-0 Populated Flag (1-bits) *
+//  |                    |
+//  |                    * 45-1 To Object Offset (45-bits)
 //  |
-//  * 63-42 From Object Index (22-bits)
+//  * 63-46 From Object Index (18-bits)
 //
 
 class ZForwardingEntry {
   friend struct PrimitiveConversions;
 
 private:
-  typedef ZBitField<uint64_t, size_t, 0,  42> field_to_offset;
-  typedef ZBitField<uint64_t, size_t, 42, 22> field_from_index;
+  typedef ZBitField<uint64_t, bool,   0,   1> field_populated;
+  typedef ZBitField<uint64_t, size_t, 1,  45> field_to_offset;
+  typedef ZBitField<uint64_t, size_t, 46, 18> field_from_index;
 
   uint64_t _entry;
 
-  static uintptr_t empty() {
-    return (uintptr_t)-1;
-  }
-
 public:
   ZForwardingEntry() :
-      _entry(empty()) {}
+      _entry(0) {}
 
   ZForwardingEntry(size_t from_index, size_t to_offset) :
-      _entry(field_from_index::encode(from_index) |
-             field_to_offset::encode(to_offset)) {}
+      _entry(field_populated::encode(true) |
+             field_to_offset::encode(to_offset) |
+             field_from_index::encode(from_index)) {}
 
-  bool is_empty() const {
-    return _entry == empty();
+  bool populated() const {
+    return field_populated::decode(_entry);
   }
 
   size_t to_offset() const {
--- a/src/hotspot/share/gc/z/zRelocate.cpp	Thu Mar 28 19:43:59 2019 +0100
+++ b/src/hotspot/share/gc/z/zRelocate.cpp	Thu Mar 28 19:43:59 2019 +0100
@@ -88,7 +88,7 @@
 
   // Lookup forwarding entry
   const ZForwardingEntry entry = forwarding->find(from_index, &cursor);
-  if (entry.from_index() == from_index) {
+  if (entry.populated() && entry.from_index() == from_index) {
     // Already relocated, return new address
     return entry.to_offset();
   }
@@ -150,7 +150,9 @@
   const uintptr_t from_index = (from_offset - forwarding->start()) >> forwarding->object_alignment_shift();
   const ZForwardingEntry entry = forwarding->find(from_index);
 
+  assert(entry.populated(), "Should be forwarded");
   assert(entry.from_index() == from_index, "Should be forwarded");
+
   return ZAddress::good(entry.to_offset());
 }
 
--- a/test/hotspot/gtest/gc/z/test_zForwarding.cpp	Thu Mar 28 19:43:59 2019 +0100
+++ b/test/hotspot/gtest/gc/z/test_zForwarding.cpp	Thu Mar 28 19:43:59 2019 +0100
@@ -70,10 +70,8 @@
     for (uint32_t i = 0; i < entries_to_check; i++) {
       uintptr_t from_index = SequenceToFromIndex::one_to_one(i);
 
-      EXPECT_TRUE(forwarding->find(from_index).is_empty()) << CAPTURE2(from_index, size);
+      EXPECT_FALSE(forwarding->find(from_index).populated()) << CAPTURE2(from_index, size);
     }
-
-    EXPECT_TRUE(forwarding->find(uintptr_t(-1)).is_empty()) << CAPTURE(size);
   }
 
   static void find_full(ZForwarding* forwarding) {
@@ -86,7 +84,7 @@
 
       ZForwardingCursor cursor;
       ZForwardingEntry entry = forwarding->find(from_index, &cursor);
-      ASSERT_TRUE(entry.is_empty()) << CAPTURE2(from_index, size);
+      ASSERT_FALSE(entry.populated()) << CAPTURE2(from_index, size);
 
       forwarding->insert(from_index, from_index, &cursor);
     }
@@ -96,7 +94,7 @@
       uintptr_t from_index = SequenceToFromIndex::one_to_one(i);
 
       ZForwardingEntry entry = forwarding->find(from_index);
-      ASSERT_FALSE(entry.is_empty()) << CAPTURE2(from_index, size);
+      ASSERT_TRUE(entry.populated()) << CAPTURE2(from_index, size);
 
       ASSERT_EQ(entry.from_index(), from_index) << CAPTURE(size);
       ASSERT_EQ(entry.to_offset(), from_index) << CAPTURE(size);
@@ -113,7 +111,7 @@
 
       ZForwardingCursor cursor;
       ZForwardingEntry entry = forwarding->find(from_index, &cursor);
-      ASSERT_TRUE(entry.is_empty()) << CAPTURE2(from_index, size);
+      ASSERT_FALSE(entry.populated()) << CAPTURE2(from_index, size);
 
       forwarding->insert(from_index, from_index, &cursor);
     }
@@ -124,7 +122,7 @@
 
       ZForwardingCursor cursor;
       ZForwardingEntry entry = forwarding->find(from_index, &cursor);
-      ASSERT_FALSE(entry.is_empty()) << CAPTURE2(from_index, size);
+      ASSERT_TRUE(entry.populated()) << CAPTURE2(from_index, size);
 
       ASSERT_EQ(entry.from_index(), from_index) << CAPTURE(size);
       ASSERT_EQ(entry.to_offset(), from_index) << CAPTURE(size);
@@ -139,7 +137,7 @@
 
       ZForwardingEntry entry = forwarding->find(from_index);
 
-      ASSERT_TRUE(entry.is_empty()) << CAPTURE2(from_index, size);
+      ASSERT_FALSE(entry.populated()) << CAPTURE2(from_index, size);
     }
   }