diff --git a/query/outputnode.go b/query/outputnode.go index 64d9ce33c0a..df03b4e1bfd 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -120,7 +120,19 @@ type node struct { child *node } -var nodeSize = int(unsafe.Sizeof(node{})) +var ( + nodeSize = int(unsafe.Sizeof(node{})) + nodeAlign = int(unsafe.Alignof(node{})) +) + +// init enforces that nodeSize is a multiple of nodeAlign so that consecutive +// bump-allocations from a base-aligned buffer all return aligned slices. This +// is what makes the AllocateAligned-free fast path in newNode safe. +func init() { + if nodeSize%nodeAlign != 0 { + panic("query: nodeSize must be a multiple of nodeAlign") + } +} func newEncoder() *encoder { idSlice := make([]string, 1) @@ -300,8 +312,18 @@ type fastJsonNode *node // newNode returns a fastJsonNode with its attr set to attr, // and all other meta set to their default value. +// +// We use Allocate (no zeroing) rather than AllocateAligned because: +// 1. The underlying allocator buffers come from z.Calloc and are zero-initialized. +// 2. The allocator is freshly created per encoder (not pooled) and never reuses +// space within its lifetime (Allocate is a strict-forward bump allocator). +// 3. nodeSize (24 on 64-bit) is a multiple of nodeAlign — see init() — so +// consecutive Allocate(nodeSize) calls preserve alignment. +// +// Skipping the per-node memclr eliminates the runtime.memclrNoHeapPointers +// hotspot (66% of CPU in BenchmarkFastJsonNode2Chilren). func (enc *encoder) newNode(attr uint16) fastJsonNode { - b := enc.alloc.AllocateAligned(nodeSize) + b := enc.alloc.Allocate(nodeSize) n := (*node)(unsafe.Pointer(&b[0])) enc.setAttr(n, attr) return n diff --git a/query/outputnode_test.go b/query/outputnode_test.go index 19e7e1f6c2b..2bf7d861528 100644 --- a/query/outputnode_test.go +++ b/query/outputnode_test.go @@ -14,6 +14,7 @@ import ( "sync" "testing" "time" + "unsafe" "github.com/stretchr/testify/require" @@ -166,6 +167,44 @@ func TestFastJsonNode(t *testing.T) { require.Equal(t, fj2, enc.children(fj)) } +// TestNewNodeZeroInit guards the safety invariant for newNode's use of +// z.Allocator.Allocate (which does not zero memory) instead of +// AllocateAligned. The allocator is backed by z.Calloc-initialized buffers +// that come up zero, and the bump allocator never re-hands-out a slot +// within an encoder's lifetime, so every node returned by newNode must +// have only the attr bits set in meta and zero next/child pointers. +func TestNewNodeZeroInit(t *testing.T) { + enc := newEncoder() + const N = 4096 + for i := 1; i <= N; i++ { + attr := uint16(i) + fj := enc.newNode(attr) + require.Equal(t, attr, enc.getAttr(fj), + "newNode #%d: attr round-trip", i) + require.Equal(t, uint64(attr)<<40, fj.meta, + "newNode #%d: meta must contain only attr bits, no leftover state", i) + require.Nil(t, fj.next, + "newNode #%d: next must be nil for a freshly allocated node", i) + require.Nil(t, fj.child, + "newNode #%d: child must be nil for a freshly allocated node", i) + } +} + +// TestNewNodeAlignment guards the alignment invariant that lets newNode +// skip AllocateAligned: every node returned from a sequence of newNode +// calls must satisfy unsafe.Alignof(node{}), so subsequent typed pointer +// access is safe. +func TestNewNodeAlignment(t *testing.T) { + enc := newEncoder() + want := uintptr(unsafe.Alignof(node{})) + for i := 0; i < 1024; i++ { + fj := enc.newNode(uint16(i + 1)) + addr := uintptr(unsafe.Pointer(fj)) + require.Zerof(t, addr&(want-1), + "node #%d at %x is not %d-byte aligned", i, addr, want) + } +} + func BenchmarkFastJsonNodeEmpty(b *testing.B) { for i := 0; i < b.N; i++ { enc := newEncoder()