My goal was to optimize for readability rather than performance. No strong preference though. Reverted.
On Mon, Jul 16, 2018 at 10:44 AM, Gary Gregory <[email protected]> wrote: > I am -1 to this and this kind of change. Please revert. > > The JVM will in-line small methods for you. > > We have actually had work in the other direction: Making sure heavily used > methods a tiny so that the JVM will in-line them. I think the limit is.. 35 > bytes? > > Gary > > On Mon, Jul 16, 2018 at 8:39 AM <[email protected]> wrote: > >> Repository: logging-log4j2 >> Updated Branches: >> refs/heads/master 1d474bf63 -> ac0bf4bbe >> >> >> Inline private StringBuilders.escapeAndDecrement >> >> >> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >> Commit: >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/ac0bf4bb >> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ac0bf4bb >> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ac0bf4bb >> >> Branch: refs/heads/master >> Commit: ac0bf4bbe85cd4a07af195b3b001717a72888794 >> Parents: 1d474bf >> Author: Carter Kozak <[email protected]> >> Authored: Mon Jul 16 07:10:13 2018 -0400 >> Committer: Carter Kozak <[email protected]> >> Committed: Mon Jul 16 07:10:13 2018 -0400 >> >> ---------------------------------------------------------------------- >> .../logging/log4j/util/StringBuilders.java | 28 +++++++++----------- >> 1 file changed, 13 insertions(+), 15 deletions(-) >> ---------------------------------------------------------------------- >> >> >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ac0bf4bb/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java >> ---------------------------------------------------------------------- >> diff --git >> a/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java >> b/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java >> index 2a83b24..505abf1 100644 >> --- >> a/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java >> +++ >> b/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java >> @@ -197,28 +197,33 @@ public final class StringBuilders { >> final char c = toAppendTo.charAt(i); >> switch (c) { >> case '\b': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, >> 'b'); >> + toAppendTo.setCharAt(lastPos--, 'b'); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> - >> case '\t': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, >> 't'); >> + toAppendTo.setCharAt(lastPos--, 't'); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> >> case '\f': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, >> 'f'); >> + toAppendTo.setCharAt(lastPos--, 'f'); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> >> case '\n': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, >> 'n'); >> + toAppendTo.setCharAt(lastPos--, 'n'); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> >> case '\r': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, >> 'r'); >> + toAppendTo.setCharAt(lastPos--, 'r'); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> >> case '"': >> case '\\': >> - lastPos = escapeAndDecrement(toAppendTo, lastPos, c); >> + toAppendTo.setCharAt(lastPos--, c); >> + toAppendTo.setCharAt(lastPos--, '\\'); >> break; >> >> default: >> @@ -231,19 +236,12 @@ public final class StringBuilders { >> toAppendTo.setCharAt(lastPos--, 'u'); >> toAppendTo.setCharAt(lastPos--, '\\'); >> } else { >> - toAppendTo.setCharAt(lastPos, c); >> - lastPos--; >> + toAppendTo.setCharAt(lastPos--, c); >> } >> } >> } >> } >> >> - private static int escapeAndDecrement(StringBuilder toAppendTo, int >> lastPos, char c) { >> - toAppendTo.setCharAt(lastPos--, c); >> - toAppendTo.setCharAt(lastPos--, '\\'); >> - return lastPos; >> - } >> - >> public static void escapeXml(final StringBuilder toAppendTo, final >> int start) { >> for (int i = toAppendTo.length() - 1; i >= start; i--) { // >> backwards: length may change >> final char c = toAppendTo.charAt(i); >> >>
