From 2aaec5b0536a9b72117f0690aa2a442a223a2117 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Tue, 3 Dec 2024 12:36:26 +0100 Subject: [PATCH 1/6] Use aliases for legacy code. --- .../java/lang/foreign/InternalStrLen.java | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java b/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java index b7867efd77109..3870406d5e531 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java @@ -55,10 +55,16 @@ "--enable-native-access=ALL-UNNAMED"}) public class InternalStrLen { - private AbstractMemorySegmentImpl singleByteSegment; - private AbstractMemorySegmentImpl singleByteSegmentMisaligned; - private AbstractMemorySegmentImpl doubleByteSegment; - private AbstractMemorySegmentImpl quadByteSegment; + private MemorySegment singleByteSegment; + private MemorySegment singleByteSegmentMisaligned; + private MemorySegment doubleByteSegment; + private MemorySegment quadByteSegment; + + // Aliases for testing internal methods that already have access to AMSI + private AbstractMemorySegmentImpl abstractSingleByteSegment; + private AbstractMemorySegmentImpl abstractSingleByteSegmentMisaligned; + private AbstractMemorySegmentImpl abstractDoubleByteSegment; + private AbstractMemorySegmentImpl abstractQuadByteSegment; @Param({"1", "4", "16", "251", "1024"}) int size; @@ -66,9 +72,9 @@ public class InternalStrLen { @Setup public void setup() { var arena = Arena.ofAuto(); - singleByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Byte.BYTES); - doubleByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Short.BYTES); - quadByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Integer.BYTES); + singleByteSegment = arena.allocate((size + 1L) * Byte.BYTES); + doubleByteSegment = arena.allocate((size + 1L) * Short.BYTES); + quadByteSegment = arena.allocate((size + 1L) * Integer.BYTES); Stream.of(singleByteSegment, doubleByteSegment, quadByteSegment) .forEach(s -> IntStream.range(0, (int) s.byteSize() - 1) .forEach(i -> s.set( @@ -79,9 +85,13 @@ public void setup() { singleByteSegment.set(ValueLayout.JAVA_BYTE, singleByteSegment.byteSize() - Byte.BYTES, (byte) 0); doubleByteSegment.set(ValueLayout.JAVA_SHORT, doubleByteSegment.byteSize() - Short.BYTES, (short) 0); quadByteSegment.set(ValueLayout.JAVA_INT, quadByteSegment.byteSize() - Integer.BYTES, 0); - singleByteSegmentMisaligned = (AbstractMemorySegmentImpl) arena.allocate(singleByteSegment.byteSize() + 1). + singleByteSegmentMisaligned = arena.allocate(singleByteSegment.byteSize() + 1). asSlice(1); MemorySegment.copy(singleByteSegment, 0, singleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); + abstractSingleByteSegment = (AbstractMemorySegmentImpl) singleByteSegment; + abstractSingleByteSegmentMisaligned = (AbstractMemorySegmentImpl) singleByteSegmentMisaligned; + abstractDoubleByteSegment = (AbstractMemorySegmentImpl) doubleByteSegment; + abstractQuadByteSegment = (AbstractMemorySegmentImpl) quadByteSegment; } @Benchmark @@ -106,22 +116,22 @@ public int elementQuad() { @Benchmark public int chunkedSingle() { - return StringSupport.strlenByte(singleByteSegment, 0, singleByteSegment.byteSize()); + return StringSupport.strlenByte(abstractSingleByteSegment, 0, singleByteSegment.byteSize()); } @Benchmark public int chunkedSingleMisaligned() { - return StringSupport.strlenByte(singleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); + return StringSupport.strlenByte(abstractSingleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); } @Benchmark public int chunkedDouble() { - return StringSupport.strlenShort(doubleByteSegment, 0, doubleByteSegment.byteSize()); + return StringSupport.strlenShort(abstractDoubleByteSegment, 0, doubleByteSegment.byteSize()); } @Benchmark public int changedElementQuad() { - return StringSupport.strlenInt(quadByteSegment, 0, quadByteSegment.byteSize()); + return StringSupport.strlenInt(abstractQuadByteSegment, 0, quadByteSegment.byteSize()); } // These are the legacy methods From f509b6c035de4ebc0553f11697be18e1adbc8830 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Tue, 3 Dec 2024 17:18:17 +0100 Subject: [PATCH 2/6] Go back to use MemorySegment types --- .../java/lang/foreign/InternalStrLen.java | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java b/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java index 3870406d5e531..4b2a5222159cf 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java @@ -60,12 +60,6 @@ public class InternalStrLen { private MemorySegment doubleByteSegment; private MemorySegment quadByteSegment; - // Aliases for testing internal methods that already have access to AMSI - private AbstractMemorySegmentImpl abstractSingleByteSegment; - private AbstractMemorySegmentImpl abstractSingleByteSegmentMisaligned; - private AbstractMemorySegmentImpl abstractDoubleByteSegment; - private AbstractMemorySegmentImpl abstractQuadByteSegment; - @Param({"1", "4", "16", "251", "1024"}) int size; @@ -88,10 +82,6 @@ public void setup() { singleByteSegmentMisaligned = arena.allocate(singleByteSegment.byteSize() + 1). asSlice(1); MemorySegment.copy(singleByteSegment, 0, singleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); - abstractSingleByteSegment = (AbstractMemorySegmentImpl) singleByteSegment; - abstractSingleByteSegmentMisaligned = (AbstractMemorySegmentImpl) singleByteSegmentMisaligned; - abstractDoubleByteSegment = (AbstractMemorySegmentImpl) doubleByteSegment; - abstractQuadByteSegment = (AbstractMemorySegmentImpl) quadByteSegment; } @Benchmark @@ -116,22 +106,22 @@ public int elementQuad() { @Benchmark public int chunkedSingle() { - return StringSupport.strlenByte(abstractSingleByteSegment, 0, singleByteSegment.byteSize()); + return StringSupport.strlenByte((AbstractMemorySegmentImpl) singleByteSegment, 0, singleByteSegment.byteSize()); } @Benchmark public int chunkedSingleMisaligned() { - return StringSupport.strlenByte(abstractSingleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); + return StringSupport.strlenByte((AbstractMemorySegmentImpl) singleByteSegmentMisaligned, 0, singleByteSegment.byteSize()); } @Benchmark public int chunkedDouble() { - return StringSupport.strlenShort(abstractDoubleByteSegment, 0, doubleByteSegment.byteSize()); + return StringSupport.strlenShort((AbstractMemorySegmentImpl) doubleByteSegment, 0, doubleByteSegment.byteSize()); } @Benchmark public int changedElementQuad() { - return StringSupport.strlenInt(abstractQuadByteSegment, 0, quadByteSegment.byteSize()); + return StringSupport.strlenInt((AbstractMemorySegmentImpl) quadByteSegment, 0, quadByteSegment.byteSize()); } // These are the legacy methods From 779b7f91a948663aed5440f8fd5a200a96c47079 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 4 Dec 2024 09:32:02 +0100 Subject: [PATCH 3/6] Fix regression on x64 --- .../jdk/internal/foreign/StringSupport.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java index 8f182f3b33845..6dabcaf76a92d 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java +++ b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java @@ -203,15 +203,18 @@ public static int strlenInt(final AbstractMemorySegmentImpl segment, segment.scope.checkValidState(); throw nullNotFound(segment, fromOffset, toOffset); } - final long longBytes = length & LONG_MASK; - final long longLimit = fromOffset + longBytes; long offset = fromOffset; - for (; offset < longLimit; offset += Long.BYTES) { - long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian()); - if (mightContainZeroInt(val)) { - for (int j = 0; j < Long.BYTES; j += Integer.BYTES) { - if (SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset + j, !Architecture.isLittleEndian()) == 0) { - return requireWithinStringSize(offset + j - fromOffset, segment, fromOffset, toOffset); + // For quad words, it does not pay off to use long scanning on x64 + if (!Architecture.isX64()) { + final long longBytes = length & LONG_MASK; + final long longLimit = fromOffset + longBytes; + for (; offset < longLimit; offset += Long.BYTES) { + long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian()); + if (mightContainZeroInt(val)) { + for (int j = 0; j < Long.BYTES; j += Integer.BYTES) { + if (SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset + j, !Architecture.isLittleEndian()) == 0) { + return requireWithinStringSize(offset + j - fromOffset, segment, fromOffset, toOffset); + } } } } From 4d41488b9224d1520ad15d4a3e7e7bf8c217cda3 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 4 Dec 2024 15:58:22 +0100 Subject: [PATCH 4/6] Update comment --- .../share/classes/jdk/internal/foreign/StringSupport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java index 6dabcaf76a92d..82d57fcc3eaab 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java +++ b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java @@ -204,7 +204,7 @@ public static int strlenInt(final AbstractMemorySegmentImpl segment, throw nullNotFound(segment, fromOffset, toOffset); } long offset = fromOffset; - // For quad words, it does not pay off to use long scanning on x64 + // For quad byte strings, it does not pay off to use long scanning on x64 if (!Architecture.isX64()) { final long longBytes = length & LONG_MASK; final long longLimit = fromOffset + longBytes; From 96c3a55caabef8ef13bb3e551d42a4a73119e505 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Thu, 5 Dec 2024 12:39:59 +0100 Subject: [PATCH 5/6] Improve short string cases --- .../jdk/internal/foreign/StringSupport.java | 73 ++++++++++++++++--- 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java index 82d57fcc3eaab..65d9751bb2c5a 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java +++ b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java @@ -130,10 +130,27 @@ public static int strlenByte(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length == 0) { - // The state has to be checked explicitly for zero-length segments - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); + if (length < 3) { + switch ((int) length) { + case 0: + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); + case 1: + if (SCOPED_MEMORY_ACCESS.getByte(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset) == 0) { + return 0; + } + throw nullNotFound(segment, fromOffset, toOffset); + case 2: + final short val = SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); + if ((val & 0x00ff) == 0) { + return 0; + } + if ((val & 0xff00) == 0) { + return 1; + } + throw nullNotFound(segment, fromOffset, toOffset); + } } final long longBytes = length & LONG_MASK; final long longLimit = fromOffset + longBytes; @@ -164,9 +181,27 @@ public static int strlenShort(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length == 0) { - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); + if (length < 3 * Short.BYTES) { + switch ((int) length >> 1) { + case 0: + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); + case 1: + if (SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()) == 0) { + return 0; + } + throw nullNotFound(segment, fromOffset, toOffset); + case 2: + final int val = SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); + if ((val & 0x0000_ffff) == 0) { + return 0; + } + if ((val & 0xffff_0000) == 0) { + return Short.BYTES; + } + throw nullNotFound(segment, fromOffset, toOffset); + } } final long longBytes = length & LONG_MASK; final long longLimit = fromOffset + longBytes; @@ -199,9 +234,27 @@ public static int strlenInt(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length == 0) { - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); + if (length < 3 * Integer.BYTES) { + switch ((int) length >> 2) { + case 0: + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); + case 1: + if (SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()) == 0) { + return 0; + } + throw nullNotFound(segment, fromOffset, toOffset); + case 2: + final long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); + if ((val & 0x0000_0000__ffff_ffffL) == 0) { + return 0; + } + if ((val & 0xffff_ffff__0000_0000L) == 0) { + return Integer.BYTES; + } + throw nullNotFound(segment, fromOffset, toOffset); + } } long offset = fromOffset; // For quad byte strings, it does not pay off to use long scanning on x64 From e4b0b479e17414af7f9c6eec7c65afcfd6123363 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Tue, 10 Dec 2024 08:26:50 +0100 Subject: [PATCH 6/6] Simplify code --- .../jdk/internal/foreign/StringSupport.java | 75 +++---------------- 1 file changed, 12 insertions(+), 63 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java index 65d9751bb2c5a..6dc104ff32394 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java +++ b/src/java.base/share/classes/jdk/internal/foreign/StringSupport.java @@ -130,27 +130,10 @@ public static int strlenByte(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length < 3) { - switch ((int) length) { - case 0: - // There can be no null terminator present - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); - case 1: - if (SCOPED_MEMORY_ACCESS.getByte(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset) == 0) { - return 0; - } - throw nullNotFound(segment, fromOffset, toOffset); - case 2: - final short val = SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); - if ((val & 0x00ff) == 0) { - return 0; - } - if ((val & 0xff00) == 0) { - return 1; - } - throw nullNotFound(segment, fromOffset, toOffset); - } + if (length < Byte.BYTES) { + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); } final long longBytes = length & LONG_MASK; final long longLimit = fromOffset + longBytes; @@ -181,27 +164,10 @@ public static int strlenShort(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length < 3 * Short.BYTES) { - switch ((int) length >> 1) { - case 0: - // There can be no null terminator present - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); - case 1: - if (SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()) == 0) { - return 0; - } - throw nullNotFound(segment, fromOffset, toOffset); - case 2: - final int val = SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); - if ((val & 0x0000_ffff) == 0) { - return 0; - } - if ((val & 0xffff_0000) == 0) { - return Short.BYTES; - } - throw nullNotFound(segment, fromOffset, toOffset); - } + if (length < Short.BYTES) { + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); } final long longBytes = length & LONG_MASK; final long longLimit = fromOffset + longBytes; @@ -234,27 +200,10 @@ public static int strlenInt(final AbstractMemorySegmentImpl segment, final long toOffset) { final long length = toOffset - fromOffset; segment.checkBounds(fromOffset, length); - if (length < 3 * Integer.BYTES) { - switch ((int) length >> 2) { - case 0: - // There can be no null terminator present - segment.scope.checkValidState(); - throw nullNotFound(segment, fromOffset, toOffset); - case 1: - if (SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()) == 0) { - return 0; - } - throw nullNotFound(segment, fromOffset, toOffset); - case 2: - final long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian()); - if ((val & 0x0000_0000__ffff_ffffL) == 0) { - return 0; - } - if ((val & 0xffff_ffff__0000_0000L) == 0) { - return Integer.BYTES; - } - throw nullNotFound(segment, fromOffset, toOffset); - } + if (length < Integer.BYTES) { + // There can be no null terminator present + segment.scope.checkValidState(); + throw nullNotFound(segment, fromOffset, toOffset); } long offset = fromOffset; // For quad byte strings, it does not pay off to use long scanning on x64