Re: [PATCH] AbstractStringBuilder.append()

2018-07-18 Thread Isaac Levy
I think adding a dedicated method would help clarify and encourage performant code. For example, I sped up the snippet below after seeing that StringBuilder.append() has a fast path when the arg is another StringBuilder, which isn't clear from the javadoc. public class ScalaSB implements

Re: [PATCH] AbstractStringBuilder.append()

2018-06-29 Thread Paul Sandoz
> On Jun 29, 2018, at 2:50 PM, Jonathan Gibbons > wrote: > > In that specific snippet, I would expect the compiler (javac) to fold the > constants together. > Yes, and behold the use of indy string concat :-) Classfile /Users/sandoz/Projects/jdk/jdk-test/target/classes/Test.class Last

Re: [PATCH] AbstractStringBuilder.append()

2018-06-29 Thread Jonathan Gibbons
In that specific snippet, I would expect the compiler (javac) to fold the constants together. -- Jon On 06/29/2018 01:02 PM, Isaac Levy wrote: Would this snippet end up merging two StringBuilders? String x = "bar" + "foo"; x += "baz" + "biz"; I had trouble reading stringopts.cpp. I was

Re: [PATCH] AbstractStringBuilder.append()

2018-06-29 Thread Isaac Levy
Would this snippet end up merging two StringBuilders? String x = "bar" + "foo"; x += "baz" + "biz"; I had trouble reading stringopts.cpp. I was thinking this append might be common due to implicit StringBuilder creation. Isaac On Fri, Jun 29, 2018 at 12:48 PM, Paul Sandoz wrote: > Hard to

Re: [PATCH] AbstractStringBuilder.append()

2018-06-29 Thread Paul Sandoz
Hard to reconstruct the culture memory around this. I suspect such a new method was not considered necessary when StringBuilder was added to Java 1.5 given the CharSequence accepting method, and performance was not considered a concern, and I doubt it's a concern now. (I don’t think there is

Re: [PATCH] AbstractStringBuilder.append()

2018-06-28 Thread Martin Buchholz
On Thu, Jun 28, 2018 at 11:59 AM, Isaac Levy wrote: > And can't remove append(StringBuffer) because of binary compatibility? > That seems right. --- Also, the JIT can optijmize away any instanceof checks after inining when it sees append(stringBuilder). And any optimizations here are far

Re: [PATCH] AbstractStringBuilder.append()

2018-06-28 Thread Isaac Levy
And can't remove append(StringBuffer) because of binary compatibility? Isaac On Thu, Jun 28, 2018 at 1:22 AM, Martin Buchholz wrote: > I'm fairly sure the append(StringBuilder) overloads were left out > intentionally. > > On Wed, Jun 27, 2018 at 8:57 PM, Isaac Levy wrote: >> >>

Re: [PATCH] AbstractStringBuilder.append()

2018-06-27 Thread Martin Buchholz
I'm fairly sure the append(StringBuilder) overloads were left out intentionally. On Wed, Jun 27, 2018 at 8:57 PM, Isaac Levy wrote: > AbstractStringBuilder already has append(). This patch > adds append(). > > The new method improves parity between the two classes. In addition, > combining

[PATCH] AbstractStringBuilder.append()

2018-06-27 Thread Isaac Levy
AbstractStringBuilder already has append(). This patch adds append(). The new method improves parity between the two classes. In addition, combining StringBuilders is presumably common. append() has a couple insteadof checks, which this new method skips. -Isaac diff --git