go-stack / stack

Package stack implements utilities to capture, manipulate, and format call stacks.
MIT License
397 stars 34 forks source link

Evaluate using runtime.CallersFrames internally. #10

Closed ChrisHines closed 7 years ago

ChrisHines commented 7 years ago

runtime.CallersFrames was added in Go 1.7. Using it could clean up some of the internals of this package and eliminate the need for the findSigpanic function.

dkushner commented 7 years ago

So, having submitted a PR for this, I can say it definitely does clean up a lot of the internal stack navigation logic but it seems to come with a decently heavy performance hit. Granted, stack generation is not really a thing that most applications are going to be doing at a high frequency but for things like logging frameworks, that may be a bit of a big deal.

There could also simply be inefficiencies in my code, so if anyone can give it a look over, perhaps that will shave off some time. The fact that this method performs far more allocations than the previous method is unavoidable, however.

ChrisHines commented 7 years ago

Thanks for giving this a shot. I'll review your PR soon.

ChrisHines commented 7 years ago

Closed by #19.