Avoid unnecessary String allocations in AstPrinter #3710
Conversation
Avoid the overhead of allocating unnecessary intermediate String instances in AstPrinter by only appending to the supplied StringBuilder. When running the AstPrinterBenchmark with the gc profiler, this significantly reduces the allocation rate: before: gc.alloc.rate: 4846.634 MB/sec gc.alloc.rate.norm: 86960.001 B/op after: gc.alloc.rate: 2524.634 MB/sec gc.alloc.rate.norm: 10432.000 B/op The benchmarks also show a doubling in throughput.
| StringBuilder sb = new StringBuilder(stringValue.length()); | ||
| escapeJsonStringTo(sb, stringValue); | ||
| return sb.toString(); | ||
| } |
|
Thanks for this PR - it makes a lot of sense to pass in the one StringBuilder - this code was originally ported from graphql-js I think and hence it would have inherited its string appending approach. Out of interest, what hot spot did you have such that you call I ask to try and gauge how likely this is to benefit the wider community |
I'll confess I don't 100% understand the use case, but while looking at performance of an internal app involved in federation, I noticed they had written their own custom printer for a subset of Node types to avoid the overhead of the graphql-java AstPrinter. I thought I'd improve AstPrinter itself so such workarounds wouldn't be necessary. If this change looks good to you all then I'd like to follow up with some API additions to allow passing in an Appendable / StringBuilder to the print methods, which also allows callers to re-use StringBuilder instances. |
|
Thanks so much for contributing these changes. It's cool to see how these improvements matter at scale!
Please do keep the PRs coming! |
Avoid the overhead of allocating unnecessary intermediate String instances in AstPrinter by only appending to the supplied StringBuilder. When running the AstPrinterBenchmark with the gc profiler, this significantly reduces the allocation rate:
before:
gc.alloc.rate: 4846.634 MB/sec
gc.alloc.rate.norm: 86960.001 B/op
after:
gc.alloc.rate: 2524.634 MB/sec
gc.alloc.rate.norm: 10432.000 B/op
The benchmarks also show a doubling in throughput.