String.replaceAll() fast path must check regex is compressed

Also, return the original String in the fast paths when nothing is replaced. Issue #19903 Signed-off-by: Peter Shipton <[email protected]>
eclipse-openj9 · Jul 23, 2024 · fb8a0da · fb8a0da
1 parent c70741a
commit fb8a0da
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 8 deletions.
diff --git a/jcl/src/java.base/share/classes/java/lang/String.java b/jcl/src/java.base/share/classes/java/lang/String.java
@@ -3202,6 +3202,9 @@ public String replaceAll(String regex, String substitute) {
 			int length = lengthInternal();
 			if (substituteLength < 2) {
 				if (COMPACT_STRINGS && isCompressed() && (substituteLength == 0 || substitute.isCompressed())) {
+					if (!regex.isCompressed()) {
+						return this;
+					}
 					byte[] newChars = new byte[length];
 					byte toReplace = helpers.getByteFromArrayByIndex(regex.value, 0);
 					byte replacement = (byte)0; // assign dummy value that isn't used
@@ -3210,14 +3213,21 @@ public String replaceAll(String regex, String substitute) {
 						checkLastChar((char)replacement);
 					}
 					int newCharIndex = 0;
+					boolean replaced = false;
 					for (int i = 0; i < length; ++i) {
 						byte current = helpers.getByteFromArrayByIndex(value, i);
 						if (current != toReplace) {
 							helpers.putByteInArrayByIndex(newChars, newCharIndex++, current);
-						} else if (substituteLength == 1) {
-							helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement);
+						} else {
+							replaced = true;
+							if (substituteLength == 1) {
+								helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement);
+							}
 						}
 					}
+					if (!replaced) {
+						return this;
+					}
 					return new String(newChars, 0, newCharIndex, true);
 				} else if (!COMPACT_STRINGS || !isCompressed()) {
 					byte[] newChars = StringUTF16.newBytesFor(length);
@@ -3228,14 +3238,21 @@ public String replaceAll(String regex, String substitute) {
 						checkLastChar(replacement);
 					}
 					int newCharIndex = 0;
+					boolean replaced = false;
 					for (int i = 0; i < length; ++i) {
 						char current = helpers.getCharFromArrayByIndex(value, i);
 						if (current != toReplace) {
 							helpers.putCharInArrayByIndex(newChars, newCharIndex++, current);
-						} else if (substituteLength == 1) {
-							helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement);
+						} else {
+							replaced = true;
+							if (substituteLength == 1) {
+								helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement);
+							}
 						}
 					}
+					if (!replaced) {
+						return this;
+					}
 					if (replacement > 255) {
 						// If the original String isn't compressed and the replacement character isn't Latin1, the result is uncompressed.
 						return new String(newChars, UTF16);
@@ -7601,6 +7618,9 @@ public String replaceAll(String regex, String substitute) {
 			int length = lengthInternal();
 			if (substituteLength < 2) {
 				if (COMPACT_STRINGS && isCompressed() && (substituteLength == 0 || substitute.isCompressed())) {
+					if (!regex.isCompressed()) {
+						return this;
+					}
 					char[] newChars = new char[(length + 1) >>> 1];
 					byte toReplace = helpers.getByteFromArrayByIndex(regex.value, 0);
 					byte replacement = (byte)-1;  // assign dummy value that will never be used
@@ -7609,14 +7629,21 @@ public String replaceAll(String regex, String substitute) {
 						checkLastChar((char)replacement);
 					}
 					int newCharIndex = 0;
+					boolean replaced = false;
 					for (int i = 0; i < length; ++i) {
 						byte current = helpers.getByteFromArrayByIndex(value, i);
 						if (current != toReplace) {
 							helpers.putByteInArrayByIndex(newChars, newCharIndex++, current);
-						} else if (substituteLength == 1) {
-							helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement);
+						} else {
+							replaced = true;
+							if (substituteLength == 1) {
+								helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement);
+							}
 						}
 					}
+					if (!replaced) {
+						return this;
+					}
 					return new String(newChars, 0, newCharIndex, true);
 				} else if (!COMPACT_STRINGS || !isCompressed()) {
 					char[] newChars = new char[length];
@@ -7627,14 +7654,21 @@ public String replaceAll(String regex, String substitute) {
 						checkLastChar(replacement);
 					}
 					int newCharIndex = 0;
+					boolean replaced = false;
 					for (int i = 0; i < length; ++i) {
 						char current = helpers.getCharFromArrayByIndex(value, i);
 						if (current != toReplace) {
 							helpers.putCharInArrayByIndex(newChars, newCharIndex++, current);
-						} else if (substituteLength == 1) {
-							helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement);
+						} else {
+							replaced = true;
+							if (substituteLength == 1) {
+								helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement);
+							}
 						}
 					}
+					if (!replaced) {
+						return this;
+					}
 					return new String(newChars, 0, newCharIndex, false);
 				}
 			}

diff --git a/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java b/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java
@@ -1860,6 +1860,19 @@ public void test_join2() {
 		}
 	}
 
+	/**
+	 * @tests java.lang.String#replaceAll(String, String)
+	 */
+	@Test
+	public void test_replaceAll() {
+		String replace1 = "1a2a3a\u0011";
+		String result1 = replace1.replaceAll("\u1161", "[");
+		AssertJUnit.assertSame("replaceAll() compact result should be identical", replace1, result1);
+		String replace2 = "1a2b3c\u1161";
+		String result2 = replace2.replaceAll("\u1162", "\u1234");
+		AssertJUnit.assertSame("replaceAll() non-compact result should be identical", replace2, result2);
+	}
+
 	/**
 	 * @tests java.lang.String#replaceAll(String, String)
 	 */
@@ -1872,6 +1885,7 @@ public void test_replaceAll_last_char_dollarsign() {
 			// expected
 		}
 	}
+
 	/**
 	 * @tests java.lang.String#replaceAll(String, String)
 	 */