goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 480 forks source link

WIP Refactor proto #151

Closed xiang90 closed 10 years ago

xiang90 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

@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 commented 10 years ago

@benbjohnson Will do.

philips commented 10 years ago

Does this change the protocol or log format?

xiang90 commented 10 years ago

@philips No. I did not change any format.

xiang90 commented 10 years ago

@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 commented 10 years ago

:+1: