RFR 8222806: Inefficient String.replace in PathFileObject.toBinaryName

[Jonathan Gibbons]

I think we should follow up with folk who understand performance trade-offs.

Looking at String.replace, it already computes the length of the 
replacement string, so the default operational cost is just a single int 
comparison, but I agree the code would be more complex there, and that 
would be for the relevant experts to consider.

-- Jon

On 4/23/19 9:14 AM, Liam Miller-Cushon wrote:
> 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 
> <mailto: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 <mailto: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/004a4c9e/attachment.html>