RFR: 8276904: Optional.toString() is unnecessarily expensive
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&pr=6337&range=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 fetch https://git.openjdk.java.net/jdk pull/6337/head:pull/6337 PR: https://git.openjdk.java.net/jdk/pull/6337
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&pr=6337&range=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
On Wed, 10 Nov 2021 17:44:04 GMT, Eamonn McManus <emcmanus@openjdk.org> 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
On Wed, 10 Nov 2021 17:44:04 GMT, Eamonn McManus <emcmanus@openjdk.org> 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
participants (4)
-
Brian Goetz
-
Eamonn McManus
-
Roger Riggs
-
Stuart Marks