8355560: SetTag/GetTag should consider equal value objects as the same object

Author: Alex Menkov

src/hotspot/share/prims/jvmtiTagMapTable.cpp

@@ -24,6 +24,7 @@
 
 #include "memory/allocation.hpp"
 #include "memory/universe.hpp"
+#include "oops/fieldStreams.inline.hpp"
 #include "oops/oop.inline.hpp"
 #include "oops/weakHandle.inline.hpp"
 #include "prims/jvmtiExport.hpp"
@@ -32,35 +33,48 @@
 
 JvmtiTagMapKey::JvmtiTagMapKey(oop obj) : _obj(obj) {}
 
-JvmtiTagMapKey::JvmtiTagMapKey(const JvmtiTagMapKey& src) {
-  // move object into WeakHandle when copying into the table
+JvmtiTagMapKey::JvmtiTagMapKey(const JvmtiTagMapKey& src) : _h() {
+  // move object into Handle when copying into the table
   if (src._obj != nullptr) {
+    _is_weak = !src._obj->klass()->is_inline_klass();
 
     // obj was read with AS_NO_KEEPALIVE, or equivalent, like during
     // a heap walk.  The object needs to be kept alive when it is published.
     Universe::heap()->keep_alive(src._obj);
 
-    _wh = WeakHandle(JvmtiExport::weak_tag_storage(), src._obj);
+    if (_is_weak) {
+      _wh = WeakHandle(JvmtiExport::weak_tag_storage(), src._obj);
+    } else {
+      _h = OopHandle(JvmtiExport::jvmti_oop_storage(), src._obj);
+    }
   } else {
     // resizing needs to create a copy.
-    _wh = src._wh;
+    if (_is_weak) {
+      _wh = src._wh;
+    } else {
+      _h = src._h;
+    }
   }
   // obj is always null after a copy.
   _obj = nullptr;
 }
 
-void JvmtiTagMapKey::release_weak_handle() {
-  _wh.release(JvmtiExport::weak_tag_storage());
+void JvmtiTagMapKey::release_handle() {
+  if (_is_weak) {
+    _wh.release(JvmtiExport::weak_tag_storage());
+  } else {
+    _h.release(JvmtiExport::jvmti_oop_storage());
+  }
 }
 
 oop JvmtiTagMapKey::object() const {
   assert(_obj == nullptr, "Must have a handle and not object");
-  return _wh.resolve();
+  return _is_weak ? _wh.resolve() : _h.resolve();
 }
 
 oop JvmtiTagMapKey::object_no_keepalive() const {
   assert(_obj == nullptr, "Must have a handle and not object");
-  return _wh.peek();
+  return _is_weak ? _wh.peek() : _h.peek();
 }
 
 unsigned JvmtiTagMapKey::get_hash(const JvmtiTagMapKey& entry) {
@@ -75,6 +89,77 @@ unsigned JvmtiTagMapKey::get_hash(const JvmtiTagMapKey& entry) {
   }
 }
 
+static bool equal_oops(oop obj1, oop obj2); // forward declaration
+
+static bool equal_fields(char type, oop obj1, oop obj2, int offset) {
+  switch (type) {
+  case JVM_SIGNATURE_BOOLEAN:
+    return obj1->bool_field(offset) == obj2->bool_field(offset);
+  case JVM_SIGNATURE_CHAR:
+    return obj1->char_field(offset) == obj2->char_field(offset);
+  case JVM_SIGNATURE_FLOAT:
+    return obj1->float_field(offset) == obj2->float_field(offset);
+  case JVM_SIGNATURE_DOUBLE:
+    return obj1->double_field(offset) == obj2->double_field(offset);
+  case JVM_SIGNATURE_BYTE:
+    return obj1->byte_field(offset) == obj2->byte_field(offset);
+  case JVM_SIGNATURE_SHORT:
+    return obj1->short_field(offset) == obj2->short_field(offset);
+  case JVM_SIGNATURE_INT:
+    return obj1->int_field(offset) == obj2->int_field(offset);
+  case JVM_SIGNATURE_LONG:
+    return obj1->long_field(offset) == obj2->long_field(offset);
+  case JVM_SIGNATURE_CLASS:
+  case JVM_SIGNATURE_ARRAY:
+    return equal_oops(obj1->obj_field(offset), obj2->obj_field(offset));
+  }
+  ShouldNotReachHere();
+}
+
+// For heap-allocated objects offset is 0 and 'klass' is obj1->klass() (== obj2->klass()).
+// For flattened objects offset is the offset in the holder object, 'klass' is inlined object class.
+static bool equal_value_objects(oop obj1, oop obj2, InlineKlass* klass, int offset) {
+  for (JavaFieldStream fld(klass); !fld.done(); fld.next()) {
+    // ignore static fields
+    if (fld.access_flags().is_static()) {
+      continue;
+    }
+    int field_offset = offset + fld.offset() - (offset > 0 ? klass->payload_offset() : 0);
+    if (fld.is_flat()) { // flat value field
+      InstanceKlass* holder_klass = fld.field_holder();
+      InlineKlass* field_klass = holder_klass->get_inline_type_field_klass(fld.index());
+      if (!equal_value_objects(obj1, obj2, field_klass, field_offset)) {
+        return false;
+      }
+    } else {
+      if (!equal_fields(fld.signature()->char_at(0), obj1, obj2, field_offset)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+static bool equal_oops(oop obj1, oop obj2) {
+  if (obj1 == obj2) {
+    return true;
+  }
+
+  if (EnableValhalla) {
+    if (obj1 != nullptr && obj2 != nullptr && obj1->klass() == obj2->klass() && obj1->is_inline_type()) {
+      InlineKlass* vk = InlineKlass::cast(obj1->klass());
+      return equal_value_objects(obj1, obj2, vk, 0);
+    }
+  }
+  return false;
+}
+
+bool JvmtiTagMapKey::equals(const JvmtiTagMapKey& lhs, const JvmtiTagMapKey& rhs) {
+  oop lhs_obj = lhs._obj != nullptr ? lhs._obj : lhs.object_no_keepalive();
+  oop rhs_obj = rhs._obj != nullptr ? rhs._obj : rhs.object_no_keepalive();
+  return equal_oops(lhs_obj, rhs_obj);
+}
+
 // Inline types don't use hash for this table.
 static inline bool fast_no_hash_check(oop obj) {
   return (obj->fast_no_hash_check() && !obj->is_inline_type());
@@ -88,7 +173,7 @@ JvmtiTagMapTable::JvmtiTagMapTable() : _table(INITIAL_TABLE_SIZE, MAX_TABLE_SIZE
 void JvmtiTagMapTable::clear() {
   struct RemoveAll {
     bool do_entry(JvmtiTagMapKey& entry, const jlong& tag) {
-      entry.release_weak_handle();
+      entry.release_handle();
       return true;
     }
   } remove_all;
@@ -142,7 +227,7 @@ void JvmtiTagMapTable::add(oop obj, jlong tag) {
 void JvmtiTagMapTable::remove(oop obj) {
   JvmtiTagMapKey jtme(obj);
   auto clean = [] (JvmtiTagMapKey& entry, jlong tag) {
-    entry.release_weak_handle();
+    entry.release_handle();
   };
   _table.remove(jtme, clean);
 }
@@ -160,7 +245,7 @@ void JvmtiTagMapTable::remove_dead_entries(GrowableArray<jlong>* objects) {
         if (_objects != nullptr) {
           _objects->append(tag);
         }
-        entry.release_weak_handle();
+        entry.release_handle();
         return true;
       }
       return false;;

src/hotspot/share/prims/jvmtiTagMapTable.hpp

@@ -40,8 +40,17 @@ class JvmtiTagMapKeyClosure;
 // This class is the Key type for inserting in ResizeableResourceHashTable
 // Its get_hash() and equals() methods are also used for getting the hash
 // value of a Key and comparing two Keys, respectively.
+//
+// Valhalla: Keep just one tag for all equal value objects including heap allocated value objects.
+// We have to keep a strong reference to each unique value object with a non-zero tag.
 class JvmtiTagMapKey : public CHeapObj<mtServiceability> {
-  WeakHandle _wh;
+  // All equal value objects should have the same tag.
+  // Keep value objects alive (1 copy for each "value") until their tags are removed.
+  union {
+    WeakHandle _wh;
+    OopHandle _h; // for value objects (_is_weak == false)
+  };
+  bool _is_weak;
   oop _obj; // temporarily hold obj while searching
  public:
   JvmtiTagMapKey(oop obj);
@@ -50,14 +59,10 @@ class JvmtiTagMapKey : public CHeapObj<mtServiceability> {
 
   oop object() const;
   oop object_no_keepalive() const;
-  void release_weak_handle();
+  void release_handle();
 
   static unsigned get_hash(const JvmtiTagMapKey& entry);
-  static bool equals(const JvmtiTagMapKey& lhs, const JvmtiTagMapKey& rhs) {
-    oop lhs_obj = lhs._obj != nullptr ? lhs._obj : lhs.object_no_keepalive();
-    oop rhs_obj = rhs._obj != nullptr ? rhs._obj : rhs.object_no_keepalive();
-    return lhs_obj == rhs_obj;
-  }
+  static bool equals(const JvmtiTagMapKey& lhs, const JvmtiTagMapKey& rhs);
 };
 
 typedef

test/hotspot/jtreg/serviceability/jvmti/valhalla/SetTag/ValueTagMapTest.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Tests SetTag/GetTag functionality for value objects.
+ * @requires vm.jvmti
+ * @modules java.base/jdk.internal.vm.annotation
+ * @enablePreview
+ * @run main/othervm/native -agentlib:ValueTagMapTest
+ *                          -XX:+PrintInlineLayout
+ *                          -XX:+PrintFlatArrayLayout
+ *                          -Xlog:jvmti+table
+ *                          ValueTagMapTest
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import jdk.internal.vm.annotation.NullRestricted;
+import jdk.internal.vm.annotation.Strict;
+
+public class ValueTagMapTest {
+
+    private static value class ValueClass {
+        public int f;
+        public ValueClass(int v) { f = v + 1; }
+        public String toString() {
+            return String.valueOf(f);
+        }
+    }
+
+    private static value class ValueHolder {
+        @Strict
+        @NullRestricted
+        public ValueClass f0;
+
+        public ValueHolder(int v) {
+            f0 = new ValueClass(v);
+        }
+        public String toString() {
+            return "holder{" + f0 + "}";
+        }
+    }
+
+    private static value class ValueHolder2 {
+        public ValueHolder f1;
+        @Strict
+        @NullRestricted
+        public ValueHolder f2;
+
+        public ValueHolder2(int v, int v2) {
+            f1 = new ValueHolder(v);
+            f2 = new ValueHolder(v2);
+        }
+        public ValueHolder2(ValueHolder h1, int v2) {
+            f1 = h1;
+            f2 = new ValueHolder(v2);
+        }
+        public String toString() {
+            return "holder2{" + f1 + ", " + f2 + "}";
+        }
+    }
+
+
+    private static native void setTag0(Object object, long tag);
+    private static void setTag(Object object, long tag) {
+        setTag0(object, tag);
+    }
+    private static native long getTag0(Object object);
+    private static long getTag(Object object) {
+        long tag = getTag0(object);
+        if (tag == 0) {
+            throw new RuntimeException("Zero tag for object " + object);
+        }
+        return tag;
+    }
+
+    private static void testGetTag(Object o1, Object o2) {
+        long tag1 = getTag(o1);
+        long tag2 = getTag(o2);
+        if (o1 == o2) {
+            if (tag1 != tag2) {
+                throw new RuntimeException("different tags for equal objects: "
+                                           + o1 + " (tag " + tag1 + "), "
+                                           + o2 + " (tag " + tag2 + ")");
+            }
+        } else {
+            if (tag1 == tag2) {
+                throw new RuntimeException("equal tags for different objects: "
+                                           + o1 + " (tag " + tag1 + "), "
+                                           + o2 + " (tag " + tag2 + ")");
+            }
+        }
+    }
+
+    public static void main(String[] args) {
+        System.loadLibrary("ValueTagMapTest");
+        List<ValueHolder2> items = new ArrayList<>();
+
+        for (int i = 0; i < 20; i++) {
+            items.add(new ValueHolder2(i % 4, i % 8));
+        }
+
+        long startTime = System.nanoTime();
+        long tag = 1;
+        for (ValueHolder2 item : items) {
+            setTag(item, tag++);
+            setTag(item.f1, tag++);
+            setTag(item.f2, tag++);
+        }
+
+        for (ValueHolder2 item: items) {
+            long tag0 = getTag(item);
+            long tag1 = getTag(item.f1);
+            long tag2 = getTag(item.f2);
+            System.out.println("getTag (" + item + "): " + tag0 + ", f1: " + tag1 + ", f2:" + tag2);
+        }
+
+        startTime = System.nanoTime();
+        for (ValueHolder2 item1: items) {
+            for (ValueHolder2 item2 : items) {
+                testGetTag(item1, item2);
+                testGetTag(item1.f1, item2.f1);
+                testGetTag(item1.f2, item2.f2);
+                testGetTag(item1.f1, item2.f2);
+                testGetTag(item1.f2, item2.f1);
+            }
+        }
+    }
+}