Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(248)

Issue 4631075: code review 4631075: sync: improve Mutex to allow successive acquisitions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by dvyukov
Modified:
10 years, 10 months ago
Reviewers:
CC:
rsc, golang-dev
Visibility:
Public.

Description

sync: improve Mutex to allow successive acquisitions This implementation allows a goroutine to do successive acquisitions of a mutex even if there are blocked goroutines. Moreover, it allows a newcomer goroutine to acquire a mutex ahead of blocked goroutines (that is, it does not enforce FIFO). On implementation level it's achieved by separating waiter count and locked flag. Benchmark results on HP Z600 (2 x Xeon E5620, 8 HT cores, 2.40GHz) are as follows (with 4631059 "replace Semacquire/Semrelease implementation" patch applied): benchmark old ns/op new ns/op delta sync_test.BenchmarkMutexUncontended 24.10 25.40 +5.39% sync_test.BenchmarkMutexUncontended-2 12.00 13.00 +8.33% sync_test.BenchmarkMutexUncontended-4 6.06 6.83 +12.71% sync_test.BenchmarkMutexUncontended-8 3.63 3.60 -0.83% sync_test.BenchmarkMutexUncontended-16 2.38 2.49 +4.62% sync_test.BenchmarkMutex 25.00 26.40 +5.60% sync_test.BenchmarkMutex-2 231.00 49.00 -78.79% sync_test.BenchmarkMutex-4 259.00 114.00 -55.98% sync_test.BenchmarkMutex-8 641.00 110.00 -82.84% sync_test.BenchmarkMutex-16 1380.00 96.30 -93.02% sync_test.BenchmarkMutexSlack 24.80 26.20 +5.65% sync_test.BenchmarkMutexSlack-2 210.00 106.00 -49.52% sync_test.BenchmarkMutexSlack-4 453.00 119.00 -73.73% sync_test.BenchmarkMutexSlack-8 1024.00 105.00 -89.75% sync_test.BenchmarkMutexSlack-16 1291.00 91.90 -92.88% sync_test.BenchmarkMutexWork 796.00 796.00 +0.00% sync_test.BenchmarkMutexWork-2 399.00 401.00 +0.50% sync_test.BenchmarkMutexWork-4 216.00 212.00 -1.85% sync_test.BenchmarkMutexWork-8 1547.00 196.00 -87.33% sync_test.BenchmarkMutexWork-16 2754.00 287.00 -89.58% sync_test.BenchmarkMutexWorkSlack 792.00 800.00 +1.01% sync_test.BenchmarkMutexWorkSlack-2 430.00 420.00 -2.33% sync_test.BenchmarkMutexWorkSlack-4 467.00 230.00 -50.75% sync_test.BenchmarkMutexWorkSlack-8 1860.00 273.00 -85.32% sync_test.BenchmarkMutexWorkSlack-16 3029.00 294.00 -90.29%

Patch Set 1 #

Patch Set 2 : diff -r 607e0f74161f https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r b9ed2ef00448 https://go.googlecode.com/hg/ #

Total comments: 11

Patch Set 4 : diff -r dd7479dd252a https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -13 lines) Patch
M src/pkg/sync/mutex.go View 1 2 3 3 chunks +50 lines, -13 lines 0 comments Download

Messages

Total messages: 6
dvyukov
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
10 years, 11 months ago (2011-06-28 19:05:28 UTC) #1
rsc
Nice speedup. http://codereview.appspot.com/4631075/diff/5001/src/pkg/sync/mutex.go File src/pkg/sync/mutex.go (right): http://codereview.appspot.com/4631075/diff/5001/src/pkg/sync/mutex.go#newcode20 src/pkg/sync/mutex.go:20: state uint32 I think using int32 will ...
10 years, 11 months ago (2011-06-28 19:33:51 UTC) #2
dvyukov
Hello rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 11 months ago (2011-06-29 09:16:31 UTC) #3
dvyukov
On 2011/06/29 09:16:31, dvyukov wrote: > Hello mailto:rsc@golang.org (cc: mailto:golang-dev@googlegroups.com), > > Please take another ...
10 years, 11 months ago (2011-06-29 18:55:11 UTC) #4
rsc
LGTM
10 years, 11 months ago (2011-06-30 14:44:02 UTC) #5
rsc
10 years, 11 months ago (2011-06-30 15:13:38 UTC) #6
*** Submitted as http://code.google.com/p/go/source/detail?r=e540ff37120d ***

sync: improve Mutex to allow successive acquisitions
This implementation allows a goroutine to do successive acquisitions
of a mutex even if there are blocked goroutines.
Moreover, it allows a newcomer goroutine to acquire a mutex ahead of
blocked goroutines (that is, it does not enforce FIFO).
On implementation level it's achieved by separating waiter count and
locked flag.
Benchmark results on HP Z600 (2 x Xeon E5620, 8 HT cores, 2.40GHz)
are as follows (with 4631059 "replace Semacquire/Semrelease implementation"
patch applied):
benchmark                                        old ns/op    new ns/op    delta
sync_test.BenchmarkMutexUncontended                  24.10        25.40   +5.39%
sync_test.BenchmarkMutexUncontended-2                12.00        13.00   +8.33%
sync_test.BenchmarkMutexUncontended-4                 6.06         6.83  +12.71%
sync_test.BenchmarkMutexUncontended-8                 3.63         3.60   -0.83%
sync_test.BenchmarkMutexUncontended-16                2.38         2.49   +4.62%

sync_test.BenchmarkMutex                             25.00        26.40   +5.60%
sync_test.BenchmarkMutex-2                          231.00        49.00  -78.79%
sync_test.BenchmarkMutex-4                          259.00       114.00  -55.98%
sync_test.BenchmarkMutex-8                          641.00       110.00  -82.84%
sync_test.BenchmarkMutex-16                        1380.00        96.30  -93.02%

sync_test.BenchmarkMutexSlack                        24.80        26.20   +5.65%
sync_test.BenchmarkMutexSlack-2                     210.00       106.00  -49.52%
sync_test.BenchmarkMutexSlack-4                     453.00       119.00  -73.73%
sync_test.BenchmarkMutexSlack-8                    1024.00       105.00  -89.75%
sync_test.BenchmarkMutexSlack-16                   1291.00        91.90  -92.88%

sync_test.BenchmarkMutexWork                        796.00       796.00   +0.00%
sync_test.BenchmarkMutexWork-2                      399.00       401.00   +0.50%
sync_test.BenchmarkMutexWork-4                      216.00       212.00   -1.85%
sync_test.BenchmarkMutexWork-8                     1547.00       196.00  -87.33%
sync_test.BenchmarkMutexWork-16                    2754.00       287.00  -89.58%

sync_test.BenchmarkMutexWorkSlack                   792.00       800.00   +1.01%
sync_test.BenchmarkMutexWorkSlack-2                 430.00       420.00   -2.33%
sync_test.BenchmarkMutexWorkSlack-4                 467.00       230.00  -50.75%
sync_test.BenchmarkMutexWorkSlack-8                1860.00       273.00  -85.32%
sync_test.BenchmarkMutexWorkSlack-16               3029.00       294.00  -90.29%

R=rsc
CC=golang-dev
http://codereview.appspot.com/4631075

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b