Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

PROPOSAL: Use strings.Builder for stack trace formatting #149

Open
cstockton opened this issue Feb 12, 2018 · 3 comments
Open

PROPOSAL: Use strings.Builder for stack trace formatting #149

cstockton opened this issue Feb 12, 2018 · 3 comments

Comments

@cstockton
Copy link

cstockton commented Feb 12, 2018

Hello, I'm not sure how this project would feel about a performance inspired change but figured I would at least present my findings since the gains are pretty significant. I was curious what was causing so many allocations when a specific code path failed, around 150 or so total and found the culprit to be in the stack trace printing. By adding a format function with a signature of func(b *strutil.Builder, s fmt.State, verb rune) I was able to lower it down to 7 by iterating the stack first for a good length estimate and calling Grow. The exact numbers:

BenchmarkStackObtain/er/lazy-24           	  500000	      3035 ns/op	     288 B/op	       1 allocs/op
BenchmarkStackObtain/pkg/errors-24        	  500000	      3412 ns/op	     320 B/op	       3 allocs/op
BenchmarkStackPrint/1_Times/er/lazy-24    	  100000	     24778 ns/op	    7244 B/op	       7 allocs/op
BenchmarkStackPrint/1_Times/pkg/errors-24 	   30000	     56817 ns/op	    4057 B/op	     130 allocs/op
BenchmarkStackPrint/6_Times/er/lazy-24    	   10000	    129898 ns/op	   42020 B/op	      37 allocs/op
BenchmarkStackPrint/6_Times/pkg/errors-24 	    5000	    323686 ns/op	   22739 B/op	     765 allocs/op

Of course printing 6 stack traces will never happen, it's just to illustrate how quickly it can add up. Here is an example implementation, this project probably wouldn't want the Itoa (much better would be adding a simple Itoa to strings.Builder)- I was just toying around at that point to see how many more allocs I could spare, but that specific piece only saves at most the stack frame count.

@davecheney
Copy link
Member

Not til 1.9 is deprecated, which won't be til the end of the year i'd say.

@cstockton
Copy link
Author

Would you accept a bytes.Buffer pull request that did the same thing? Would just allocate a backing ahead of time similar to string builder and string() the byte slice.

@davecheney
Copy link
Member

davecheney commented Feb 12, 2018 via email

cstockton added a commit to cstockton/errors that referenced this issue Feb 17, 2018
cstockton added a commit to cstockton/errors that referenced this issue Feb 17, 2018
cstockton added a commit to cstockton/errors that referenced this issue Feb 17, 2018
lysu pushed a commit to pingcap/errors that referenced this issue Sep 11, 2018
gregwebs added a commit to pingcap/errors that referenced this issue Sep 11, 2018
reduce allocations when printing stack traces (pkg#149)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants