On Sat, 6 Aug 2022 14:55:17 GMT, David Schlosnagle <d...@openjdk.org> wrote:
>> I would like to contribute an optimized version of >> `StackTraceElement#toString()` that uses a single StringBuilder throughout >> creation to avoid intermediate `String` allocations. >> `StackTraceElement#toString()` is used in a number of JDK code paths >> including `Throwable#printStackTrace()`, as well as many JDK consumers may >> transform `StackTraceElement` `toString()` in logging frameworks capturing >> throwables and exceptions, and diagnostics performing dumps. >> >> Given this usage and some observed JFR profiles from production services, >> I'd like to reduce the intermediate allocations to reduce CPU pressure in >> these circumstances. I have added a couple benchmarks for a sample >> `Throwable#printStackTrace()` converted to String via `StringWriter` and >> individual `StackTraceElement` `toString`. The former shows ~15% >> improvement, while the latter shows ~40% improvement. >> >> Before >> >> Benchmark Mode Cnt Score Error >> Units >> StackTraceElementBench.printStackTrace avgt 15 167147.066 ± 4260.521 >> ns/op >> StackTraceElementBench.toString avgt 15 132.781 ± 2.095 >> ns/op >> >> >> After >> >> Benchmark Mode Cnt Score Error >> Units >> StackTraceElementBench.printStackTrace avgt 15 142909.133 ± 2290.720 >> ns/op >> StackTraceElementBench.toString avgt 15 78.939 ± 0.469 >> ns/op > > David Schlosnagle has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 11 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into ds/StackTraceElement > - Mark StackTraceElement SerialTest for bug 8291641 > - Revert "Mark StackTraceElement SerialTest for bug 8291641" > > This reverts commit e7b04faafb026e61829c81c75121e2d3be6644d9. > - Mark StackTraceElement SerialTest for bug 8291641 > - Inline max Integer.stringSize > - Estimate length > - Address comments > - Precompute StackTraceElement toString length > - Merge remote-tracking branch 'origin/master' into ds/StackTraceElement > - Optimize StackTraceElement.toString() > - ... and 1 more: https://git.openjdk.org/jdk/compare/f9948294...c9ae3897 Looks good to me. Minor suggestions. src/java.base/share/classes/java/lang/StackTraceElement.java line 359: > 357: @Override > 358: public String toString() { > 359: int estimatedLength = > Objects.requireNonNullElse(classLoaderName, "").length() + 1 Nit: suggest to refactor this and define a private static `length(String s)` method. src/java.base/share/classes/java/lang/StackTraceElement.java line 364: > 362: + declaringClass.length() + 1 > 363: + methodName.length() + 1 > 364: + Math.max("Unknown Source".length(), > Objects.requireNonNullElse(fileName, "").length()) + 1 I suggest to define a static variable = "Unknown Source" be used here and line 387. Same for "Native Method" which can also be accounted for. ------------- PR: https://git.openjdk.org/jdk/pull/9665
