Reduce memory allocations when computing a tree hash

[pvragov]

Hello. I've decided to rework ComputeTreeHash function because it makes lots of memory allocations.
Benchmark I used:

func BenchmarkComputeTreeHash(b *testing.B) {
	var hashes [][]byte
	for i := 0; i < 1024; i++ {
		h := sha256.Sum256([]byte("test"))
		hashes = append(hashes, h[:])
	}
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_ = glacier.ComputeTreeHash(hashes)
	}
	b.ReportAllocs()
}

Benchcmp result:

benchmark                       old ns/op     new ns/op     delta
BenchmarkComputeTreeHash-10     128886        88221         -31.55%

benchmark                       old allocs     new allocs     delta
BenchmarkComputeTreeHash-10     3124           1              -99.97%

benchmark                       old bytes     new bytes     delta
BenchmarkComputeTreeHash-10     179808        32768         -81.78%

[lucix-aws]

This is a solid change. I have one request: please update treehash_test.go to include actual unit tests for this logic. Right now it doesn't have any real *testing.T-based tests, it just has a standalone function that you'd have to run manually (that doesn't even assert anything). Since the discrete cases here are basically no hash, one hash, even hashes, and odd hashes, I think a case for each # of input hashes in the range [0,3] should cover everything.

When you do so, please verify that the resulting hashes in both the old and new implementation are identical.

As an aside, v2 of the SDK has this same code for glacier. If you'd like the contribution credit there, please open an identical PR with these changes, otherwise we'll happily do ourselves.

[pvragov]

This is a solid change. I have one request: please update treehash_test.go to include actual unit tests for this logic. Right now it doesn't have any real *testing.T-based tests, it just has a standalone function that you'd have to run manually (that doesn't even assert anything). Since the discrete cases here are basically no hash, one hash, even hashes, and odd hashes, I think a case for each # of input hashes in the range [0,3] should cover everything.

When you do so, please verify that the resulting hashes in both the old and new implementation are identical.

As an aside, v2 of the SDK has this same code for glacier. If you'd like the contribution credit there, please open an identical PR with these changes, otherwise we'll happily do ourselves.

Hello, @lucix-aws. Thanks for the review. I've added the tests you asked. Also I've tested that the old implementation passes the tests.

P.S. I'll open the identical PR for v2 after approval. Thanks for the advice.

[lucix-aws]

@nsofanat1k Thanks again for the contribution. This is good to merge - we plan to do so either later this afternoon or early tomorrow morning.

In the meantime, you're free to open the equivalent in v2.

[isaiahvita]

@nsofanat1k LGTM thanks again for the contribution. @lucix-aws will handle the merge