Skip to content

8349183: [BACKOUT] Optimization for StringBuilder append boolean & null #23420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

8349183: [BACKOUT] Optimization for StringBuilder append boolean & null #23420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6556a83
Revert "8343650: Reuse StringLatin1::putCharsAt and StringUTF16::putC…
jaikiran Feb 3, 2025
23d01f0
Revert "8333893: Optimization for StringBuilder append boolean & null"
jaikiran Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/java.base/share/classes/java/lang/AbstractStringBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,14 @@ private AbstractStringBuilder appendNull() {
int count = this.count;
byte[] val = this.value;
if (isLatin1()) {
StringLatin1.putCharsAt(val, count, 'n', 'u', 'l', 'l');
val[count++] = 'n';
val[count++] = 'u';
val[count++] = 'l';
val[count++] = 'l';
} else {
StringUTF16.putCharsAt(val, count, 'n', 'u', 'l', 'l');
count = StringUTF16.putCharsAt(val, count, 'n', 'u', 'l', 'l');
}
this.count = count + 4;
this.count = count;
return this;
}

Expand Down Expand Up @@ -769,18 +772,25 @@ public AbstractStringBuilder append(boolean b) {
byte[] val = this.value;
if (isLatin1()) {
if (b) {
StringLatin1.putCharsAt(val, count, 't', 'r', 'u', 'e');
val[count++] = 't';
val[count++] = 'r';
val[count++] = 'u';
val[count++] = 'e';
} else {
StringLatin1.putCharsAt(val, count, 'f', 'a', 'l', 's', 'e');
val[count++] = 'f';
val[count++] = 'a';
val[count++] = 'l';
val[count++] = 's';
val[count++] = 'e';
}
} else {
if (b) {
StringUTF16.putCharsAt(val, count, 't', 'r', 'u', 'e');
count = StringUTF16.putCharsAt(val, count, 't', 'r', 'u', 'e');
} else {
StringUTF16.putCharsAt(val, count, 'f', 'a', 'l', 's', 'e');
count = StringUTF16.putCharsAt(val, count, 'f', 'a', 'l', 's', 'e');
}
}
this.count = count + (b ? 4 : 5);
this.count = count;
return this;
}

Expand Down
44 changes: 36 additions & 8 deletions src/java.base/share/classes/java/lang/StringConcatHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,21 +236,35 @@ static long prepend(long indexCoder, byte[] buf, boolean value, String prefix) {
if (indexCoder < UTF16) {
if (value) {
index -= 4;
StringLatin1.putCharsAt(buf, index, 't', 'r', 'u', 'e');
buf[index] = 't';
buf[index + 1] = 'r';
buf[index + 2] = 'u';
buf[index + 3] = 'e';
} else {
index -= 5;
StringLatin1.putCharsAt(buf, index, 'f', 'a', 'l', 's', 'e');
buf[index] = 'f';
buf[index + 1] = 'a';
buf[index + 2] = 'l';
buf[index + 3] = 's';
buf[index + 4] = 'e';
}
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
return index;
} else {
if (value) {
index -= 4;
StringUTF16.putCharsAt(buf, index, 't', 'r', 'u', 'e');
StringUTF16.putChar(buf, index, 't');
StringUTF16.putChar(buf, index + 1, 'r');
StringUTF16.putChar(buf, index + 2, 'u');
StringUTF16.putChar(buf, index + 3, 'e');
} else {
index -= 5;
StringUTF16.putCharsAt(buf, index, 'f', 'a', 'l', 's', 'e');
StringUTF16.putChar(buf, index, 'f');
StringUTF16.putChar(buf, index + 1, 'a');
StringUTF16.putChar(buf, index + 2, 'l');
StringUTF16.putChar(buf, index + 3, 's');
StringUTF16.putChar(buf, index + 4, 'e');
}
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
Expand Down Expand Up @@ -624,20 +638,34 @@ static int prepend(int index, byte coder, byte[] buf, boolean value, String pref
if (coder == String.LATIN1) {
if (value) {
index -= 4;
StringLatin1.putCharsAt(buf, index, 't', 'r', 'u', 'e');
buf[index] = 't';
buf[index + 1] = 'r';
buf[index + 2] = 'u';
buf[index + 3] = 'e';
} else {
index -= 5;
StringLatin1.putCharsAt(buf, index, 'f', 'a', 'l', 's', 'e');
buf[index] = 'f';
buf[index + 1] = 'a';
buf[index + 2] = 'l';
buf[index + 3] = 's';
buf[index + 4] = 'e';
}
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
} else {
if (value) {
index -= 4;
StringUTF16.putCharsAt(buf, index, 't', 'r', 'u', 'e');
StringUTF16.putChar(buf, index, 't');
StringUTF16.putChar(buf, index + 1, 'r');
StringUTF16.putChar(buf, index + 2, 'u');
StringUTF16.putChar(buf, index + 3, 'e');
} else {
index -= 5;
StringUTF16.putCharsAt(buf, index, 'f', 'a', 'l', 's', 'e');
StringUTF16.putChar(buf, index, 'f');
StringUTF16.putChar(buf, index + 1, 'a');
StringUTF16.putChar(buf, index + 2, 'l');
StringUTF16.putChar(buf, index + 3, 's');
StringUTF16.putChar(buf, index + 4, 'e');
}
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
Expand Down
24 changes: 0 additions & 24 deletions src/java.base/share/classes/java/lang/StringLatin1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.function.IntConsumer;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import jdk.internal.misc.Unsafe;
import jdk.internal.util.ArraysSupport;
import jdk.internal.vm.annotation.IntrinsicCandidate;

Expand All @@ -42,8 +41,6 @@
import static java.lang.String.checkOffset;

final class StringLatin1 {
private static final Unsafe UNSAFE = Unsafe.getUnsafe();

public static char charAt(byte[] value, int index) {
checkIndex(index, value.length);
return (char)(value[index] & 0xff);
Expand Down Expand Up @@ -712,27 +709,6 @@ static Stream<String> lines(byte[] value) {
return StreamSupport.stream(LinesSpliterator.spliterator(value), false);
}

static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4) {
assert index >= 0 && index + 3 < length(val) : "Trusted caller missed bounds check";
// Don't use the putChar method, Its instrinsic will cause C2 unable to combining values into larger stores.
long offset = Unsafe.ARRAY_BYTE_BASE_OFFSET + index;
UNSAFE.putByte(val, offset , (byte)(c1));
UNSAFE.putByte(val, offset + 1, (byte)(c2));
UNSAFE.putByte(val, offset + 2, (byte)(c3));
UNSAFE.putByte(val, offset + 3, (byte)(c4));
}

static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4, int c5) {
assert index >= 0 && index + 4 < length(val) : "Trusted caller missed bounds check";
// Don't use the putChar method, Its instrinsic will cause C2 unable to combining values into larger stores.
long offset = Unsafe.ARRAY_BYTE_BASE_OFFSET + index;
UNSAFE.putByte(val, offset , (byte)(c1));
UNSAFE.putByte(val, offset + 1, (byte)(c2));
UNSAFE.putByte(val, offset + 2, (byte)(c3));
UNSAFE.putByte(val, offset + 3, (byte)(c4));
UNSAFE.putByte(val, offset + 4, (byte)(c5));
}

public static void putChar(byte[] val, int index, int c) {
//assert (canEncode(c));
val[index] = (byte)(c);
Expand Down
36 changes: 22 additions & 14 deletions src/java.base/share/classes/java/lang/StringUTF16.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static java.lang.String.LATIN1;

final class StringUTF16 {

// Return a new byte array for a UTF16-coded string for len chars
// Throw an exception if out of range
public static byte[] newBytesFor(int len) {
Expand Down Expand Up @@ -1532,20 +1533,27 @@ public static boolean contentEquals(byte[] value, CharSequence cs, int len) {
return true;
}

static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4) {
assert index >= 0 && index + 3 < length(val) : "Trusted caller missed bounds check";
putChar(val, index , c1);
putChar(val, index + 1, c2);
putChar(val, index + 2, c3);
putChar(val, index + 3, c4);
}

static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4, int c5) {
putChar(val, index , c1);
putChar(val, index + 1, c2);
putChar(val, index + 2, c3);
putChar(val, index + 3, c4);
putChar(val, index + 4, c5);
public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
int end = i + 4;
checkBoundsBeginEnd(i, end, value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have this explicit check for null and true, false has another bound check. This backout version should be safe.

putChar(value, i++, c1);
putChar(value, i++, c2);
putChar(value, i++, c3);
putChar(value, i++, c4);
assert(i == end);
return end;
}

public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
int end = i + 5;
checkBoundsBeginEnd(i, end, value);
putChar(value, i++, c1);
putChar(value, i++, c2);
putChar(value, i++, c3);
putChar(value, i++, c4);
putChar(value, i++, c5);
assert(i == end);
return end;
}

public static char charAt(byte[] value, int index) {
Expand Down
12 changes: 3 additions & 9 deletions test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2018, 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
Expand Down Expand Up @@ -141,17 +141,11 @@ public static boolean contentEquals(byte[] value, CharSequence cs, int len) {
}

public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
int end = i + 4;
StringUTF16.checkBoundsBeginEnd(i, end, value);
StringUTF16.putCharsAt(value, i, c1, c2, c3, c4);
return end;
return StringUTF16.putCharsAt(value, i, c1, c2, c3, c4);
}

public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
int end = i + 5;
StringUTF16.checkBoundsBeginEnd(i, end, value);
StringUTF16.putCharsAt(value, i, c1, c2, c3, c4, c5);
return end;
return StringUTF16.putCharsAt(value, i, c1, c2, c3, c4, c5);
}

public static char charAt(byte[] value, int index) {
Expand Down
71 changes: 11 additions & 60 deletions test/micro/org/openjdk/bench/java/lang/StringBuilders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,66 +274,17 @@ public int appendWithLongUtf16() {
}

@Benchmark
public int appendWithBool8Latin1() {
StringBuilder buf = sbLatin1;
buf.setLength(0);
buf.append(true);
buf.append(false);
buf.append(true);
buf.append(true);
buf.append(false);
buf.append(true);
buf.append(false);
buf.append(false);
return buf.length();
}


@Benchmark
public int appendWithBool8Utf16() {
StringBuilder buf = sbUtf16;
buf.setLength(0);
buf.append(true);
buf.append(false);
buf.append(true);
buf.append(true);
buf.append(false);
buf.append(true);
buf.append(false);
buf.append(false);
return buf.length();
}


@Benchmark
public int appendWithNull8Latin1() {
StringBuilder buf = sbLatin1;
buf.setLength(0);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
return buf.length();
}


@Benchmark
public int appendWithNull8Utf16() {
StringBuilder buf = sbUtf16;
buf.setLength(0);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
buf.append((String) null);
return buf.length();
public String toStringCharWithBool8() {
StringBuilder result = new StringBuilder();
result.append(true);
result.append(false);
result.append(true);
result.append(true);
result.append(false);
result.append(true);
result.append(false);
result.append(false);
return result.toString();
}


Expand Down