-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkg/log: Avoid unnecessary mutex lock/unlock #19438
Comments
As someone not familiar with neither the |
I was just curious how this would hurt log performance, and whether it is insignificant for most cases. |
I'm going to close this because I don't think there is anything to do here. We can't simply remove the lock, or we would get race conditions if one goroutine logs while a different goroutine calls |
I think the race conditions caused by removing the lock would not cause any bug or un-expected in real scenarios. Thanks for the discussion anyway. |
@ShevaXu making the log package not thread-safe would break backwards compatibility, so it's not an option. In any case, if what you need is performance in your log calls, you'd probably want to write custom code instead of using the standard logger. |
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?go1.8
What operating system and processor architecture are you using (
go env
)?darwin/amd64 & linux/amd64
What did you do?
I found
func (l *Logger) Output(calldepth int, s string) error
inpkg/log
had a strange lock-then-unlock logic for the expensiveruntime.Caller(calldepth)
.I tried to remove this lock-unlock logic and benchmark it, which results in ~20% improvement for concurrent logging with flags
Lshortfile
orLlongfile
(tested on both darwin&linux).Benchmark suit here: https://github.com/ShevaXu/bench-log
The text was updated successfully, but these errors were encountered: