From 2372aba6a21c569d4d724396e59b9fd1bec90682 Mon Sep 17 00:00:00 2001 From: Jorn Vernee Date: Tue, 5 Mar 2024 13:32:24 +0000 Subject: [PATCH] 8326172: Dubious claim on long[]/double[] alignment in MemorySegment javadoc Reviewed-by: mcimadamore --- .../java/lang/foreign/MemorySegment.java | 35 ++++++------------- .../java/lang/foreign/ValueLayout.java | 3 -- .../foreign/HeapMemorySegmentImpl.java | 3 -- .../foreign/abi/fallback/FallbackLinker.java | 4 +-- .../foreign/abi/fallback/LibFallback.java | 7 ++++ .../internal/foreign/layout/ValueLayouts.java | 4 +-- .../native/libfallbackLinker/fallbackLinker.c | 11 ++++++ test/jdk/java/foreign/TestLayouts.java | 20 +++++------ test/jdk/java/foreign/TestValueLayouts.java | 4 +-- .../stackwalk/TestReentrantUpcalls.java | 2 +- 10 files changed, 44 insertions(+), 49 deletions(-) diff --git a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java index faf28b01bf0..a58595cb541 100644 --- a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java +++ b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java @@ -305,8 +305,7 @@ import jdk.internal.vm.annotation.ForceInline; * and/or garbage collection behavior). *

* In practice, the Java runtime lays out arrays in memory so that each n-byte element - * occurs at an n-byte aligned physical address (except for {@code long[]} and - * {@code double[]}, where alignment is platform-dependent, as explained below). The + * occurs at an n-byte aligned physical address. The * runtime preserves this invariant even if the array is relocated during garbage * collection. Access operations rely on this invariant to determine if the specified * offset in a heap segment refers to an aligned address in physical memory. @@ -325,26 +324,17 @@ import jdk.internal.vm.annotation.ForceInline; * would correspond to physical address 1008 but offset 4 would correspond to * physical address 1010. *

  • The starting physical address of a {@code long[]} array will be 8-byte aligned - * (e.g. 1000) on 64-bit platforms, so that successive long elements occur at - * 8-byte aligned addresses (e.g., 1000, 1008, 1016, 1024, etc.) On 64-bit platforms, - * a heap segment backed by a {@code long[]} array can be accessed at offsets - * 0, 8, 16, 24, etc under an 8-byte alignment constraint. In addition, the segment - * can be accessed at offsets 0, 4, 8, 12, etc under a 4-byte alignment constraint, - * because the target addresses (1000, 1004, 1008, 1012) are 4-byte aligned. And, - * the segment can be accessed at offsets 0, 2, 4, 6, etc under a 2-byte alignment - * constraint, because the target addresses (e.g. 1000, 1002, 1004, 1006) are - * 2-byte aligned.
    • - *
  • The starting physical address of a {@code long[]} array will be 4-byte aligned - * (e.g. 1004) on 32-bit platforms, so that successive long elements occur at 4-byte - * aligned addresses (e.g., 1004, 1008, 1012, 1016, etc.) On 32-bit platforms, a heap - * segment backed by a {@code long[]} array can be accessed at offsets - * 0, 4, 8, 12, etc under a 4-byte alignment constraint, because the target addresses - * (1004, 1008, 1012, 1016) are 4-byte aligned. And, the segment can be accessed at - * offsets 0, 2, 4, 6, etc under a 2-byte alignment constraint, because the target - * addresses (e.g. 1000, 1002, 1004, 1006) are 2-byte aligned.
    • + * (e.g. 1000), so that successive long elements occur at 8-byte aligned addresses + * (e.g., 1000, 1008, 1016, 1024, etc.) A heap segment backed by a {@code long[]} + * array can be accessed at offsets 0, 8, 16, 24, etc under an 8-byte alignment + * constraint. In addition, the segment can be accessed at offsets 0, 4, 8, 12, + * etc under a 4-byte alignment constraint, because the target addresses (1000, 1004, + * 1008, 1012) are 4-byte aligned. And, the segment can be accessed at offsets 0, 2, + * 4, 6, etc under a 2-byte alignment constraint, because the target addresses (e.g. + * 1000, 1002, 1004, 1006) are 2-byte aligned. * *

    - * In other words, heap segments feature a (platform-dependent) maximum + * In other words, heap segments feature a maximum * alignment which is derived from the size of the elements of the Java array backing the * segment, as shown in the following table: * @@ -389,10 +379,7 @@ import jdk.internal.vm.annotation.ForceInline; * In such circumstances, clients have two options. They can use a heap segment backed * by a different array type (e.g. {@code long[]}), capable of supporting greater maximum * alignment. More specifically, the maximum alignment associated with {@code long[]} is - * set to {@code ValueLayout.JAVA_LONG.byteAlignment()} which is a platform-dependent - * value (set to {@code ValueLayout.ADDRESS.byteSize()}). That is, {@code long[]}) is - * guaranteed to provide at least 8-byte alignment in 64-bit platforms, but only 4-byte - * alignment in 32-bit platforms: + * set to {@code ValueLayout.JAVA_LONG.byteAlignment()}, which is 8 bytes: * * {@snippet lang=java : * MemorySegment longSegment = MemorySegment.ofArray(new long[10]); diff --git a/src/java.base/share/classes/java/lang/foreign/ValueLayout.java b/src/java.base/share/classes/java/lang/foreign/ValueLayout.java index e8740be387c..77bfe0e0209 100644 --- a/src/java.base/share/classes/java/lang/foreign/ValueLayout.java +++ b/src/java.base/share/classes/java/lang/foreign/ValueLayout.java @@ -47,9 +47,6 @@ import jdk.internal.foreign.layout.ValueLayouts; * For instance, the byte order of these constants is set to the * {@linkplain ByteOrder#nativeOrder() native byte order}, thus making it easy * to work with other APIs, such as arrays and {@link java.nio.ByteBuffer}. - * Moreover, the alignment constraint of {@link ValueLayout#JAVA_LONG} and - * {@link ValueLayout#JAVA_DOUBLE} is set to 8 bytes on 64-bit platforms, - * but only to 4 bytes on 32-bit platforms. * * @implSpec implementing classes and subclasses are immutable, thread-safe and * value-based. diff --git a/src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java b/src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java index e1ea65ff308..0b95c43b440 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java +++ b/src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java @@ -49,9 +49,6 @@ import jdk.internal.vm.annotation.ForceInline; abstract sealed class HeapMemorySegmentImpl extends AbstractMemorySegmentImpl { // Constants defining the maximum alignment supported by various kinds of heap arrays. - // While for most arrays, the maximum alignment is constant (the size, in bytes, of the array elements), - // note that the alignment of a long[]/double[] depends on the platform: it's 4-byte on x86, but 8 bytes on x64 - // (as specified by the JAVA_LONG layout constant). private static final long MAX_ALIGN_BYTE_ARRAY = ValueLayout.JAVA_BYTE.byteAlignment(); private static final long MAX_ALIGN_SHORT_ARRAY = ValueLayout.JAVA_SHORT.byteAlignment(); diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java index 77c81c6dd58..d617e535dd6 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java @@ -307,8 +307,8 @@ public final class FallbackLinker extends AbstractLinker { Map.entry("bool", JAVA_BOOLEAN), Map.entry("char", JAVA_BYTE), Map.entry("float", JAVA_FLOAT), - Map.entry("long long", JAVA_LONG), - Map.entry("double", JAVA_DOUBLE), + Map.entry("long long", JAVA_LONG.withByteAlignment(LibFallback.longLongAlign())), + Map.entry("double", JAVA_DOUBLE.withByteAlignment(LibFallback.doubleAlign())), Map.entry("void*", ADDRESS), // platform-dependent sizes Map.entry("size_t", FFIType.SIZE_T), diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java index b0b6bac3d64..58d6baf8525 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java @@ -73,6 +73,8 @@ final class LibFallback { static int intSize() { return NativeConstants.SIZEOF_INT; } static int longSize() {return NativeConstants.SIZEOF_LONG; } static int wcharSize() {return NativeConstants.SIZEOF_WCHAR; } + static int longLongAlign() { return NativeConstants.ALIGNOF_LONG_LONG; } + static int doubleAlign() { return NativeConstants.ALIGNOF_DOUBLE; } static short structTag() { return NativeConstants.STRUCT_TAG; } @@ -242,6 +244,9 @@ final class LibFallback { private static native int ffi_sizeof_long(); private static native int ffi_sizeof_wchar(); + private static native int alignof_long_long(); + private static native int alignof_double(); + // put these in a separate class to avoid an UnsatisfiedLinkError // when LibFallback is initialized but the library is not present private static final class NativeConstants { @@ -263,6 +268,8 @@ final class LibFallback { static final int SIZEOF_LONG = ffi_sizeof_long(); static final int SIZEOF_WCHAR = ffi_sizeof_wchar(); + static final int ALIGNOF_LONG_LONG = alignof_long_long(); + static final int ALIGNOF_DOUBLE = alignof_double(); static final MemorySegment VOID_TYPE = MemorySegment.ofAddress(ffi_type_void()); static final short STRUCT_TAG = ffi_type_struct(); diff --git a/src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java b/src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java index cc61ed41a98..4b41c80f2eb 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java +++ b/src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java @@ -278,7 +278,7 @@ public final class ValueLayouts { } public static OfLong of(ByteOrder order) { - return new OfLongImpl(order, ADDRESS_SIZE_BYTES, Optional.empty()); + return new OfLongImpl(order, Long.BYTES, Optional.empty()); } } @@ -294,7 +294,7 @@ public final class ValueLayouts { } public static OfDouble of(ByteOrder order) { - return new OfDoubleImpl(order, ADDRESS_SIZE_BYTES, Optional.empty()); + return new OfDoubleImpl(order, Double.BYTES, Optional.empty()); } } diff --git a/src/java.base/share/native/libfallbackLinker/fallbackLinker.c b/src/java.base/share/native/libfallbackLinker/fallbackLinker.c index 39d869adb63..2ee64fb05bc 100644 --- a/src/java.base/share/native/libfallbackLinker/fallbackLinker.c +++ b/src/java.base/share/native/libfallbackLinker/fallbackLinker.c @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -275,3 +276,13 @@ JNIEXPORT jint JNICALL Java_jdk_internal_foreign_abi_fallback_LibFallback_ffi_1sizeof_1wchar(JNIEnv* env, jclass cls) { return sizeof(wchar_t); } + +JNIEXPORT jint JNICALL +Java_jdk_internal_foreign_abi_fallback_LibFallback_alignof_1long_1long(JNIEnv* env, jclass cls) { + return alignof(long long); +} + +JNIEXPORT jint JNICALL +Java_jdk_internal_foreign_abi_fallback_LibFallback_alignof_1double(JNIEnv* env, jclass cls) { + return alignof(double); +} diff --git a/test/jdk/java/foreign/TestLayouts.java b/test/jdk/java/foreign/TestLayouts.java index 2aa398468cc..eb271db4e5e 100644 --- a/test/jdk/java/foreign/TestLayouts.java +++ b/test/jdk/java/foreign/TestLayouts.java @@ -161,7 +161,7 @@ public class TestLayouts { ValueLayout.JAVA_LONG ); assertEquals(struct.byteSize(), 1 + 1 + 2 + 4 + 8); - assertEquals(struct.byteAlignment(), ADDRESS.byteSize()); + assertEquals(struct.byteAlignment(), 8); } @Test(dataProvider="basicLayouts") @@ -192,7 +192,7 @@ public class TestLayouts { ValueLayout.JAVA_LONG ); assertEquals(struct.byteSize(), 8); - assertEquals(struct.byteAlignment(), ADDRESS.byteSize()); + assertEquals(struct.byteAlignment(), 8); } @Test @@ -477,24 +477,24 @@ public class TestLayouts { List layoutsAndAlignments = new ArrayList<>(); int i = 0; //add basic layouts - for (MemoryLayout l : basicLayoutsNoLongDouble) { + for (MemoryLayout l : basicLayouts) { layoutsAndAlignments.add(new Object[] { l, l.byteAlignment() }); } //add basic layouts wrapped in a sequence with given size - for (MemoryLayout l : basicLayoutsNoLongDouble) { + for (MemoryLayout l : basicLayouts) { layoutsAndAlignments.add(new Object[] { MemoryLayout.sequenceLayout(4, l), l.byteAlignment() }); } //add basic layouts wrapped in a struct - for (MemoryLayout l1 : basicLayoutsNoLongDouble) { - for (MemoryLayout l2 : basicLayoutsNoLongDouble) { + for (MemoryLayout l1 : basicLayouts) { + for (MemoryLayout l2 : basicLayouts) { if (l1.byteSize() % l2.byteAlignment() != 0) continue; // second element is not aligned, skip long align = Math.max(l1.byteAlignment(), l2.byteAlignment()); layoutsAndAlignments.add(new Object[]{MemoryLayout.structLayout(l1, l2), align}); } } //add basic layouts wrapped in a union - for (MemoryLayout l1 : basicLayoutsNoLongDouble) { - for (MemoryLayout l2 : basicLayoutsNoLongDouble) { + for (MemoryLayout l1 : basicLayouts) { + for (MemoryLayout l2 : basicLayouts) { long align = Math.max(l1.byteAlignment(), l2.byteAlignment()); layoutsAndAlignments.add(new Object[]{MemoryLayout.unionLayout(l1, l2), align}); } @@ -543,8 +543,4 @@ public class TestLayouts { ValueLayout.JAVA_LONG, ValueLayout.JAVA_DOUBLE, }; - - static MemoryLayout[] basicLayoutsNoLongDouble = Stream.of(basicLayouts) - .filter(l -> l.carrier() != long.class && l.carrier() != double.class) - .toArray(MemoryLayout[]::new); } diff --git a/test/jdk/java/foreign/TestValueLayouts.java b/test/jdk/java/foreign/TestValueLayouts.java index ec44ad60aca..50f7f4c753f 100644 --- a/test/jdk/java/foreign/TestValueLayouts.java +++ b/test/jdk/java/foreign/TestValueLayouts.java @@ -70,7 +70,7 @@ public class TestValueLayouts { @Test public void testLong() { - testAligned(JAVA_LONG, long.class, Long.BYTES, ADDRESS.byteSize()); + testAligned(JAVA_LONG, long.class, Long.BYTES, Long.BYTES); } @Test @@ -90,7 +90,7 @@ public class TestValueLayouts { @Test public void testDouble() { - testAligned(JAVA_DOUBLE, double.class, Double.BYTES, ADDRESS.byteSize()); + testAligned(JAVA_DOUBLE, double.class, Double.BYTES, Double.BYTES); } @Test diff --git a/test/jdk/java/foreign/stackwalk/TestReentrantUpcalls.java b/test/jdk/java/foreign/stackwalk/TestReentrantUpcalls.java index e69b44b4aa0..1d776aed128 100644 --- a/test/jdk/java/foreign/stackwalk/TestReentrantUpcalls.java +++ b/test/jdk/java/foreign/stackwalk/TestReentrantUpcalls.java @@ -76,7 +76,7 @@ public class TestReentrantUpcalls extends NativeTestHelper { } static void m(int depth, MemorySegment thisStub, MethodHandle downcallHandle) throws Throwable { - if (depth < 100) { + if (depth < 50) { downcallHandle.invokeExact(depth + 1, thisStub); } else { WB.verifyFrames(/*log=*/true, /*updateRegisterMap=*/true);