Skip to content

Commit

Permalink
compact: Visit the seed node in the Append method (#20)
Browse files Browse the repository at this point in the history
Most uses of compact range invoke the visitor for the leaf node before
the Append call. This change makes it the default behaviour of Append.

The asymmetry between Append and AppendRange can be explained by the
fact that Append is usually called when the new leaf hasn't been
processed yet, whereas AppendRange is usually called when all the nodes
of the compact ranges (as well as nodes below them) have been processed.
  • Loading branch information
pav-kv authored Apr 11, 2022
1 parent f40960a commit cfdaeb1
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
7 changes: 4 additions & 3 deletions compact/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ func (r *Range) Hashes() [][]byte {
}

// Append extends the compact range by appending the passed in hash to it. It
// uses the tree hasher to calculate hashes of newly created nodes, and reports
// them through the visitor function (if non-nil).
// reports all the added nodes through the visitor function (if non-nil).
func (r *Range) Append(hash []byte, visitor VisitFn) error {
// TODO(pphaneuf): Consider calling `visitor` for this hash, for consistency.
if visitor != nil {
visitor(NewNodeID(0, r.end), hash)
}
return r.appendImpl(r.end+1, hash, nil, visitor)
}

Expand Down
4 changes: 0 additions & 4 deletions compact/range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ func TestAppend(t *testing.T) {
cr := factory.NewEmptyRange(0)
tree.verifyRange(t, cr, true)
for i := uint64(0); i < size; i++ {
visit(NewNodeID(0, i), tree.leaf(i))
if err := cr.Append(tree.leaf(i), visit); err != nil {
t.Errorf("Append()=%v", err)
}
Expand Down Expand Up @@ -227,7 +226,6 @@ func TestMergeBackwards(t *testing.T) {
rng := factory.NewEmptyRange(numNodes)
tree.verifyRange(t, rng, true)
for i := numNodes; i > 0; i-- {
visit(NewNodeID(0, i-1), tree.leaf(i-1))
prepend := factory.NewEmptyRange(i - 1)
tree.verifyRange(t, prepend, true)
if err := prepend.Append(tree.leaf(i-1), visit); err != nil {
Expand Down Expand Up @@ -256,7 +254,6 @@ func TestMergeInBatches(t *testing.T) {
rng := factory.NewEmptyRange(i)
tree.verifyRange(t, rng, true)
for node := i; node < i+batch && node < numNodes; node++ {
visit(NewNodeID(0, node), tree.leaf(node))
if err := rng.Append(tree.leaf(node), visit); err != nil {
t.Fatalf("Append: %v", err)
}
Expand Down Expand Up @@ -289,7 +286,6 @@ func TestMergeRandomly(t *testing.T) {
mergeAll = func(begin, end uint64) *Range {
rng := factory.NewEmptyRange(begin)
if begin+1 == end {
visit(NewNodeID(0, begin), tree.leaf(begin))
if err := rng.Append(tree.leaf(begin), visit); err != nil {
t.Fatalf("Append(%d): %v", begin, err)
}
Expand Down

0 comments on commit cfdaeb1

Please sign in to comment.