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

Issue 11573043: sync: faster Cond (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by dvyukov
Modified:
4 years, 3 months ago
Reviewers:
rsc
CC:
sougou, rsc, bradfitz, r, golang-dev
Visibility:
Public.

Description

sync: faster Cond The new version does not require any memory allocations and is 30-50% faster. Also detect and painc if Cond is copied after first. benchmark old ns/op new ns/op delta BenchmarkCond1 317 195 -38.49% BenchmarkCond1-2 875 607 -30.63% BenchmarkCond1-4 1116 548 -50.90% BenchmarkCond1-8 1013 613 -39.49% BenchmarkCond1-16 983 450 -54.22% BenchmarkCond2 559 352 -37.03% BenchmarkCond2-2 1916 1378 -28.08% BenchmarkCond2-4 1518 1322 -12.91% BenchmarkCond2-8 2313 1291 -44.19% BenchmarkCond2-16 1885 1078 -42.81% BenchmarkCond4 1070 614 -42.62% BenchmarkCond4-2 4899 3047 -37.80% BenchmarkCond4-4 3813 3006 -21.16% BenchmarkCond4-8 3605 3045 -15.53% BenchmarkCond4-16 4148 2637 -36.43% BenchmarkCond8 2086 1264 -39.41% BenchmarkCond8-2 9961 6736 -32.38% BenchmarkCond8-4 8135 7689 -5.48% BenchmarkCond8-8 9623 7517 -21.89% BenchmarkCond8-16 11661 8093 -30.60%

Patch Set 1 #

Patch Set 2 : diff -r f98e0eb03af9 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 3 : diff -r d7db8c804ffa https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 4 : diff -r d7db8c804ffa https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 5 : diff -r d7db8c804ffa https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 6 : diff -r d7db8c804ffa https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 7 : diff -r d7db8c804ffa https://dvyukov%40google.com@code.google.com/p/go/ #

Total comments: 6

Patch Set 8 : diff -r 3c7cac86a284 https://dvyukov%40google.com@code.google.com/p/go/ #

Total comments: 3

Patch Set 9 : diff -r 7064d3304d65 https://dvyukov%40google.com@code.google.com/p/go/ #

Total comments: 2

Patch Set 10 : diff -r ebf9b6a68289 https://dvyukov%40google.com@code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+298 lines, -75 lines) Patch
M src/pkg/runtime/sema.goc View 1 2 3 4 5 6 7 8 6 chunks +101 lines, -10 lines 0 comments Download
M src/pkg/sync/cond.go View 1 2 3 4 5 chunks +50 lines, -65 lines 0 comments Download
M src/pkg/sync/cond_test.go View 1 2 3 2 chunks +129 lines, -0 lines 0 comments Download
M src/pkg/sync/runtime.go View 1 2 3 4 5 6 7 2 chunks +18 lines, -0 lines 0 comments Download

Messages

Total messages: 18
sougou
Don't know enough about runtime to comment there, but the Cond side looks peachy.
4 years, 4 months ago (2013-07-19 15:54:34 UTC) #1
rsc
Okay, now I understand why this works. I want to avoid putting runtime Locks into ...
4 years, 3 months ago (2013-08-09 14:44:51 UTC) #2
rsc
If you don't want to use an address-indexed table it is also fine to make ...
4 years, 3 months ago (2013-08-09 15:15:10 UTC) #3
dvyukov
On Fri, Aug 9, 2013 at 7:15 PM, Russ Cox <rsc@golang.org> wrote: > If you ...
4 years, 3 months ago (2013-08-09 15:19:49 UTC) #4
dvyukov
On Fri, Aug 9, 2013 at 6:44 PM, <rsc@golang.org> wrote: > Okay, now I understand ...
4 years, 3 months ago (2013-08-09 16:52:09 UTC) #5
dvyukov
On Fri, Aug 9, 2013 at 6:44 PM, <rsc@golang.org> wrote: > Okay, now I understand ...
4 years, 3 months ago (2013-08-09 16:58:48 UTC) #6
rsc
On Fri, Aug 9, 2013 at 12:50 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Fri, ...
4 years, 3 months ago (2013-08-09 18:02:31 UTC) #7
dvyukov
On 2013/08/09 18:02:31, rsc wrote: > On Fri, Aug 9, 2013 at 12:50 PM, Dmitry ...
4 years, 3 months ago (2013-08-09 18:07:48 UTC) #8
rsc
> > As a side effect of the back pointer trick, we will be able ...
4 years, 3 months ago (2013-08-09 18:13:43 UTC) #9
dvyukov
Hello sougou@google.com, rsc@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://dvyukov%40google.com@code.google.com/p/go/
4 years, 3 months ago (2013-08-12 13:49:31 UTC) #10
dvyukov
On 2013/08/09 18:13:43, rsc wrote: > > > > As a side effect of the ...
4 years, 3 months ago (2013-08-12 13:55:36 UTC) #11
bradfitz
https://codereview.appspot.com/11573043/diff/29001/src/pkg/sync/runtime.go File src/pkg/sync/runtime.go (right): https://codereview.appspot.com/11573043/diff/29001/src/pkg/sync/runtime.go#newcode22 src/pkg/sync/runtime.go:22: type syncSema [3]uintptr add a comment referencing where/what this ...
4 years, 3 months ago (2013-08-12 14:55:34 UTC) #12
dvyukov
PTAL https://codereview.appspot.com/11573043/diff/29001/src/pkg/sync/runtime.go File src/pkg/sync/runtime.go (right): https://codereview.appspot.com/11573043/diff/29001/src/pkg/sync/runtime.go#newcode22 src/pkg/sync/runtime.go:22: type syncSema [3]uintptr On 2013/08/12 14:55:35, bradfitz wrote: ...
4 years, 3 months ago (2013-08-12 17:23:12 UTC) #13
bradfitz
https://codereview.appspot.com/11573043/diff/36001/src/pkg/runtime/sema.goc File src/pkg/runtime/sema.goc (right): https://codereview.appspot.com/11573043/diff/36001/src/pkg/runtime/sema.goc#newcode221 src/pkg/runtime/sema.goc:221: // Returns false if the semaphore is corrupted. also ...
4 years, 3 months ago (2013-08-12 17:52:53 UTC) #14
dvyukov
PTAL https://codereview.appspot.com/11573043/diff/36001/src/pkg/runtime/sema.goc File src/pkg/runtime/sema.goc (right): https://codereview.appspot.com/11573043/diff/36001/src/pkg/runtime/sema.goc#newcode221 src/pkg/runtime/sema.goc:221: // Returns false if the semaphore is corrupted. ...
4 years, 3 months ago (2013-08-12 18:05:57 UTC) #15
r
https://codereview.appspot.com/11573043/diff/43001/src/pkg/sync/runtime.go File src/pkg/sync/runtime.go (right): https://codereview.appspot.com/11573043/diff/43001/src/pkg/sync/runtime.go#newcode28 src/pkg/sync/runtime.go:28: // Syncsemrelease waits for n pairing Syncsemacquire on the ...
4 years, 3 months ago (2013-08-13 03:50:09 UTC) #16
rsc
LGTM https://codereview.appspot.com/11573043/diff/43001/src/pkg/sync/runtime.go File src/pkg/sync/runtime.go (right): https://codereview.appspot.com/11573043/diff/43001/src/pkg/sync/runtime.go#newcode28 src/pkg/sync/runtime.go:28: // Syncsemrelease waits for n pairing Syncsemacquire on ...
4 years, 3 months ago (2013-08-13 04:05:50 UTC) #17
dvyukov
4 years, 3 months ago (2013-08-13 10:45:41 UTC) #18
*** Submitted as https://code.google.com/p/go/source/detail?r=0b7a3fb56379 ***

sync: faster Cond
The new version does not require any memory allocations and is 30-50% faster.
Also detect and painc if Cond is copied after first.

benchmark            old ns/op    new ns/op    delta
BenchmarkCond1             317          195  -38.49%
BenchmarkCond1-2           875          607  -30.63%
BenchmarkCond1-4          1116          548  -50.90%
BenchmarkCond1-8          1013          613  -39.49%
BenchmarkCond1-16          983          450  -54.22%
BenchmarkCond2             559          352  -37.03%
BenchmarkCond2-2          1916         1378  -28.08%
BenchmarkCond2-4          1518         1322  -12.91%
BenchmarkCond2-8          2313         1291  -44.19%
BenchmarkCond2-16         1885         1078  -42.81%
BenchmarkCond4            1070          614  -42.62%
BenchmarkCond4-2          4899         3047  -37.80%
BenchmarkCond4-4          3813         3006  -21.16%
BenchmarkCond4-8          3605         3045  -15.53%
BenchmarkCond4-16         4148         2637  -36.43%
BenchmarkCond8            2086         1264  -39.41%
BenchmarkCond8-2          9961         6736  -32.38%
BenchmarkCond8-4          8135         7689   -5.48%
BenchmarkCond8-8          9623         7517  -21.89%
BenchmarkCond8-16        11661         8093  -30.60%

R=sougou, rsc, bradfitz, r
CC=golang-dev
https://codereview.appspot.com/11573043
Sign in to reply to this message.

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