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

Issue 46970043: code review 46970043: runtime: allocate goroutine ids in batches (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by dvyukov
Modified:
11 years, 1 month ago
Reviewers:
khr
CC:
golang-codereviews, dave_cheney.net, bradfitz, khr, rsc
Visibility:
Public.

Description

runtime: allocate goroutine ids in batches Helps reduce contention on sched.goidgen. benchmark old ns/op new ns/op delta BenchmarkCreateGoroutines-16 259 237 -8.49% BenchmarkCreateGoroutinesParallel-16 127 43 -66.06%

Patch Set 1 #

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

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

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

Total comments: 1

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -6 lines) Patch
M src/pkg/runtime/proc.c View 1 2 3 4 5 4 chunks +19 lines, -6 lines 0 comments Download
M src/pkg/runtime/runtime.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 10
dvyukov
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://dvyukov%40google.com@code.google.com/p/go/
11 years, 2 months ago (2014-01-01 21:26:18 UTC) #1
dave_cheney.net
This change makes good sense. How did you arrive at the batching figure of 16 ...
11 years, 2 months ago (2014-01-01 21:29:18 UTC) #2
dvyukov
that's the first number that came to my mind... i've also tried 64 and it ...
11 years, 2 months ago (2014-01-01 21:36:32 UTC) #3
dvyukov
> https://codereview.appspot.com/46970043/diff/60001/src/pkg/runtime/proc.c#newcode1787 > > src/pkg/runtime/proc.c:1787: p->goidcacheend = p->goidcache + 16; > > 16 needs to ...
11 years, 2 months ago (2014-01-02 08:59:42 UTC) #4
dave_cheney.net
On 2014/01/02 08:59:42, dvyukov wrote: > > > https://codereview.appspot.com/46970043/diff/60001/src/pkg/runtime/proc.c#newcode1787 > > > src/pkg/runtime/proc.c:1787: p->goidcacheend = ...
11 years, 2 months ago (2014-01-02 09:04:52 UTC) #5
bradfitz
On Thu, Jan 2, 2014 at 1:04 AM, <dave@cheney.net> wrote: > On 2014/01/02 08:59:42, dvyukov ...
11 years, 2 months ago (2014-01-02 18:15:47 UTC) #6
dave_cheney.net
Excellent. I'm glad you feel this way. > On 3 Jan 2014, at 5:15, Brad ...
11 years, 2 months ago (2014-01-02 21:51:04 UTC) #7
dvyukov
ping
11 years, 1 month ago (2014-01-18 15:44:16 UTC) #8
khr
On 2014/01/18 15:44:16, dvyukov wrote: > ping LGTM.
11 years, 1 month ago (2014-01-21 22:04:25 UTC) #9
dvyukov
11 years, 1 month ago (2014-01-22 06:34:47 UTC) #10
*** Submitted as https://code.google.com/p/go/source/detail?r=32309d9c952e ***

runtime: allocate goroutine ids in batches
Helps reduce contention on sched.goidgen.

benchmark                               old ns/op    new ns/op    delta
BenchmarkCreateGoroutines-16                  259          237   -8.49%
BenchmarkCreateGoroutinesParallel-16          127           43  -66.06%

R=golang-codereviews, dave, bradfitz, khr
CC=golang-codereviews, rsc
https://codereview.appspot.com/46970043
Sign in to reply to this message.

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