On Mon, 1 Aug 2022 17:08:12 GMT, Сергей Цыпанов <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
>
> src/java.base/share/classes/java/lang/StackTraceElement.java line 366:
> 
>> 364:         if (!dropClassLoaderName() && classLoaderName != null && 
>> !classLoaderName.isEmpty()) {
>> 365:             prefixClassLoader = true;
>> 366:             length += classLoaderName.length() + 1 /* '/' */;
> 
> Do we still need this comment `/* '/' */`?

happy to remove these if desired

> src/java.base/share/classes/java/lang/StackTraceElement.java line 400:
> 
>> 398:                 dest.append(fileName)
>> 399:                         .append(':')
>> 400:                         .append(Integer.toString(lineNumber))
> 
> I think `Integer.toString(lineNumber)` is redundant here, you can pass `int` 
> directly

`Appendable` does not currently have an `append(int)` method on the interface, 
though `StringBuilder` does. We could add it to `Appendable` via default method 
if so desired:


default Appendable append(int value) {
    return append(Integer.toString(value));
}

-------------

PR: https://git.openjdk.org/jdk/pull/9665

Reply via email to