Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

Use ugorji codec for unmarshalling responses from etcd server. #218

Merged
merged 1 commit into from
Jun 5, 2015

Conversation

fgrzadkowski
Copy link
Contributor

This change speeds up RawResponse.Unmarshal ~2x:

BEFORE:

$ go test -test.run=XXXXX -bench=.*
PASS
BenchmarkSmallResponseUnmarshal        10000        180181 ns/op
BenchmarkManySmallResponseUnmarshal      100      15115904 ns/op
BenchmarkMediumResponseUnmarshal        1000       2302976 ns/op
BenchmarkLargeResponseUnmarshal           20      94022971 ns/op
ok      github.com/coreos/go-etcd/etcd  8.963s

AFTER:

$ go test -test.run=XXXXX -bench=.*
PASS
BenchmarkSmallResponseUnmarshal        20000         85873 ns/op
BenchmarkManySmallResponseUnmarshal      200       7069753 ns/op
BenchmarkMediumResponseUnmarshal        1000       1339490 ns/op
BenchmarkLargeResponseUnmarshal           20      60579560 ns/op
ok      github.com/coreos/go-etcd/etcd  7.604s

I also updated install section in README.md file to make it clear that user have to go get ugorji codec.

Fixes #217
Refs kubernetes/kubernetes#4521

@davidopp @wojtek-t @xiang90 @timothysc

@xiang90
Copy link
Contributor

xiang90 commented Jun 3, 2015

@fgrzadkowski I am OK with this change. I am not sure how will this improve the overall perf of k8s.

A more interesting question is: do you also want to do this at server side? Why not let etcd server accept protobuf if you really care about perf?

@fgrzadkowski
Copy link
Contributor Author

I think that moving towards protobuf makes sense, but it seems like a major change. This one is simple and from manual tests it improves latency metrics by 20-30%.

@timothysc
Copy link

@xiang90 crossref - kubernetes/kubernetes#8132

@philips
Copy link
Contributor

philips commented Jun 3, 2015

This is great. We should also do it for the still developing github.com/coreos/etcd/client package. Also, we should not vendor the codec package as go-etcd is a library.

Overall, LGTM.

@xiang90
Copy link
Contributor

xiang90 commented Jun 3, 2015

LGTM

/cc @yichengq you are more familiar with go-etcd codebase.

@yichengq
Copy link
Contributor

yichengq commented Jun 5, 2015

lgtm

yichengq added a commit that referenced this pull request Jun 5, 2015
Use ugorji codec for unmarshalling responses from etcd server.
@yichengq yichengq merged commit 4cceaf7 into coreos:master Jun 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants