I know it was like that before :) I asked similar question about fixing String.contains to not call toString on it's CharSequence argument ( http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-March/032394.html). Responses were generally encouraging.
Regards, Tomasz On Tue, May 26, 2015 at 7:39 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote: > But the current implementation converts the argument to String in the > first place too: > > return Pattern.compile(*target.toString()*, > Pattern.LITERAL).matcher( > this).replaceAll(Matcher.quoteReplacement( > *replacement.toString()*)); > > So I didn't introduce this conversion :-) > > When browsing the source code, looking for usages of this method, I only > saw Strings passed as the parameters. > Thus, I guess it is reasonable to have the variant which works best in > this case. > > Sincerely yours, > Ivan > > > On 26.05.2015 20:26, Tomasz Kowalczewski wrote: > > I second that. API accepts CharSequence for a reason. The benchmarks might > look a little different if you pass StringBuilder in a call to replace. It > looks like removing toString() call on CharSequences will be easy with one > drawback of loosing the benefit of System.arraycopy call. Note that > String.indexOf requires String argument but that can also be fixed. > > -- > Tomasz > > On Tue, May 26, 2015 at 7:17 PM, Xueming Shen <xueming.s...@oracle.com> > <xueming.s...@oracle.com> > wrote: > > > On 5/26/15 10:08 AM, Ivan Gerasimov wrote: > > > Thank you Roger for looking at this! > > On 26.05.2015 19:40, Roger Riggs wrote: > > > Hi Ivan, > > Did you consider doing the optimization inside of > Pattern.compile/replaceAll. > That would have a broader impact and benefit. > > > > In Pattern.compile the flag LITERAL can be combined with several other > flags, which makes things more complex. > However, in String.replace we've got a special case, which can be > optimized in a straight-forward (though a little bit verbose) way. > > > It probably makes big difference to access those characters via > CharSequence.charAt() > and the direct internal char[] access. Though avoiding allocating those > objects created > by Pattern/Matcher/SB definitely helps. > > -Sherman > > > > > Sincerely yours, > Ivan > > Roger > > > On 5/26/2015 12:36 PM, Ivan Gerasimov wrote: > > > Thank you Mark for looking at this! > > On 26.05.2015 18:39, mark.reinh...@oracle.com wrote: > > > Your micro-benchmark improvements are significant, but do you have > evidence to suggest that the performance of this method is actually > critical to real applications? > > In other words, is the added code complexity really worth it? > > > The enhancement request contains a few links to the discussions of this > method's performance at open forums. > The most frequent suggestion is to use alternatives from 3rd party > libraries. > > That should prove the benefits of this fix -- by improving performance > we can keep some users from moving away from JDK :) > > grep shows that langtools would also benefit from making replace() > faster. > > Sincerely yours, > Ivan > > - Mark > > > > > -- Tomasz Kowalczewski
