Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Batch NewLeaf node work in base API #349

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 0 additions & 159 deletions conversion.go

This file was deleted.

167 changes: 131 additions & 36 deletions tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
"encoding/json"
"errors"
"fmt"
"runtime"
"sync"

"github.com/crate-crypto/go-ipa/banderwagon"
)
Expand Down Expand Up @@ -197,6 +199,7 @@ func (n *InternalNode) toExportable() *ExportableInternalNode {
case *InternalNode:
exportable.Children[i] = child.toExportable()
case *LeafNode:
child.Commit()
exportable.Children[i] = &ExportableLeafNode{
Stem: child.stem,
Values: child.values,
Expand Down Expand Up @@ -245,47 +248,14 @@ func NewStatelessInternal(depth byte, comm *Point) VerkleNode {

// New creates a new leaf node
func NewLeafNode(stem []byte, values [][]byte) *LeafNode {
cfg := GetConfig()

// C1.
var c1poly [NodeWidth]Fr
var c1 *Point
count := fillSuffixTreePoly(c1poly[:], values[:NodeWidth/2])
containsEmptyCodeHash := len(c1poly) >= EmptyCodeHashSecondHalfIdx &&
c1poly[EmptyCodeHashFirstHalfIdx].Equal(&EmptyCodeHashFirstHalfValue) &&
c1poly[EmptyCodeHashSecondHalfIdx].Equal(&EmptyCodeHashSecondHalfValue)
if containsEmptyCodeHash {
// Clear out values of the cached point.
c1poly[EmptyCodeHashFirstHalfIdx] = FrZero
c1poly[EmptyCodeHashSecondHalfIdx] = FrZero
// Calculate the remaining part of c1 and add to the base value.
partialc1 := cfg.CommitToPoly(c1poly[:], NodeWidth-count-2)
c1 = new(Point)
c1.Add(&EmptyCodeHashPoint, partialc1)
} else {
c1 = cfg.CommitToPoly(c1poly[:], NodeWidth-count)
}

// C2.
var c2poly [NodeWidth]Fr
count = fillSuffixTreePoly(c2poly[:], values[NodeWidth/2:])
c2 := cfg.CommitToPoly(c2poly[:], NodeWidth-count)

// Root commitment preparation for calculation.
stem = stem[:StemSize] // enforce a 31-byte length
var poly [NodeWidth]Fr
poly[0].SetUint64(1)
StemFromBytes(&poly[1], stem)
toFrMultiple([]*Fr{&poly[2], &poly[3]}, []*Point{c1, c2})
Comment on lines -248 to -279
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We stop doing work in NewLeafNode and allow c1, c2 and commitment to be nil.


return &LeafNode{
// depth will be 0, but the commitment calculation
// does not need it, and so it won't be free.
values: values,
stem: stem,
commitment: cfg.CommitToPoly(poly[:], NodeWidth-4),
c1: c1,
c2: c2,
commitment: nil,
c1: nil,
c2: nil,
}
}

Expand Down Expand Up @@ -642,11 +612,33 @@ func (n *InternalNode) fillLevels(levels [][]*InternalNode) {
}
}

func (n *InternalNode) findNewLeafNodes(newLeaves []*LeafNode) []*LeafNode {
for idx := range n.cow {
child := n.children[idx]
if childInternalNode, ok := child.(*InternalNode); ok && len(childInternalNode.cow) > 0 {
newLeaves = childInternalNode.findNewLeafNodes(newLeaves)
} else if leafNode, ok := child.(*LeafNode); ok {
if leafNode.commitment == nil {
newLeaves = append(newLeaves, leafNode)
}
}
}
return newLeaves
}
Comment on lines +615 to +627
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See L634 to understand the context for this, it's quite simple.


func (n *InternalNode) Commit() *Point {
if len(n.cow) == 0 {
return n.commitment
}

// New leaf nodes.
newLeaves := make([]*LeafNode, 0, 64)
newLeaves = n.findNewLeafNodes(newLeaves)
if len(newLeaves) > 0 {
batchCommitLeafNodes(newLeaves)
}
Comment on lines +634 to +639
Copy link
Collaborator Author

@jsign jsign Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so now in (*InternalNode).Commit() we can't continue assuming all LeafNodes are prepared.
We're responsible now to detect any newly created LeafNodes, and do the CPU-heavy work.
Note that I mention new leaf nodes. Existing LeafNodes were kept up to date with the usual diff-updating.

What findeNewLeafNodes(...) does is quite simple: simply walk the tree and look for LeafNode that have commitment == nil, which signals is a newly created leaf node.

The batchCommitLeafNodes(...) is a twist of the previous BatchNewLeafNodes(...) that we had in the specialized API that was removed. I'll comment on it later below.


// Internal nodes.
internalNodeLevels := make([][]*InternalNode, StemSize)
n.fillLevels(internalNodeLevels)

Expand Down Expand Up @@ -996,6 +988,11 @@ func (n *LeafNode) updateCn(index byte, value []byte, c *Point) {
}

func (n *LeafNode) updateLeaf(index byte, value []byte) {
if n.commitment == nil {
n.values[index] = value
return
}
Comment on lines +994 to +997
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we receive an update for a key in a leaf that was created before, we simply put the value and move on.
We don't do diff-updating for obvious reasons: there's no previous commitment.

This is good. If a new leaf node is created, and touched and re-touched in the same block execution, it's very cheap now.


// Update the corresponding C1 or C2 commitment.
var c *Point
var oldC Point
Expand All @@ -1020,6 +1017,15 @@ func (n *LeafNode) updateLeaf(index byte, value []byte) {
}

func (n *LeafNode) updateMultipleLeaves(values [][]byte) {
if n.commitment == nil {
for i, v := range values {
if len(v) != 0 && !bytes.Equal(v, n.values[i]) {
n.values[i] = v
}
}
return
}
Comment on lines +1026 to +1033
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here when we update multiple keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's great, but could you just add a comment so that we remember why that is when debugging later on? Same thing with L991

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gballet, done in 104f849.

Copy link
Collaborator Author

@jsign jsign May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like adding comments breaks tests? 🤷 ha
Looking...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gballet, go test ./... -race looks to be working fine in my machine. I wonder if this one of those weird CI things that happened before of pulling wrong code.

Do you mind doing go test ./... -race on your machine? To double check.


var oldC1, oldC2 *Point

// We iterate the values, and we update the C1 and/or C2 commitments depending on the index.
Expand Down Expand Up @@ -1110,6 +1116,10 @@ func (n *LeafNode) Commitment() *Point {
}

func (n *LeafNode) Commit() *Point {
if n.commitment == nil {
commitLeafNodes([]*LeafNode{n})
}
Comment on lines +1126 to +1128
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now (*LeafNode).Commit() can't assume that n.commitment != nil, we call commitLeafNodes(...) to do the proper calculation.


return n.commitment
}

Expand Down Expand Up @@ -1297,6 +1307,7 @@ func (n *LeafNode) GetProofItems(keys keylist) (*ProofElements, []byte, [][]byte
// Serialize serializes a LeafNode.
// The format is: <nodeType><stem><bitlist><c1comm><c2comm><children...>
func (n *LeafNode) Serialize() ([]byte, error) {
n.Commit()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we always make sure that leaf node is committed.

cBytes := banderwagon.ElementsToBytes([]*banderwagon.Element{n.c1, n.c2})
return n.serializeWithCompressedCommitments(cBytes[0], cBytes[1]), nil
}
Expand Down Expand Up @@ -1527,3 +1538,87 @@ func (n *LeafNode) serializeWithCompressedCommitments(c1Bytes [32]byte, c2Bytes

return result
}

func batchCommitLeafNodes(leaves []*LeafNode) {
minBatchSize := 8
if len(leaves) < minBatchSize {
commitLeafNodes(leaves)
return
}

batchSize := len(leaves) / runtime.NumCPU()
if batchSize < minBatchSize {
batchSize = minBatchSize
}

var wg sync.WaitGroup
for start := 0; start < len(leaves); start += batchSize {
end := start + batchSize
if end > len(leaves) {
end = len(leaves)
}
wg.Add(1)
go func(leaves []*LeafNode) {
defer wg.Done()
commitLeafNodes(leaves)
}(leaves[start:end])
}
wg.Wait()
}
Comment on lines +1549 to +1574
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so if you remember this is what we called when we collected all the new leaf nodes in (*InternalNode).Commit().

The idea in this method is the following:

  • If we have less than 8 new leaves, avoid spinning up any new goroutine and do the work serially (but batching work; you'll see commitLeafNode(...) below).
  • If we have more than 8 new leaves, we split the work in groups of 8 leaves at a minimum.

The idea of the last point is that even if we have 16 cores and 16 leaves, it doesn't make sense to spin up 16 goroutines since that's a lot of goroutine overhead for the amount of work. What this logic will do is always make sure each goroutine is at least doing work for 8 leaf nodes, so the overhead of spinning up goroutines is well justified.

That's just a "good practice" of not always assuming that you should split up work in the number of cores, but just be sure each one will have enough work to do to justify the scheduling overhead.

Note that "8" is a magic number that I handwaved it considering what's usually the amount of CPU work. This is leaning more into going single core, and going multi-core (up to N cores in you machine) if clearly justified. We can also play moving this number and the rest will accommodate alone.


func commitLeafNodes(leaves []*LeafNode) {
cfg := GetConfig()

c1c2points := make([]*Point, 2*len(leaves))
c1c2frs := make([]*Fr, 2*len(leaves))
for i, n := range leaves {
// C1.
var c1poly [NodeWidth]Fr
count := fillSuffixTreePoly(c1poly[:], n.values[:NodeWidth/2])
containsEmptyCodeHash := len(c1poly) >= EmptyCodeHashSecondHalfIdx &&
c1poly[EmptyCodeHashFirstHalfIdx].Equal(&EmptyCodeHashFirstHalfValue) &&
c1poly[EmptyCodeHashSecondHalfIdx].Equal(&EmptyCodeHashSecondHalfValue)
if containsEmptyCodeHash {
// Clear out values of the cached point.
c1poly[EmptyCodeHashFirstHalfIdx] = FrZero
c1poly[EmptyCodeHashSecondHalfIdx] = FrZero
// Calculate the remaining part of c1 and add to the base value.
partialc1 := cfg.CommitToPoly(c1poly[:], NodeWidth-count-2)
n.c1 = new(Point)
n.c1.Add(&EmptyCodeHashPoint, partialc1)
} else {
n.c1 = cfg.CommitToPoly(c1poly[:], NodeWidth-count)
}

// C2.
var c2poly [NodeWidth]Fr
count = fillSuffixTreePoly(c2poly[:], n.values[NodeWidth/2:])
n.c2 = cfg.CommitToPoly(c2poly[:], NodeWidth-count)

c1c2points[2*i], c1c2points[2*i+1] = n.c1, n.c2
c1c2frs[2*i], c1c2frs[2*i+1] = new(Fr), new(Fr)
}

toFrMultiple(c1c2frs, c1c2points)

var poly [NodeWidth]Fr
poly[0].SetUint64(1)
for i, nv := range leaves {
StemFromBytes(&poly[1], nv.stem)
poly[2] = *c1c2frs[2*i]
poly[3] = *c1c2frs[2*i+1]

nv.commitment = cfg.CommitToPoly(poly[:], 252)
}
}
Comment on lines +1576 to +1620
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what each goroutine (if spinned) will work for their batch. This code isn't new, but a twist/mixture of what we did previously, NewLeafNode and the previous BatchNewLeafNode.


// firstDiffByteIdx will return the first index in which the two stems differ.
// Both stems *must* be different.
func firstDiffByteIdx(stem1 []byte, stem2 []byte) int {
for i := range stem1 {
if stem1[i] != stem2[i] {
return i
}
}
panic("stems are equal")
}
Loading