[GitHub] logging-log4j2 pull request #190: [LOG4J2-2373] Reduce unnecessary char move...
Github user asfgit closed the pull request at: https://github.com/apache/logging-log4j2/pull/190 ---
[GitHub] logging-log4j2 pull request #190: [LOG4J2-2373] Reduce unnecessary char move...
Github user cakofony commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/190#discussion_r201844005
--- Diff:
log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java ---
@@ -169,53 +169,85 @@ public static void trimToMaxSize(final StringBuilder
stringBuilder, final int ma
}
public static void escapeJson(final StringBuilder toAppendTo, final
int start) {
-for (int i = toAppendTo.length() - 1; i >= start; i--) { //
backwards: length may change
+int escapeCount = 0;
+for (int i = start; i < toAppendTo.length(); i++) {
final char c = toAppendTo.charAt(i);
switch (c) {
case '\b':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'b');
+case '\t':
+case '\f':
+case '\n':
+case '\r':
+case '"':
+case '\\':
+escapeCount++;
+break;
+default:
+if (Character.isISOControl(c)) {
+escapeCount += 5;
+}
+}
+}
+
+int lastChar = toAppendTo.length() - 1;
+toAppendTo.setLength(toAppendTo.length() + escapeCount);
+int lastPos = toAppendTo.length() - 1;
+
+for (int i = lastChar; i >= start; i--) {
+final char c = toAppendTo.charAt(i);
+switch (c) {
+case '\b':
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'b');
break;
case '\t':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 't');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 't');
break;
case '\f':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'f');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'f');
break;
case '\n':
-// Json string newline character must be encoded as
literal "\n"
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'n');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'n');
break;
case '\r':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'r');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'r');
break;
case '"':
+lastPos = escapeAndDecrement(toAppendTo, lastPos, '"');
+break;
case '\\':
-// only " and \ need to be escaped; other escapes are
optional
-toAppendTo.insert(i, '\\');
+lastPos = escapeAndDecrement(toAppendTo, lastPos,
'\\');
break;
default:
if (Character.isISOControl(c)) {
-// all iso control characters are in U+00xx
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, "u");
-toAppendTo.setCharAt(i + 4,
Chars.getUpperCaseHex((c & 0xF0) >> 4));
-toAppendTo.setCharAt(i + 5,
Chars.getUpperCaseHex(c & 0xF));
+// all iso control characters are in U+00xx, JSON
output format is \u00XX
+toAppendTo.setCharAt(lastPos - 5, '\\');
+toAppendTo.setCharAt(lastPos - 4, 'u');
+toAppendTo.setCharAt(lastPos - 3, '0');
+toAppendTo.setCharAt(lastPos - 2, '0');
+toAppendTo.setCharAt(lastPos - 1,
Chars.getUpperCaseHex((c & 0xF0) >> 4));
+toAppendTo.setCharAt(lastPos,
Chars.getUpperCaseHex(c & 0xF));
+lastPos -= 6;
+} else {
+toAppendTo.setCharAt(lastPos, c);
+lastPos--;
}
}
}
}
+private static int escapeAndDecrement(StringBuilder toAppendTo, int
lastPos, char b) {
--- End diff --
I might inline this
```java
toAppendTo.setCharAt(lastPos--, );
toAppendTo.setCharAt(lastPos--, '\\');
```
---
[GitHub] logging-log4j2 pull request #190: [LOG4J2-2373] Reduce unnecessary char move...
Github user cakofony commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/190#discussion_r201843300
--- Diff:
log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java ---
@@ -169,53 +169,85 @@ public static void trimToMaxSize(final StringBuilder
stringBuilder, final int ma
}
public static void escapeJson(final StringBuilder toAppendTo, final
int start) {
-for (int i = toAppendTo.length() - 1; i >= start; i--) { //
backwards: length may change
+int escapeCount = 0;
+for (int i = start; i < toAppendTo.length(); i++) {
final char c = toAppendTo.charAt(i);
switch (c) {
case '\b':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'b');
+case '\t':
+case '\f':
+case '\n':
+case '\r':
+case '"':
+case '\\':
+escapeCount++;
+break;
+default:
+if (Character.isISOControl(c)) {
+escapeCount += 5;
+}
+}
+}
+
+int lastChar = toAppendTo.length() - 1;
+toAppendTo.setLength(toAppendTo.length() + escapeCount);
+int lastPos = toAppendTo.length() - 1;
+
+for (int i = lastChar; i >= start; i--) {
--- End diff --
I think we can change the condition from `i >= start` to `lastPos > i`. On
the fast path there's nothing to escape, so this loop should be a no-op. This
allows us to stop iterating once we've escaped the last unescaped character.
---
[GitHub] logging-log4j2 pull request #190: [LOG4J2-2373] Reduce unnecessary char move...
Github user cakofony commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/190#discussion_r201844357
--- Diff:
log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java ---
@@ -169,53 +169,85 @@ public static void trimToMaxSize(final StringBuilder
stringBuilder, final int ma
}
public static void escapeJson(final StringBuilder toAppendTo, final
int start) {
-for (int i = toAppendTo.length() - 1; i >= start; i--) { //
backwards: length may change
+int escapeCount = 0;
+for (int i = start; i < toAppendTo.length(); i++) {
final char c = toAppendTo.charAt(i);
switch (c) {
case '\b':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'b');
+case '\t':
+case '\f':
+case '\n':
+case '\r':
+case '"':
+case '\\':
+escapeCount++;
+break;
+default:
+if (Character.isISOControl(c)) {
+escapeCount += 5;
+}
+}
+}
+
+int lastChar = toAppendTo.length() - 1;
+toAppendTo.setLength(toAppendTo.length() + escapeCount);
+int lastPos = toAppendTo.length() - 1;
+
+for (int i = lastChar; i >= start; i--) {
+final char c = toAppendTo.charAt(i);
+switch (c) {
+case '\b':
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'b');
break;
case '\t':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 't');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 't');
break;
case '\f':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'f');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'f');
break;
case '\n':
-// Json string newline character must be encoded as
literal "\n"
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'n');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'n');
break;
case '\r':
-toAppendTo.setCharAt(i, '\\');
-toAppendTo.insert(i + 1, 'r');
+lastPos = escapeAndDecrement(toAppendTo, lastPos, 'r');
break;
case '"':
+lastPos = escapeAndDecrement(toAppendTo, lastPos, '"');
+break;
case '\\':
-// only " and \ need to be escaped; other escapes are
optional
-toAppendTo.insert(i, '\\');
+lastPos = escapeAndDecrement(toAppendTo, lastPos,
'\\');
--- End diff --
Might as well combine `\` and `"`, in both cases the char `c` is the
escaped character
---
