Hi Sherman, Thanks for clarifying. Looks like exceptions are caused by invalid format string. I wouldn't expect most programs to be catching this and preserving their buffer, but dunno.
How much does it affect perf? Well it depends on use case, a jmh of replaceAll with a length 200 string of digits and regex "(\w)" shows about 30% speedup. [info] Benchmark Mode Cnt Score Error Units [info] origM avgt 10 11.669 ± 0.211 us/op [info] newM avgt 10 8.926 ± 0.095 us/op Isaac On Tue, Apr 24, 2018 at 12:53 PM, Xueming Shen <xueming.s...@oracle.com> wrote: > Hi Isaac, > > I actually meant to say "we are not supposed to output the partial text into > the output buffer in case of an exception". It has nothing to do with the > changeset you cited. This has been the behavior since day one/JDK1.4, > though it is not specified explicitly in the API doc. The newly added > StringBuilder variant simply follows this behavior. If it's really desired > it > is kinda doable to save that StringBuilder without the "incompatible" > behavior > change but just wonder if it is really worth the effort. > > Thanks, > Sherman > > > On 4/24/18, 9:11 AM, Isaac Levy wrote: >> >> (moving this to a separate discussion) >> >> >> --- a/src/java.base/share/classes/java/util/regex/Matcher.java >> +++ b/src/java.base/share/classes/java/util/regex/Matcher.java >> @@ -993,13 +993,11 @@ >> public Matcher appendReplacement(StringBuilder sb, String >> replacement) { >> // If no match, return error >> if (first< 0) >> throw new IllegalStateException("No match available"); >> - StringBuilder result = new StringBuilder(); >> - appendExpandedReplacement(replacement, result); >> // Append the intervening text >> sb.append(text, lastAppendPosition, first); >> // Append the match substitution >> + appendExpandedReplacement(replacement, sb); >> - sb.append(result); >> >> >> >> On Mon, Apr 23, 2018 at 5:05 PM Xueming Shen<xueming.s...@oracle.com> >> wrote: >>> >>> >>> I would assume in case of an exception thrown from >>> appendExpandedReplacement() we don't >>> want "text" to be pushed into the "sb". >>> >>> -sherman >> >> >> Perhaps. Though the behavior under exception is undefined and this >> function is probably primarily used though the replaceAll API, which >> wouldn’t return the intermediate sb under the case of exception. >> >> My reading of the blame was the temp StringBuilder was an artifact of >> the api previously using StringBuffer externally. See >> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/763c564451b3 > >