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

Issue 122090043: code review 122090043: runtime: convert markallocated from C to Go (Closed)

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

Description

runtime: convert markallocated from C to Go benchmark old ns/op new ns/op delta BenchmarkMalloc8 28.7 22.4 -21.95% BenchmarkMalloc16 44.8 33.8 -24.55% BenchmarkMallocTypeInfo8 49.0 32.9 -32.86% BenchmarkMallocTypeInfo16 46.7 35.8 -23.34% BenchmarkMallocLargeStruct 907 901 -0.66% BenchmarkGobDecode 13235542 12036851 -9.06% BenchmarkGobEncode 10639699 9539155 -10.34% BenchmarkJSONEncode 25193036 21898922 -13.08% BenchmarkJSONDecode 96104044 89464904 -6.91% Fixes issue 8452.

Patch Set 1 #

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

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

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

Total comments: 13

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -120 lines) Patch
M src/pkg/runtime/malloc.h View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M src/pkg/runtime/malloc.go View 1 2 3 4 3 chunks +123 lines, -7 lines 0 comments Download
M src/pkg/runtime/mgc0.c View 1 2 3 4 4 chunks +19 lines, -110 lines 0 comments Download
M src/pkg/runtime/stubs.go View 1 2 3 4 2 chunks +9 lines, -2 lines 0 comments Download
M src/pkg/runtime/stubs.goc View 1 2 3 4 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 9
dvyukov
Hello golang-codereviews@googlegroups.com (cc: khr@golang.org, rsc@golang.org), I'd like you to review this change to https://dvyukov%40google.com@code.google.com/p/go/
10 years, 7 months ago (2014-08-05 18:30:57 UTC) #1
bradfitz
It pays not to bound between two stacks, huh? Nice. On Tue, Aug 5, 2014 ...
10 years, 7 months ago (2014-08-05 18:39:13 UTC) #2
dvyukov
Yes, but it just recovers performance that we lost several CLs ago. This is not ...
10 years, 7 months ago (2014-08-05 18:42:44 UTC) #3
rsc
R=khr
10 years, 7 months ago (2014-08-05 18:49:41 UTC) #4
dave_cheney.net
https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go File src/pkg/runtime/malloc.go (right): https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go#newcode240 src/pkg/runtime/malloc.go:240: //if (runtime·atomicload((uint32*)ptrmask)&0xff) == 0 { left over debugging ?
10 years, 7 months ago (2014-08-06 01:38:17 UTC) #5
dvyukov
https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go File src/pkg/runtime/malloc.go (right): https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go#newcode240 src/pkg/runtime/malloc.go:240: //if (runtime·atomicload((uint32*)ptrmask)&0xff) == 0 { On 2014/08/06 01:38:17, dfc ...
10 years, 7 months ago (2014-08-06 14:57:46 UTC) #6
khr
LGTM, pending the atomic stuff. https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go File src/pkg/runtime/malloc.go (right): https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go#newcode294 src/pkg/runtime/malloc.go:294: *xbitsb = 0 = ...
10 years, 7 months ago (2014-08-06 22:29:16 UTC) #7
dvyukov
https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go File src/pkg/runtime/malloc.go (right): https://codereview.appspot.com/122090043/diff/60001/src/pkg/runtime/malloc.go#newcode240 src/pkg/runtime/malloc.go:240: //if (runtime·atomicload((uint32*)ptrmask)&0xff) == 0 { On 2014/08/06 14:57:45, dvyukov ...
10 years, 7 months ago (2014-08-07 09:32:48 UTC) #8
dvyukov
10 years, 7 months ago (2014-08-07 09:34:36 UTC) #9
*** Submitted as https://code.google.com/p/go/source/detail?r=d50ba39e60ab ***

runtime: convert markallocated from C to Go

benchmark                      old ns/op     new ns/op     delta
BenchmarkMalloc8               28.7          22.4          -21.95%
BenchmarkMalloc16              44.8          33.8          -24.55%
BenchmarkMallocTypeInfo8       49.0          32.9          -32.86%
BenchmarkMallocTypeInfo16      46.7          35.8          -23.34%
BenchmarkMallocLargeStruct     907           901           -0.66%
BenchmarkGobDecode             13235542      12036851      -9.06%
BenchmarkGobEncode             10639699      9539155       -10.34%
BenchmarkJSONEncode            25193036      21898922      -13.08%
BenchmarkJSONDecode            96104044      89464904      -6.91%

Fixes issue 8452.

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

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