Skip to content

perf(iavlx): use NodeID and absolute offsets in branch layout #25482

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: aaronc/iavlx
Choose a base branch
from
+114 −142
Open

perf(iavlx): use NodeID and absolute offsets in branch layout #25482

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aa26a92
i think thats everything? i guess?
technicallyty Oct 21, 2025
ca2d85a
fix struct alignment
technicallyty Oct 21, 2025
47c029a
remove leafOffsetRemappings and add a comment to offsetCache
technicallyty Oct 21, 2025
d4d932e
branch persisted removals and method removals
technicallyty Oct 21, 2025
e85a1aa
fully remove leafOffsetRemapping code
technicallyty Oct 21, 2025
8585d2b
fix
technicallyty Oct 21, 2025
09e0de9
fix resolve
technicallyty Oct 23, 2025
93aa22c
some comments
technicallyty Oct 23, 2025
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
8 changes: 5 additions & 3 deletions iavl/branch_layout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ func init() {
}

const (
SizeBranch = 72
SizeBranch = 88
)

type BranchLayout struct {
Id NodeID
Left NodeRef
Right NodeRef
Left NodeID
LeftOffset uint32 // absolute offset
Right NodeID
RightOffset uint32 // absolute offset
Copy link
Member

@aaronc aaronc Oct 21, 2025

Choose a reason for hiding this comment

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

Ah so I can see how this snippet looks totally equivalent to what I posted on slack! But the order actually matters and affects the struct size. So when we place NodeID (8 bytes) next to a uint32 (4 bytes), the compiler actually makes it take up 16 bytes with 4 bytes of padding. And if we do this twice, we use 32 bytes of space. If we put the offsets together, there is no extra padding and the same data only uses 24 bytes. If you're not too familiar with struct alignment and padding, there a bunch of articles you can find by googling.

Suggested change
SizeBranch = 88
)
type BranchLayout struct {
Id NodeID
Left NodeRef
Right NodeRef
Left NodeID
LeftOffset uint32 // absolute offset
Right NodeID
RightOffset uint32 // absolute offset
SizeBranch = 80
)
type BranchLayout struct {
Id NodeID
Left NodeID
Right NodeID
LeftOffset uint32 // absolute offset
RightOffset uint32 // absolute offset

Copy link
Contributor Author

@technicallyty technicallyty Oct 21, 2025

Choose a reason for hiding this comment

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

fix struct alignment

KeyOffset uint32
Height uint8
Size uint32 // TODO 5 bytes?
Expand Down
12 changes: 10 additions & 2 deletions iavl/changeset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ func (cr *Changeset) resolveBranchWithIdx(nodeId NodeID, fileIdx uint32) (Branch
}
}

func (cr *Changeset) resolveNodeID(id NodeID) *NodePointer {
return &NodePointer{
id: id,
store: cr.treeStore.getChangesetForVersion(uint32(id.Version())),
}
}

// TODO(technicallyty): remove this?
func (cr *Changeset) resolveNodeRef(nodeRef NodeRef, selfIdx uint32) *NodePointer {
if nodeRef.IsNodeID() {
id := nodeRef.AsNodeID()
Expand Down Expand Up @@ -231,8 +239,8 @@ func (cr *Changeset) Resolve(nodeId NodeID, fileIdx uint32) (Node, error) {
return nil, err
}

leftPtr := cr.resolveNodeRef(layout.Left, actualIdx)
rightPtr := cr.resolveNodeRef(layout.Right, actualIdx)
leftPtr := cr.resolveNodeID(layout.Left)
rightPtr := cr.resolveNodeID(layout.Right)

return &BranchPersisted{
layout: layout,
Expand Down
24 changes: 18 additions & 6 deletions iavl/changeset_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,27 @@
}
}

// now write parent
parentIdx := int64(cs.branchesData.Count() + 1) // +1 to account for the node being written
leftRef := cs.createNodeRef(parentIdx, node.left)
rightRef := cs.createNodeRef(parentIdx, node.right)
leftVersion := node.left.id.Version()
rightVersion := node.right.id.Version()

var leftOffset uint32
var rightOffset uint32

// If the child node is in the same changeset, store its 1-based file offset.
// fileIdx is already 1-based (set to Count() after append), and 0 means no offset.
if leftVersion >= uint64(cs.StartVersion()) {
leftOffset = node.left.fileIdx
}
if rightVersion >= uint64(cs.StartVersion()) {
rightOffset = node.right.fileIdx
}

layout := BranchLayout{
Id: np.id,
Left: leftRef,
Right: rightRef,
Left: node.left.id,
Right: node.right.id,
LeftOffset: leftOffset,
RightOffset: rightOffset,
KeyOffset: keyOffset,
Height: node.height,
Size: uint32(node.size), // TODO check overflow
Expand Down Expand Up @@ -217,7 +229,7 @@
func (cs *ChangesetWriter) createNodeRef(parentIdx int64, np *NodePointer) NodeRef {
if np.store == cs.reader {
if np.id.IsLeaf() {
//return NodeRef(np.id)

Check failure on line 232 in iavl/changeset_writer.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

commentFormatting: put a space between `//` and comment text (gocritic)

Check failure on line 232 in iavl/changeset_writer.go

View workflow job for this annotation

GitHub Actions / Analyze 

commentFormatting: put a space between `//` and comment text (gocritic)
return NodeRef(NewNodeRelativePointer(true, int64(np.fileIdx)))
} else {
// for branch nodes the relative offset is the difference between the parent ID index and the branch ID index
Expand Down
56 changes: 10 additions & 46 deletions iavl/compactor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type Compactor struct {

leafOffsetRemappings map[uint32]uint32
keyCache map[string]uint32
offsetCache map[NodeID]uint32

// Running totals across all processed changesets
leafOrphanCount uint32
Expand Down Expand Up @@ -83,6 +84,7 @@ func NewCompacter(logger *slog.Logger, reader *Changeset, opts CompactOptions, s
versionsWriter: NewStructWriter[VersionInfo](newFiles.versionsFile),
keyCache: make(map[string]uint32),
leafOffsetRemappings: make(map[uint32]uint32),
Copy link
Member

@aaronc aaronc Oct 21, 2025

Choose a reason for hiding this comment

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

Suggested change
leafOffsetRemappings: make(map[uint32]uint32),

We no longer need this

Copy link
Contributor Author

@technicallyty technicallyty Oct 21, 2025

Choose a reason for hiding this comment

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

updated remove leafOffsetRemappings and add a comment to offsetCache

offsetCache: make(map[NodeID]uint32),
}

// Process first changeset immediately
Expand Down Expand Up @@ -162,7 +164,8 @@ func (c *Compactor) processChangeset(reader *Changeset) error {
}

oldLeafFileIdx := leafStartOffset + j
c.leafOffsetRemappings[oldLeafFileIdx] = uint32(c.leavesWriter.Count()) - 1
c.leafOffsetRemappings[oldLeafFileIdx] = uint32(c.leavesWriter.Count())
c.offsetCache[id] = uint32(c.leavesWriter.Count())
}

newBranchStartIdx := uint32(0)
Expand Down Expand Up @@ -191,24 +194,11 @@ func (c *Compactor) processChangeset(reader *Changeset) error {
newBranchEndIdx = id.Index()
newBranchCount++

var err error
left := branch.Left
branch.Left, err = c.updateNodeRef(reader, left, skippedBranches)
if err != nil {
c.logger.Error("failed to update left ref",
"branchId", id,
"branchOrphanVersion", branch.OrphanVersion,
"leftRef", left)
return fmt.Errorf("failed to update left ref for branch %s: %w", id, err)
if newLeftOffset, ok := c.offsetCache[branch.Left]; ok {
branch.LeftOffset = newLeftOffset
}
right := branch.Right
branch.Right, err = c.updateNodeRef(reader, right, skippedBranches)
if err != nil {
c.logger.Error("failed to update right ref",
"branchId", id,
"branchOrphanVersion", branch.OrphanVersion,
"rightRef", right)
return fmt.Errorf("failed to update right ref for branch %s: %w", id, err)
if newRightOffset, ok := c.offsetCache[branch.Right]; ok {
branch.RightOffset = newRightOffset
}

if c.compactWAL {
Expand All @@ -229,10 +219,11 @@ func (c *Compactor) processChangeset(reader *Changeset) error {
branch.KeyOffset += kvOffsetDelta
}

err = c.branchesWriter.Append(&branch)
err := c.branchesWriter.Append(&branch)
if err != nil {
return fmt.Errorf("failed to append branch %s: %w", id, err)
}
c.offsetCache[id] = uint32(c.branchesWriter.Count())
}

verInfo = VersionInfo{
Expand Down Expand Up @@ -307,33 +298,6 @@ func (c *Compactor) Seal() (*Changeset, error) {
return cs, nil
}

func (c *Compactor) updateNodeRef(reader *Changeset, ref NodeRef, skipped int) (NodeRef, error) {
if ref.IsNodeID() {
return ref, nil
}
relPtr := ref.AsRelativePointer()
if relPtr.IsLeaf() {
oldOffset := relPtr.Offset()
newOffset, ok := c.leafOffsetRemappings[uint32(oldOffset)]
if !ok {
// Debug: look up the orphaned leaf
oldLeaf := reader.leavesData.UnsafeItem(uint32(oldOffset) - 1)
c.logger.Error("leaf remapping failed - orphaned leaf still referenced",
"leafOffset", oldOffset,
"leafId", oldLeaf.Id,
"leafOrphanVersion", oldLeaf.OrphanVersion,
"remappings", c.leafOffsetRemappings)
return 0, fmt.Errorf("failed to find remapping for leaf offset %d", oldOffset)
}
return NodeRef(NewNodeRelativePointer(true, int64(newOffset))), nil
} else {
// branch nodes we reduce by the number of skipped nodes
oldOffset := relPtr.Offset()
newOffset := oldOffset - int64(skipped)
return NodeRef(NewNodeRelativePointer(false, newOffset)), nil
}
}

func (c *Compactor) Abort() error {
err := c.files.Close()
if err != nil {
Expand Down
Loading