From 0ff7aec4e1e071102273bf4484bfad5f9c2d6e37 Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Tue, 7 May 2024 16:30:29 +0100 Subject: [PATCH 1/4] Initial pusg --- .../jdk/internal/foreign/LayoutPath.java | 7 +++- test/jdk/java/foreign/TestAccessModes.java | 38 +++++++++++-------- .../lang/foreign/LoopOverNonConstant.java | 29 +++++++++++++- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java index 5eee17573eed9..a64b3809afcbb 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java +++ b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java @@ -26,6 +26,8 @@ package jdk.internal.foreign; import jdk.internal.vm.annotation.ForceInline; +import sun.security.action.GetPropertyAction; + import java.lang.foreign.AddressLayout; import java.lang.foreign.GroupLayout; import java.lang.foreign.MemoryLayout; @@ -67,6 +69,9 @@ public class LayoutPath { private static final MethodHandle MH_SEGMENT_RESIZE; private static final MethodHandle MH_ADD; + private static final boolean USE_FULL_CHECKS = Boolean.parseBoolean( + GetPropertyAction.privilegedGetProperty("jdk.internal.foreign.handle.USE_FULL_CHECKS", "false")); + static { try { MethodHandles.Lookup lookup = MethodHandles.lookup(); @@ -206,7 +211,7 @@ public VarHandle dereferenceHandle(boolean adapt) { // If we have an enclosing layout, drop the alignment check for the accessed element, // we check the root layout instead - ValueLayout accessedLayout = enclosing != null ? valueLayout.withByteAlignment(1) : valueLayout; + ValueLayout accessedLayout = (enclosing != null && !USE_FULL_CHECKS) ? valueLayout.withByteAlignment(1) : valueLayout; VarHandle handle = accessedLayout.varHandle(); handle = MethodHandles.collectCoordinates(handle, 1, offsetHandle()); diff --git a/test/jdk/java/foreign/TestAccessModes.java b/test/jdk/java/foreign/TestAccessModes.java index 9ee5020fd9e83..b6908f682f227 100644 --- a/test/jdk/java/foreign/TestAccessModes.java +++ b/test/jdk/java/foreign/TestAccessModes.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2024, 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 @@ -23,17 +23,13 @@ /* * @test - * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes - * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes - * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes - * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes + * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes + * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes + * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes + * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes */ -import java.lang.foreign.AddressLayout; -import java.lang.foreign.Arena; -import java.lang.foreign.MemoryLayout; -import java.lang.foreign.MemorySegment; -import java.lang.foreign.ValueLayout; +import java.lang.foreign.*; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; @@ -50,10 +46,12 @@ public class TestAccessModes { @Test(dataProvider = "segmentsAndLayoutsAndModes") - public void testAccessModes(MemorySegment segment, ValueLayout layout, AccessMode mode) throws Throwable { - VarHandle varHandle = layout.varHandle(); + public void testAccessModes(MemorySegment segment, MemoryLayout layout, AccessMode mode) throws Throwable { + VarHandle varHandle = layout instanceof ValueLayout ? + layout.varHandle() : + layout.varHandle(MemoryLayout.PathElement.groupElement(0)); MethodHandle methodHandle = varHandle.toMethodHandle(mode); - boolean compatible = AccessModeKind.supportedModes(layout).contains(AccessModeKind.of(mode)); + boolean compatible = AccessModeKind.supportedModes(accessLayout(layout)).contains(AccessModeKind.of(mode)); try { Object o = methodHandle.invokeWithArguments(makeArgs(segment, varHandle.accessModeType(mode))); assertTrue(compatible); @@ -61,10 +59,19 @@ public void testAccessModes(MemorySegment segment, ValueLayout layout, AccessMod assertFalse(compatible); } catch (IllegalArgumentException ex) { // access is unaligned, but access mode is supported - assertTrue(compatible); + assertTrue(compatible || + (layout instanceof GroupLayout && segment.maxByteAlignment() < layout.byteAlignment())); } } + static ValueLayout accessLayout(MemoryLayout layout) { + return switch (layout) { + case ValueLayout vl -> vl; + case GroupLayout gl -> accessLayout(gl.memberLayouts().get(0)); + default -> throw new IllegalStateException(); + }; + } + Object[] makeArgs(MemorySegment segment, MethodType type) throws Throwable { List args = new ArrayList<>(); args.add(segment); @@ -143,8 +150,9 @@ static MemoryLayout[] layouts() { }; List layouts = new ArrayList<>(); for (MemoryLayout layout : valueLayouts) { - for (int align : new int[] { 1, 2, 4, 8 }) { + for (int align : new int[] { 2 }) { layouts.add(layout.withByteAlignment(align)); + layouts.add(MemoryLayout.structLayout(layout.withByteAlignment(align))); } } return layouts.toArray(new MemoryLayout[0]); diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java b/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java index c1c04ebd8d704..5ccfb8dba66d6 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2024, 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 @@ -23,6 +23,7 @@ package org.openjdk.bench.java.lang.foreign; import java.lang.foreign.Arena; +import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; import org.openjdk.jmh.annotations.Benchmark; @@ -37,6 +38,8 @@ import org.openjdk.jmh.annotations.Warmup; import sun.misc.Unsafe; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.concurrent.TimeUnit; @@ -57,6 +60,13 @@ public class LoopOverNonConstant extends JavaLayouts { static final int CARRIER_SIZE = (int)JAVA_INT.byteSize(); static final int ALLOC_SIZE = ELEM_SIZE * CARRIER_SIZE; + static final VarHandle VH_SEQ_INT = bindToZeroOffset(MemoryLayout.sequenceLayout(ELEM_SIZE, JAVA_INT).varHandle(PathElement.sequenceElement())); + static final VarHandle VH_SEQ_INT_UNALIGNED = bindToZeroOffset(MemoryLayout.sequenceLayout(ELEM_SIZE, JAVA_INT.withByteAlignment(1)).varHandle(PathElement.sequenceElement())); + + static VarHandle bindToZeroOffset(VarHandle varHandle) { + return MethodHandles.insertCoordinates(varHandle, 1, 0L); + } + Arena arena; MemorySegment segment; long unsafe_addr; @@ -132,6 +142,23 @@ public int segment_loop_unaligned() { return sum; } + public int segment_loop_nested() { + int sum = 0; + for (int i = 0; i < ELEM_SIZE; i++) { + sum += (int) VH_SEQ_INT.get(segment, (long) i); + } + return sum; + } + + @Benchmark + public int segment_loop_nested_unaligned() { + int sum = 0; + for (int i = 0; i < ELEM_SIZE; i++) { + sum += (int) VH_SEQ_INT_UNALIGNED.get(segment, (long) i); + } + return sum; + } + @Benchmark public int segment_loop_instance() { int sum = 0; From aa5ed59518012e292d04ee3f80a73272f1f0ef15 Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Tue, 7 May 2024 16:46:16 +0100 Subject: [PATCH 2/4] Drop JDK property --- .../share/classes/jdk/internal/foreign/LayoutPath.java | 9 +-------- test/jdk/java/foreign/TestAccessModes.java | 8 ++++---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java index a64b3809afcbb..390ad1b2f97da 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java +++ b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java @@ -26,7 +26,6 @@ package jdk.internal.foreign; import jdk.internal.vm.annotation.ForceInline; -import sun.security.action.GetPropertyAction; import java.lang.foreign.AddressLayout; import java.lang.foreign.GroupLayout; @@ -69,9 +68,6 @@ public class LayoutPath { private static final MethodHandle MH_SEGMENT_RESIZE; private static final MethodHandle MH_ADD; - private static final boolean USE_FULL_CHECKS = Boolean.parseBoolean( - GetPropertyAction.privilegedGetProperty("jdk.internal.foreign.handle.USE_FULL_CHECKS", "false")); - static { try { MethodHandles.Lookup lookup = MethodHandles.lookup(); @@ -209,10 +205,7 @@ public VarHandle dereferenceHandle(boolean adapt) { String.format("Path does not select a value layout: %s", breadcrumbs())); } - // If we have an enclosing layout, drop the alignment check for the accessed element, - // we check the root layout instead - ValueLayout accessedLayout = (enclosing != null && !USE_FULL_CHECKS) ? valueLayout.withByteAlignment(1) : valueLayout; - VarHandle handle = accessedLayout.varHandle(); + VarHandle handle = valueLayout.varHandle(); handle = MethodHandles.collectCoordinates(handle, 1, offsetHandle()); // we only have to check the alignment of the root layout for the first dereference we do, diff --git a/test/jdk/java/foreign/TestAccessModes.java b/test/jdk/java/foreign/TestAccessModes.java index b6908f682f227..3942fe3ca0ccd 100644 --- a/test/jdk/java/foreign/TestAccessModes.java +++ b/test/jdk/java/foreign/TestAccessModes.java @@ -23,10 +23,10 @@ /* * @test - * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes - * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes - * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes - * @run testng/othervm -Djdk.internal.foreign.handle.USE_FULL_CHECKS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes + * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes + * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=true -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes + * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=false -Xverify:all TestAccessModes + * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_GUARDS=false -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true -Xverify:all TestAccessModes */ import java.lang.foreign.*; From 829c5e281d069f23947cc410a1383029bce3ccd6 Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Wed, 8 May 2024 16:32:12 +0100 Subject: [PATCH 3/4] Revert spurious change to test --- test/jdk/java/foreign/TestAccessModes.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/java/foreign/TestAccessModes.java b/test/jdk/java/foreign/TestAccessModes.java index 3942fe3ca0ccd..89662c95252ce 100644 --- a/test/jdk/java/foreign/TestAccessModes.java +++ b/test/jdk/java/foreign/TestAccessModes.java @@ -150,7 +150,7 @@ static MemoryLayout[] layouts() { }; List layouts = new ArrayList<>(); for (MemoryLayout layout : valueLayouts) { - for (int align : new int[] { 2 }) { + for (int align : new int[] { 1, 2, 4, 8 }) { layouts.add(layout.withByteAlignment(align)); layouts.add(MemoryLayout.structLayout(layout.withByteAlignment(align))); } From 40ba693c32e5f07128333b72f6fe0e446f1dd56b Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Wed, 8 May 2024 17:33:44 +0100 Subject: [PATCH 4/4] Add benchmark anno --- .../org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java b/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java index 5ccfb8dba66d6..91ce8faec3854 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java @@ -142,6 +142,7 @@ public int segment_loop_unaligned() { return sum; } + @Benchmark public int segment_loop_nested() { int sum = 0; for (int i = 0; i < ELEM_SIZE; i++) {