RFR 8222806: Inefficient String.replace in PathFileObject.toBinaryName

[Liam Miller-Cushon]

This does seem like a small trade-off between
readability/conciseness/maintainability and performance. Jon, do you have
suggestions for how to evaluate that trade-off?

re: doing this in String.replace, I suspect that the usage in
PathFileObject is unusually biased towards single-character arguments,
since we'd only see longer arguments with a non-default filesystem with an
unusual path separator. In the general case, String.replace is probably
more likely to see longer arguments, and adding the additional logic and
branches there may be less of a slam-dunk.

On Mon, Apr 22, 2019 at 11:08 AM Ron Shapiro <ronshapiro at google.com> wrote:

> I mentioned in the bug that it had a 300ms improvement for a 30s build.
>
> I think it would be reasonable in String.replace() if it was common enough
> to be worth checking the lengths. I don't know what the benchmarking
> considerations are there.
>
> On Mon, Apr 22, 2019 at 2:03 PM Jonathan Gibbons <
> jonathan.gibbons at oracle.com> wrote:
>
>> That seems like micro-optimization.  Do you have numbers to back it up?
>>
>> Should this sort of optimization be done in String.replace?
>>
>> -- Jon
>> On 4/22/19 8:51 AM, Ron Shapiro wrote:
>>
>> Hi,
>>
>> Please review this small change to improve the performance of
>> PathFileObject.toBinaryName:
>>
>> webrev: http://cr.openjdk.java.net/~ronsh/8222806/webrev.00/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8222806
>>
>> Thanks!
>> Ron
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190423/8de211f8/attachment.html>