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

WIP Refactor proto #151

Merged
merged 6 commits into from Jan 23, 2014
Merged

WIP Refactor proto #151

merged 6 commits into from Jan 23, 2014

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Jan 11, 2014

switch to use gogoprotobuf generated codes.
Gogoprobuf will generate helper codes that do not use reflection.
Performance improvement about 50% for the switching.
Another 100%+ improvement for refactoring.

Before using gogoprotobuf
BenchmarkAppendEntriesRequestEncoding 1000 1806812 ns/op 74.17 MB/s
BenchmarkAppendEntriesRequestDecoding 1000 2217673 ns/op 60.43 MB/s
BenchmarkAppendEntriesResponseEncoding 1000000 1424 ns/op 5.62 MB/s
BenchmarkAppendEntriesResponseDecoding 1000000 3434 ns/op 2.33 MB/s

After using gogoprotobuf
BenchmarkAppendEntriesRequestEncoding 2000 1084766 ns/op 123.54 MB/s
BenchmarkAppendEntriesRequestDecoding 1000 1715724 ns/op 78.11 MB/s
BenchmarkAppendEntriesResponseEncoding 5000000 636 ns/op 12.57 MB/s
BenchmarkAppendEntriesResponseDecoding 1000000 2469 ns/op 3.24 MB/s

After refactoring (stop copying around)
BenchmarkAppendEntriesRequestEncoding 5000 367500 ns/op 364.67 MB/s
BenchmarkAppendEntriesRequestDecoding 2000 1448770 ns/op 92.50 MB/s
BenchmarkAppendEntriesResponseEncoding 5000000 642 ns/op 12.46 MB/s
BenchmarkAppendEntriesResponseDecoding 1000000 2225 ns/op 3.59 MB/s

There are still some improving space. I am working on this.

@xiang90
Copy link
Contributor Author

xiang90 commented Jan 11, 2014

@benbjohnson There are still some cleaning up stuff to work on. But you can review this pr to get a rough idea what I am doing.

@benbjohnson
Copy link
Contributor

@xiangli-cmu That looks good to me. Can we remove the Proto prefix from all the type names? It seems redundant since they're already in the protobuf package.

@xiang90
Copy link
Contributor Author

xiang90 commented Jan 13, 2014

@benbjohnson Will do.

@philips
Copy link
Member

philips commented Jan 13, 2014

Does this change the protocol or log format?

@xiang90
Copy link
Contributor Author

xiang90 commented Jan 23, 2014

@philips No. I did not change any format.

@xiang90
Copy link
Contributor Author

xiang90 commented Jan 23, 2014

@benbjohnson Can we merge this? I have some more cleaning up work to do. But it is better to do in another pull request I think. Thanks.

benbjohnson added a commit that referenced this pull request Jan 23, 2014
@benbjohnson benbjohnson merged commit ad05eb4 into goraft:master Jan 23, 2014
@benbjohnson
Copy link
Contributor

👍

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.

None yet

3 participants