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

[Tomasz Kowalczewski]

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.gerasimov at 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.shen at oracle.com> <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
>
>
>
>
>


-- 
Tomasz Kowalczewski