8058779: Faster implementation of String.replace(CharSequence, CharSequence) (original) (raw)

Ivan Gerasimov ivan.gerasimov at oracle.com
Tue May 26 17:39:22 UTC 2015

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.shen at 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.reinhold at 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

More information about the core-libs-dev mailing list