Re: RFR: 8276904: Optional.toString() is unnecessarily expensive

2021-12-02 Thread Roger Riggs
On Wed, 10 Nov 2021 17:44:04 GMT, Eamonn McManus  wrote:

> Use string concatenation instead of `String.format`.

Looks ok on the face of it. I would be useful to see the performance of the 
improvement.

-

PR: https://git.openjdk.java.net/jdk/pull/6337


Re: RFR: 8276904: Optional.toString() is unnecessarily expensive

2021-11-22 Thread Stuart Marks
On Wed, 10 Nov 2021 17:44:04 GMT, Eamonn McManus  wrote:

> Use string concatenation instead of `String.format`.

I talked to Brian about this a bit. Changing `format` to concatenation is 
probably ok this case for two reasons: 1) it's the simplest possible such 
transformation, and as such it's not a very good example for the templating 
stuff; and 2) presumably you bothered filing the bug and the PR because you 
believe the performance improvement would be noticeable in actual systems.

I note that some Amazon folks had filed 
[JDK-8275190](https://bugs.openjdk.java.net/browse/JDK-8275190) for what is 
essentially the same issue. (I closed it as a dupe.) They did include some 
performance numbers, but not the actual code they measured, so without guessing 
it's hard to say what they actually measured. Could you run some numbers and 
post them, and can you share some estimates of the impact it might have on your 
systems?

The reason I'm interested in some data here is that this sort of change is 
worth doing only if it has a real, measurable impact. In turn that implies that 
you're probably using `Optional::toString` a lot. That's an important 
criterion; we don't want to do a wholesale replacement of `format` with 
concatenation in the JDK, based on some micro benchmarks, that end up garbaging 
up the JDK code base while not actually improving any real systems.

-

PR: https://git.openjdk.java.net/jdk/pull/6337


Re: RFR: 8276904: Optional.toString() is unnecessarily expensive

2021-11-10 Thread Brian Goetz
I would suggest that we hold this patch until the string interpolation 
JEP lands.  It will offer both better readability *and* better 
performance, and probably better to have one round of "replace all the 
toString implementations" than two?


On 11/10/2021 1:04 PM, Eamonn McManus wrote:

Use string concatenation instead of `String.format`.

-

Commit messages:
  - 8276904: Optional.toString() is unnecessarily expensive

Changes:https://git.openjdk.java.net/jdk/pull/6337/files
  Webrev:https://webrevs.openjdk.java.net/?repo=jdk=6337=00
   Issue:https://bugs.openjdk.java.net/browse/JDK-8276904
   Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod
   Patch:https://git.openjdk.java.net/jdk/pull/6337.diff
   Fetch: git fetchhttps://git.openjdk.java.net/jdk  pull/6337/head:pull/6337

PR:https://git.openjdk.java.net/jdk/pull/6337