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

Issue 9129044: code review 9129044: encoding/json: faster encoding (Closed)

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

Description

encoding/json: faster encoding The old code was caching per-type struct field info. Instead, cache type-specific encoding funcs, tailored for that particular type to avoid unnecessary reflection at runtime. Once the machine is built once, future encodings of that type just run the func. benchmark old ns/op new ns/op delta BenchmarkCodeEncoder 48424939 36975320 -23.64% benchmark old MB/s new MB/s speedup BenchmarkCodeEncoder 40.07 52.48 1.31x Additionally, the numbers seem stable now at ~52 MB/s, whereas the numbers for the old code were all over the place: 11 MB/s, 40 MB/s, 13 MB/s, 39 MB/s, etc. In the benchmark above I compared against the best I saw the old code do.

Patch Set 1 #

Patch Set 2 : diff -r 740d244b2047 https://go.googlecode.com/hg/ #

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

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

Patch Set 5 : diff -r 740d244b2047 https://go.googlecode.com/hg/ #

Total comments: 7

Patch Set 6 : diff -r d881cb1ffc14 https://go.googlecode.com/hg/ #

Patch Set 7 : diff -r d881cb1ffc14 https://go.googlecode.com/hg/ #

Patch Set 8 : diff -r d881cb1ffc14 https://go.googlecode.com/hg/ #

Total comments: 3

Patch Set 9 : diff -r 07295f40fe71 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+410 lines, -162 lines) Patch
M src/pkg/encoding/json/decode_test.go View 1 2 3 4 5 4 chunks +43 lines, -2 lines 0 comments Download
M src/pkg/encoding/json/encode.go View 1 2 3 4 5 6 7 8 2 chunks +367 lines, -160 lines 0 comments Download

Messages

Total messages: 13
bradfitz
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
10 years, 11 months ago (2013-05-04 17:04:53 UTC) #1
bradfitz
(obviously for after Go 1.1. just getting off my laptop...) On Sat, May 4, 2013 ...
10 years, 11 months ago (2013-05-04 17:05:57 UTC) #2
gobot
R=r (assigned by bradfitz)
10 years, 10 months ago (2013-05-14 22:15:03 UTC) #3
gobot
R=rsc (assigned by r)
10 years, 10 months ago (2013-05-17 21:13:57 UTC) #4
bradfitz
Ping. This CL is now 4 weeks old, without any feedback. I'll take a review ...
10 years, 10 months ago (2013-06-03 13:09:54 UTC) #5
rsc
Still having trouble getting through my inbox. How about this: I will review this CL ...
10 years, 10 months ago (2013-06-03 13:11:33 UTC) #6
adg
Looks good. https://codereview.appspot.com/9129044/diff/9001/src/pkg/encoding/json/encode.go File src/pkg/encoding/json/encode.go (right): https://codereview.appspot.com/9129044/diff/9001/src/pkg/encoding/json/encode.go#newcode233 src/pkg/encoding/json/encode.go:233: if _, ok := r.(runtime.Error); ok { ...
10 years, 9 months ago (2013-07-03 00:09:01 UTC) #7
rsc
I took a look at this CL. I understand that it is a bit like ...
10 years, 7 months ago (2013-08-06 16:33:26 UTC) #8
r
also, gob needs a rewrite, now that we know how to write code like this. ...
10 years, 7 months ago (2013-08-06 20:48:57 UTC) #9
bradfitz
Hello rsc@golang.org, adg@golang.org (cc: gobot@golang.org, golang-dev@googlegroups.com, r@golang.org), Please take another look.
10 years, 7 months ago (2013-08-07 16:35:44 UTC) #10
bradfitz
PTAL Rearranged the code a bit to try to minimize patch size. Not sure it ...
10 years, 7 months ago (2013-08-07 16:37:30 UTC) #11
rsc
LGTM https://codereview.appspot.com/9129044/diff/26001/src/pkg/encoding/json/encode.go File src/pkg/encoding/json/encode.go (right): https://codereview.appspot.com/9129044/diff/26001/src/pkg/encoding/json/encode.go#newcode356 src/pkg/encoding/json/encode.go:356: // buildTypeEncoder constructs an encoderFunc for a type. ...
10 years, 7 months ago (2013-08-09 02:04:12 UTC) #12
bradfitz
10 years, 7 months ago (2013-08-09 16:46:52 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=5a51d54e34bb ***

encoding/json: faster encoding

The old code was caching per-type struct field info. Instead,
cache type-specific encoding funcs, tailored for that
particular type to avoid unnecessary reflection at runtime.
Once the machine is built once, future encodings of that type
just run the func.

benchmark               old ns/op    new ns/op    delta
BenchmarkCodeEncoder     48424939     36975320  -23.64%

benchmark                old MB/s     new MB/s  speedup
BenchmarkCodeEncoder        40.07        52.48    1.31x

Additionally, the numbers seem stable now at ~52 MB/s, whereas
the numbers for the old code were all over the place: 11 MB/s,
40 MB/s, 13 MB/s, 39 MB/s, etc.  In the benchmark above I compared
against the best I saw the old code do.

R=rsc, adg
CC=gobot, golang-dev, r
https://codereview.appspot.com/9129044
Sign in to reply to this message.

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