Skip to content

Remove unnecessary allocations in HopData encode/decode methods #26

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrekucci
Copy link

No description provided.

@halseth
Copy link

halseth commented Apr 26, 2018

Thanks for the PR!

This PR needs some explanation/rationale for the change it is making. Could you add some comments and/or benchmarks to the PR description and commit message?

@mrekucci mrekucci force-pushed the remove_unnecessary_allocations_in_hopdata_encode_decode branch from e386a4b to b4b6a03 Compare April 26, 2018 22:21
@mrekucci
Copy link
Author

I added the explanation to the commit message.

By removing io.Writer and io.Reader abstraction from encode and decode
methods, the following benchmars:

old:

func BenchmarkHopDataEncode(b *testing.B) {
	hd := new(HopData)
	for i := 0; i < b.N; i++ {
		hd.Encode(ioutil.Discard)
	}
}

type nopReader struct{}

func (nopReader) Read(b []byte) (int, error) { return len(b), nil }

func BenchmarkHopDataDecode(b *testing.B) {
	hd := new(HopData)
	src := nopReader{}
	for i := 0; i < b.N; i++ {
		hd.Decode(src)
	}
}

new:

func BenchmarkHopDataEncode(b *testing.B) {
	hd := new(HopData)
	dst := make([]byte, hopDataSize)
	for i := 0; i < b.N; i++ {
		hd.Encode(dst)
	}
}

func BenchmarkHopDataDecode(b *testing.B) {
	hd := new(HopData)
	src := make([]byte, hopDataSize)
	for i := 0; i < b.N; i++ {
		hd.Decode(src)
	}
}

show the following insrease in performance:

benchmark                    old ns/op     new ns/op     delta
BenchmarkHopDataEncode-4     80.7          12.1          -85.01%

benchmark                    old allocs     new allocs     delta
BenchmarkHopDataEncode-4     3              0              -100.00%

benchmark                    old bytes     new bytes     delta
BenchmarkHopDataEncode-4     24            0             -100.00%

benchmark                    old ns/op     new ns/op     delta
BenchmarkHopDataDecode-4     218           9.77          -95.52%

benchmark                    old allocs     new allocs     delta
BenchmarkHopDataDecode-4     4              0              -100.00%

benchmark                    old bytes     new bytes     delta
BenchmarkHopDataDecode-4     56            0             -100.00%
@mrekucci mrekucci force-pushed the remove_unnecessary_allocations_in_hopdata_encode_decode branch from b4b6a03 to b1eb2d0 Compare April 26, 2018 22:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants