From f2bc36d8ddfd7fd011162bdd796a6988dd82db9e Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Thu, 28 May 2026 07:55:45 +0000 Subject: [PATCH] 8383421: ZGC: Problematic interactions between `JVMTI_EVENT_SAMPLED_OBJECT_ALLOC` and `clone` Reviewed-by: stefank, jsikstro --- src/hotspot/share/gc/z/zBarrierSet.cpp | 94 ++++++++- src/hotspot/share/gc/z/zBarrierSet.hpp | 3 +- src/hotspot/share/gc/z/zBarrierSet.inline.hpp | 23 +-- src/hotspot/share/gc/z/zUtils.hpp | 1 + src/hotspot/share/gc/z/zUtils.inline.hpp | 7 + .../ZCloneWithTenuredAllocation.java | 192 ++++++++++++++++++ .../libZCloneWithTenuredAllocation.cpp | 148 ++++++++++++++ 7 files changed, 438 insertions(+), 30 deletions(-) create mode 100644 test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/ZCloneWithTenuredAllocation.java create mode 100644 test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/libZCloneWithTenuredAllocation.cpp diff --git a/src/hotspot/share/gc/z/zBarrierSet.cpp b/src/hotspot/share/gc/z/zBarrierSet.cpp index 5a7e4aa6511..f6f99672886 100644 --- a/src/hotspot/share/gc/z/zBarrierSet.cpp +++ b/src/hotspot/share/gc/z/zBarrierSet.cpp @@ -32,12 +32,14 @@ #include "gc/z/zHeap.inline.hpp" #include "gc/z/zStackWatermark.hpp" #include "gc/z/zThreadLocalData.hpp" +#include "gc/z/zUtils.inline.hpp" #include "runtime/atomicAccess.hpp" #include "runtime/deoptimization.hpp" #include "runtime/frame.inline.hpp" #include "runtime/javaThread.hpp" #include "runtime/registerMap.hpp" #include "runtime/stackWatermarkSet.hpp" +#include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" #ifdef COMPILER1 #include "gc/z/c1/zBarrierSetC1.hpp" @@ -125,18 +127,90 @@ zaddress ZBarrierSet::load_barrier_on_oop_field(volatile zpointer* p) { return ZBarrier::load_barrier_on_oop_field(p); } -void ZBarrierSet::clone_obj_array(objArrayOop src_obj, objArrayOop dst_obj) { - volatile zpointer* src = (volatile zpointer*)src_obj->base(); - volatile zpointer* dst = (volatile zpointer*)dst_obj->base(); - const int length = src_obj->length(); +class ZBarrierSet::ZClonerOopClosure : public BasicOopIterateClosure { + const zaddress _src; + const zaddress _dst; + const size_t _size; + const bool _is_dst_old; - for (const volatile zpointer* const end = src + length; src < end; src++, dst++) { - zaddress elem = ZBarrier::load_barrier_on_oop_field(src); - // We avoid healing here because the store below colors the pointer store good, - // hence avoiding the cost of a CAS. - ZBarrier::store_barrier_on_heap_oop_field(dst, false /* heal */); - AtomicAccess::store(dst, ZAddress::store_good(elem)); + size_t _copied_bytes; + + void copy_to(size_t byte_offset) { + assert(byte_offset != 0 && _copied_bytes <= byte_offset, + "Unexpected size and oop iteration order: %zu <= %zu", + _copied_bytes, byte_offset); + + if (_copied_bytes == byte_offset) { + // Already copied + return; + } + + // Copy up to byte_offset + const size_t copy_size = byte_offset - _copied_bytes; + ZUtils::object_copy_disjoint_atomic(_src, _dst, _copied_bytes, copy_size); + + // Account copied bytes + _copied_bytes = byte_offset; } + +public: + ZClonerOopClosure(zaddress src, zaddress dst, size_t size) + : _src(src), + _dst(dst), + _size(size), + _is_dst_old(ZHeap::heap()->page(dst)->is_old()), + _copied_bytes(0) {} + + ~ZClonerOopClosure() { + precond(!to_oop(_src)->is_typeArray() || _copied_bytes == 0); + + // Copy any potential tail + copy_to(_size); + + // Copy will have copied the header, clear it. + to_oop(_dst)->init_mark(); + + postcond(_copied_bytes == _size); + } + + virtual void do_oop(oop* p) { + volatile zpointer* const src_p = (volatile zpointer*)p; + const size_t offset = (uintptr_t)src_p - untype(_src); + volatile zpointer* const dst_p = (volatile zpointer*)(untype(_dst) + offset); + + // Copy payload up to element or field + copy_to(offset); + + // Load source object + const zaddress obj = ZBarrier::load_barrier_on_oop_field(src_p); + + // Store barrier + + // Store barrier over null (or uninitialized) requires only remembered-set handling + if (_is_dst_old) { + // "page is old" may be racy w.r.t. flip aging, but relocation handles + // missing remembered-set entries via ZRelocateAddRemsetForFlipPromoted. + ZGeneration::young()->remember(dst_p); + } + + // No concurrent writes are allowed to the dst object, except potential + // GC barrier healing. + AtomicAccess::store(dst_p, ZAddress::store_good(obj)); + + _copied_bytes += oopSize; + + postcond(_copied_bytes == offset + oopSize); + } + + virtual void do_oop(narrowOop* p) { + ShouldNotReachHere(); + } +}; + +void ZBarrierSet::clone_obj(zaddress src, zaddress dst, size_t size) { + // Clone the object + ZClonerOopClosure cl(src, dst, size); + ZIterator::oop_iterate(to_oop(src), &cl); } ZBarrierSet::ZBarrierSet() diff --git a/src/hotspot/share/gc/z/zBarrierSet.hpp b/src/hotspot/share/gc/z/zBarrierSet.hpp index 074d0e859fd..12efbdf1b98 100644 --- a/src/hotspot/share/gc/z/zBarrierSet.hpp +++ b/src/hotspot/share/gc/z/zBarrierSet.hpp @@ -48,7 +48,8 @@ private: static zaddress load_barrier_on_oop_field(volatile zpointer* p); - static void clone_obj_array(objArrayOop src, objArrayOop dst); + class ZClonerOopClosure; + static void clone_obj(zaddress src, zaddress dst, size_t size); public: ZBarrierSet(); diff --git a/src/hotspot/share/gc/z/zBarrierSet.inline.hpp b/src/hotspot/share/gc/z/zBarrierSet.inline.hpp index d99aa561764..d38068e92f7 100644 --- a/src/hotspot/share/gc/z/zBarrierSet.inline.hpp +++ b/src/hotspot/share/gc/z/zBarrierSet.inline.hpp @@ -30,6 +30,7 @@ #include "gc/z/zAddress.inline.hpp" #include "gc/z/zHeap.hpp" #include "gc/z/zNMethod.hpp" +#include "gc/z/zUtils.inline.hpp" #include "oops/objArrayOop.hpp" #include "utilities/debug.hpp" @@ -416,26 +417,10 @@ inline OopCopyResult ZBarrierSet::AccessBarrier::oop_ar template inline void ZBarrierSet::AccessBarrier::clone_in_heap(oop src, oop dst, size_t size) { check_is_valid_zaddress(src); + check_is_valid_zaddress(dst); + precond(src->klass() == dst->klass()); - if (dst->is_objArray()) { - // Cloning an object array is similar to performing array copy. - // If an array is large enough to have its allocation segmented, - // this operation might require GC barriers. However, the intrinsics - // for cloning arrays transform the clone to an optimized allocation - // and arraycopy sequence, so the performance of this runtime call - // does not matter for object arrays. - clone_obj_array(objArrayOop(src), objArrayOop(dst)); - return; - } - - // Fix the oops - ZBarrierSet::load_barrier_all(src, size); - - // Clone the object - Raw::clone_in_heap(src, dst, size); - - // Color store good before handing out - ZBarrierSet::color_store_good_all(dst, size); + clone_obj(to_zaddress(src), to_zaddress(dst), ZUtils::words_to_bytes(size)); } // diff --git a/src/hotspot/share/gc/z/zUtils.hpp b/src/hotspot/share/gc/z/zUtils.hpp index 731136c4c99..cadb1159433 100644 --- a/src/hotspot/share/gc/z/zUtils.hpp +++ b/src/hotspot/share/gc/z/zUtils.hpp @@ -44,6 +44,7 @@ public: static size_t object_size(zaddress addr); static void object_copy_disjoint(zaddress from, zaddress to, size_t size); static void object_copy_conjoint(zaddress from, zaddress to, size_t size); + static void object_copy_disjoint_atomic(zaddress from, zaddress to, size_t offset, size_t size); // Memory static void fill(uintptr_t* addr, size_t count, uintptr_t value); diff --git a/src/hotspot/share/gc/z/zUtils.inline.hpp b/src/hotspot/share/gc/z/zUtils.inline.hpp index a221a925498..658e278a8dd 100644 --- a/src/hotspot/share/gc/z/zUtils.inline.hpp +++ b/src/hotspot/share/gc/z/zUtils.inline.hpp @@ -72,6 +72,13 @@ inline void ZUtils::object_copy_conjoint(zaddress from, zaddress to, size_t size } } +inline void ZUtils::object_copy_disjoint_atomic(zaddress from, zaddress to, size_t offset, size_t size) { + const uintptr_t from_addr = untype(from) + offset; + const uintptr_t to_addr = untype(to) + offset; + + Copy::disjoint_words_atomic((HeapWord*)from_addr, (HeapWord*)to_addr, bytes_to_words(size)); +} + template inline void ZUtils::copy_disjoint(T* dest, const T* src, size_t count) { memcpy(dest, src, sizeof(T) * count); diff --git a/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/ZCloneWithTenuredAllocation.java b/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/ZCloneWithTenuredAllocation.java new file mode 100644 index 00000000000..bfee16bfda0 --- /dev/null +++ b/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/ZCloneWithTenuredAllocation.java @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2026, 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 id=Z + * @bug 8383421 + * @summary Exercise ZGC clone barriers on tenured new allocation. + * @requires vm.jvmti & vm.gc.Z + * @library /test/lib + * @modules java.base/jdk.internal.misc + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm/native -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -XX:+UseZGC + * -Xms128M + * -Xmx128M + * -Xint + * -agentlib:ZCloneWithTenuredAllocation + * ZCloneWithTenuredAllocation + */ + +/* + * @test id=ZVerify + * @bug 8383421 + * @summary Exercise ZGC clone barriers on tenured new allocation. + * @requires vm.jvmti & vm.gc.Z & vm.debug + * @library /test/lib + * @modules java.base/jdk.internal.misc + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm/native -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -XX:+UseZGC + * -XX:+ZVerifyRoots + * -XX:+ZVerifyObjects + * -XX:+ZVerifyMarking + * -XX:+ZVerifyForwarding + * -XX:+ZVerifyRemembered + * -XX:+ZVerifyOops + * -Xms128M + * -Xmx128M + * -Xint + * -agentlib:ZCloneWithTenuredAllocation + * ZCloneWithTenuredAllocation + */ + +import java.util.Arrays; + +import jdk.test.whitebox.WhiteBox; + +public class ZCloneWithTenuredAllocation { + private static final String AGENT_LIB = "ZCloneWithTenuredAllocation"; + + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + private static native void init(Class payloadClass, Class markerPairArrayClass); + private static native boolean isSampledObject(Object object); + + static { + System.loadLibrary(AGENT_LIB); + } + + public static void main(String[] args) { + init(Payload.class, MarkerPair[].class); + + testObjectClone(); + testObjectArrayClone(); + } + + private static void testObjectClone() { + final var clone = createPayload().clone(); + + if (!isSampledObject(clone)) { + throw new RuntimeException("Payload clone was not the sampled object"); + } + + // Run young collection to catch missing rememberset + WB.youngGC(); + + final var expected = createPayload(); + if (!expected.equals(clone)) { + throw new RuntimeException("Unexpected Payload clone: " + clone); + } + } + + private static void testObjectArrayClone() { + final var clone = createMarkerPairArray().clone(); + + if (!isSampledObject(clone)) { + throw new RuntimeException("MarkerPair[] clone was not the sampled object"); + } + + // Run young collection to catch missing rememberset + WB.youngGC(); + + final var expected = createMarkerPairArray(); + if (!Arrays.equals(expected, clone)) { + throw new RuntimeException("Unexpected MarkerPair[] clone: " + Arrays.toString(clone)); + } + } + + private static Payload createPayload() { + return new Payload(true, + (byte) 12, + '\u2345', + (short) 1234, + 0x12345678, + 0x1122334455667788L, + 12.25F, + 23.5D, + Boolean.TRUE, + Byte.valueOf((byte) 34), + Character.valueOf('\u2345'), + Short.valueOf((short) 1234), + Integer.valueOf(123456), + Long.valueOf(0x1122334455667788L), + Float.valueOf(12.25F), + Double.valueOf(23.5D), + new Marker(1), + new MarkerPair(new Marker(2), new Marker(3))); + } + + private static MarkerPair[] createMarkerPairArray() { + return new MarkerPair[] { + new MarkerPair(new Marker(101), new Marker(102)), + null, + new MarkerPair(new Marker(103), new Marker(104)), + new MarkerPair(new Marker(105), new Marker(106)) + }; + } + + private static void tenure(Object sampledObject) { + while (!WB.isObjectInOldGen(sampledObject)) { + WB.fullGC(); + } + } + + private record Payload(boolean booleanValue, + byte byteValue, + char charValue, + short shortValue, + int intValue, + long longValue, + float floatValue, + double doubleValue, + Boolean boxedBoolean, + Byte boxedByte, + Character boxedCharacter, + Short boxedShort, + Integer boxedInteger, + Long boxedLong, + Float boxedFloat, + Double boxedDouble, + Marker marker, + MarkerPair markerPair) implements Cloneable { + @Override + public Payload clone() { + try { + return (Payload) super.clone(); + } catch (CloneNotSupportedException e) { + throw new RuntimeException("Unexpected CloneNotSupportedException", e); + } + } + } + + private record Marker(int id) {} + + private record MarkerPair(Marker first, Marker second) {} +} diff --git a/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/libZCloneWithTenuredAllocation.cpp b/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/libZCloneWithTenuredAllocation.cpp new file mode 100644 index 00000000000..24a647d2858 --- /dev/null +++ b/test/hotspot/jtreg/serviceability/jvmti/events/SampledObjectAlloc/ZCloneWithTenuredAllocation/libZCloneWithTenuredAllocation.cpp @@ -0,0 +1,148 @@ +/* + * Copyright (c) 2026, 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. + */ + +#include "jvmti.h" +#include "jvmti_common.hpp" + +#include +#include + +extern "C" { + +static jvmtiEnv* jvmti = nullptr; + +static jclass test_class = nullptr; +static jclass payload_class = nullptr; +static jclass marker_pair_array_class = nullptr; +static jmethodID tenure_method = nullptr; + +static std::atomic in_callback(false); +static jobject sample = nullptr; + +static bool is_same_class(JNIEnv* jni, jclass first, jclass second) { + return jni->IsSameObject(first, second) == JNI_TRUE; +} + +static bool should_tenure(JNIEnv* jni, jclass object_klass) { + return is_same_class(jni, object_klass, payload_class) || + is_same_class(jni, object_klass, marker_pair_array_class); +} + +static void record_sampled_object(JNIEnv* jni, jobject object) { + if (sample != nullptr) { + jni->DeleteGlobalRef(sample); + } + + sample = jni->NewGlobalRef(object); + if (sample == nullptr) { + jni->FatalError("Could not create sampled object reference"); + } +} + +static void call_tenure(JNIEnv* jni, jobject object) { + jni->CallStaticVoidMethod(test_class, tenure_method, object); + if (jni->ExceptionCheck()) { + jni->ExceptionDescribe(); + jni->FatalError("ZCloneWithTenuredAllocation.tenure failed"); + } +} + +JNIEXPORT void JNICALL +SampledObjectAlloc(jvmtiEnv* jvmti_env, + JNIEnv* jni, + jthread thread, + jobject object, + jclass object_klass, + jlong size) { + if (!should_tenure(jni, object_klass)) { + return; + } + + if (in_callback.exchange(true)) { + return; + } + + record_sampled_object(jni, object); + call_tenure(jni, object); + + in_callback.store(false); +} + +JNIEXPORT void JNICALL +Java_ZCloneWithTenuredAllocation_init(JNIEnv* env, jclass cls, jclass payload_cls, jclass marker_pair_array_cls) { + if (test_class != nullptr || payload_class != nullptr || marker_pair_array_class != nullptr || tenure_method != nullptr) { + env->FatalError("ZCloneWithTenuredAllocation.init called more than once"); + } + + test_class = (jclass)env->NewGlobalRef(cls); + payload_class = (jclass)env->NewGlobalRef(payload_cls); + marker_pair_array_class = (jclass)env->NewGlobalRef(marker_pair_array_cls); + + tenure_method = env->GetStaticMethodID(cls, "tenure", "(Ljava/lang/Object;)V"); + if (tenure_method == nullptr) { + env->FatalError("Could not find ZCloneWithTenuredAllocation.tenure"); + } +} + +JNIEXPORT jboolean JNICALL +Java_ZCloneWithTenuredAllocation_isSampledObject(JNIEnv* env, jclass cls, jobject object) { + return env->IsSameObject(object, sample); +} + +static jint Agent_Initialize(JavaVM* jvm, char* options, void* reserved) { + jint res = jvm->GetEnv((void**)&jvmti, JVMTI_VERSION_9); + if (res != JNI_OK || jvmti == nullptr) { + return JNI_ERR; + } + + jvmtiCapabilities caps; + memset(&caps, 0, sizeof(caps)); + caps.can_generate_sampled_object_alloc_events = 1; + jvmtiError err = jvmti->AddCapabilities(&caps); + check_jvmti_error(err, "AddCapabilities"); + + jvmtiEventCallbacks callbacks; + memset(&callbacks, 0, sizeof(callbacks)); + callbacks.SampledObjectAlloc = &SampledObjectAlloc; + + err = jvmti->SetEventCallbacks(&callbacks, sizeof(callbacks)); + check_jvmti_error(err, "SetEventCallbacks"); + + err = jvmti->SetHeapSamplingInterval(0); + check_jvmti_error(err, "SetHeapSamplingInterval"); + + err = jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr); + check_jvmti_error(err, "SetEventNotificationMode"); + + return JNI_OK; +} + +JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) { + return Agent_Initialize(jvm, options, reserved); +} + +JNIEXPORT jint JNICALL Agent_OnAttach(JavaVM* jvm, char* options, void* reserved) { + return Agent_Initialize(jvm, options, reserved); +} + +}