Skip to content
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

Jump to bottom

sync: speed up Mutex.Unlock #17684

Closed
johnrs opened this issue Oct 31, 2016 · 3 comments
Closed

sync: speed up Mutex.Unlock #17684

johnrs opened this issue Oct 31, 2016 · 3 comments
Labels
FrozenDueToAge
Milestone
Go1.8Maybe

Comments

@johnrs
Copy link

johnrs commented Oct 31, 2016

Proposal

I was looking at the Unlock method in mutex.go and I noticed something about line #110:

  if (new+mutexLocked)&mutexLocked == 0 {

It can be simplified to:

  if new&mutexLocked != 0 {

This does not change the functionality of Unlock in any way, other than to speed it up slightly.

Benefit

A quick and dirty test on my machine showed about a 5% run time improvement when doing a Lock/Unlock in a tight loop.

Before: 58.8-59.1 ns
After: 55.6-56.0 ns

Note: I did this in Jan 2016. I discussed it with Dmitry and he recommended submitting it. Then I misplaced it and didn't notice it till now. Sorry for the delay!
@mvdan
Copy link
Member

mvdan commented Oct 31, 2016

@johnrs you should use a benchmark with https://github.com/rsc/benchstat, that will give more useful results as to whether this change speeds up the function or not.

Also, I think you should open a CL instead of an issue for this. This issue is pretty much just a manual patch submission.

@josharian josharian changed the title sync.mutex.Unlock: Slight speedup sync: speed up Mutex.Unlock Oct 31, 2016
@josharian
Copy link
Contributor

Please feel free to send a CL. Thanks. Today is the last day before the (three month) freeze, though.

@josharian josharian added this to the Go1.8Maybe milestone Oct 31, 2016
@bradfitz
Copy link
Contributor

I benchmarked this with benchstat and don't see any difference:

name                old time/op  new time/op  delta
MutexUncontended-4  8.37ns ± 0%  8.37ns ± 0%  -0.11%  (p=0.001 n=20+17)

I assume SSA is helping. Please reopen if you find otherwise, or just send a CL in February (or today, soon).

@golang golang locked and limited conversation to collaborators Oct 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
5 participants
@bradfitz @josharian @mvdan @johnrs @gopherbot