Avoid allocations when building a tree

[poszu]

Reuse layer parking buffer

It avoids reallocating memory for layer parking by not discarding the parking when it is not needed anymore (it sets slice length to 0 instead of assigning a nil).

Allow the user to reuse memory that was passed to Tree::AddLeaf.

This is achieved by copying the passed data to the parking instead of just assigning the slice.

Allow reusing memory for calculating the hash of a parent

This is achieved by changing the shared.HashFunc to accept a memory buffer into which the hash can be directly calculated. There is a permanent parent buffer (Tree::parentBuf) to keep the memory around.

[noamnelke]

Looks great, added some minor improvement suggestions.

[noamnelke]

Perhaps it would be clearer like this?

if rChild.OnProvenPath && !lChild.OnProvenPath {
	copy := append([]byte(nil), lChild.value...)
	t.proof = append(t.proof, copy)
}
if lChild.OnProvenPath && !rChild.OnProvenPath {
	copy := append([]byte(nil), rChild.value...)
	t.proof = append(t.proof, copy)
}

If the t.proof was a flat byte slice this code would be cleaner, but not top priority if it's hard to do.

[poszu]

Thanks!

If the t.proof was a flat byte slice this code would be cleaner, but not top priority if it's hard to do.

I was also thinking about this. But it's more complicated and changes the public API so I left it for the future.

[noamnelke]

This assignment seems redundant. In fact I don't understand why t.calcParent needs to get a buffer at all. It seems that it should always use t.parentBuf directly since it has access to it.

There are only two uses of t.calcParent and I don't think there's a reason not to use t.parentBuf for the second usage, too (here you use nil in the other instance, but I don't see a reason to, as Tree is not designed to be thread-safe anyway).

Am I missing something?

[noamnelke]

I think you can also just assign the result of t.calcParent directly into n a few lines below, not a big deal, though.

[poszu]

I didn't use t.parentBuf in RootAndProof as it complicated it without any perf improvement. The calculated parent needed to be copied anyway in every code path.

[noamnelke]

1. What I meant is that inside the implementation of t.calcParent you can pass t.parentBuf into t.hash directly, instead of accepting it as an argument.
2. Why do this t.parentBuf = parent.value? Why not just delete this line? (it's now t.parentBuf = n.value)

[poszu]

Ad 1.
Because I don't want to reuse t.parentBuf in RootAndProof method (hence it passes nil). It only complicated the code there.

Ad 2.
It needs to reassign because the buffer passed into t.hash() will be reallocated the first time it is called (we don't know the size of the required buffer upfront to preallocate it or even use an array). It's the same story as with buf = append(buf, ...).