ostiwe/go-toml
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

AST Tweaks (#551)

Browse Source 
* Use pointers instead of copying around ast.Node

Node is a 56B struct that is constantly in the hot path. Passing nodes
around by copy had a cost that started to add up. This change replaces
them by pointers. Using unsafe pointer arithmetic and converting
sibling/child indexes to relative offsets, it removes the need to carry
around a pointer to the root of the tree. This saves 8B per Node. This
space will be used to store an extra []byte slice to provide contextual
error handling on all nodes, including the ones whose data is different
than the raw input (for example: strings with escaped characters), while
staying under the size of a cache line.

* Remove conditional

* Add Raw to track range in data for parsed values

* Simplify reference tracking
This commit is contained in:
Thomas Pelletier
2021-06-03 21:48:51 -04:00
committed by GitHub
parent f3bb20ea79
commit 618f0181ac
13 changed files with 239 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/ast/ast.go
+37 -29
View File
@@ -2,6 +2,9 @@ package ast
import (
"fmt"
"unsafe"
"github.com/pelletier/go-toml/v2/internal/danger"
)
// Iterator starts uninitialized, you need to call Next() first.
@@ -14,7 +17,7 @@ import (
// }
type Iterator struct {
started bool
node Node
node *Node
}
// Next moves the iterator forward and returns true if points to a node, false
@@ -31,11 +34,11 @@ func (c *Iterator) Next() bool {
// IsLast returns true if the current node of the iterator is the last one.
// Subsequent call to Next() will return false.
func (c *Iterator) IsLast() bool {
return c.node.next <= 0
return c.node.next == 0
}
// Node returns a copy of the node pointed at by the iterator.
func (c *Iterator) Node() Node {
func (c *Iterator) Node() *Node {
return c.node
}
@@ -50,14 +53,13 @@ type Root struct {
func (r *Root) Iterator() Iterator {
it := Iterator{}
if len(r.nodes) > 0 {
it.node = r.nodes[0]
it.node = &r.nodes[0]
}
return it
}
func (r *Root) at(idx int) Node {
// TODO: unsafe to point to the node directly
return r.nodes[idx]
func (r *Root) at(idx Reference) *Node {
return &r.nodes[idx]
}
// Arrays have one child per element in the array.
@@ -69,42 +71,48 @@ func (r *Root) at(idx int) Node {
// children []Node
type Node struct {
Kind Kind
Data []byte // Raw bytes from the input
Raw Range // Raw bytes from the input.
Data []byte // Node value (could be either allocated or referencing the input).
// next idx (in the root array). 0 if last of the collection.
next int
// child idx (in the root array). 0 if no child.
child int
// pointer to the root array
root *Root
// References to other nodes, as offsets in the backing array from this
// node. References can go backward, so those can be negative.
next int // 0 if last element
child int // 0 if no child
}
type Range struct {
Offset uint32
Length uint32
}
// Next returns a copy of the next node, or an invalid Node if there is no
// next node.
func (n Node) Next() Node {
if n.next <= 0 {
return noNode
func (n *Node) Next() *Node {
if n.next == 0 {
return nil
}
return n.root.at(n.next)
ptr := unsafe.Pointer(n)
size := unsafe.Sizeof(Node{})
return (*Node)(danger.Stride(ptr, size, n.next))
}
// Child returns a copy of the first child node of this node. Other children
// can be accessed calling Next on the first child.
// Returns an invalid Node if there is none.
func (n Node) Child() Node {
if n.child <= 0 {
return noNode
func (n *Node) Child() *Node {
if n.child == 0 {
return nil
}
return n.root.at(n.child)
ptr := unsafe.Pointer(n)
size := unsafe.Sizeof(Node{})
return (*Node)(danger.Stride(ptr, size, n.child))
}
// Valid returns true if the node's kind is set (not to Invalid).
func (n Node) Valid() bool {
return n.Kind != Invalid
func (n *Node) Valid() bool {
return n != nil
}
var noNode = Node{}
// Key returns the child nodes making the Key on a supported node. Panics
// otherwise.
// They are guaranteed to be all be of the Kind Key. A simple key would return
@@ -127,13 +135,13 @@ func (n *Node) Key() Iterator {
// Value returns a pointer to the value node of a KeyValue.
// Guaranteed to be non-nil.
// Panics if not called on a KeyValue node, or if the Children are malformed.
func (n Node) Value() Node {
assertKind(KeyValue, n)
func (n *Node) Value() *Node {
assertKind(KeyValue, *n)
return n.Child()
}
// Children returns an iterator over a node's children.
func (n Node) Children() Iterator {
func (n *Node) Children() Iterator {
return Iterator{node: n.Child()}
}
internal/ast/builder.go
+11 -20
View File
@@ -1,12 +1,11 @@
package ast
type Reference struct {
idx int
set bool
}
type Reference int
const InvalidReference Reference = -1
func (r Reference) Valid() bool {
return r.set
return r != InvalidReference
}
type Builder struct {
@@ -18,8 +17,8 @@ func (b *Builder) Tree() *Root {
return &b.tree
}
func (b *Builder) NodeAt(ref Reference) Node {
return b.tree.at(ref.idx)
func (b *Builder) NodeAt(ref Reference) *Node {
return b.tree.at(ref)
}
func (b *Builder) Reset() {
@@ -28,33 +27,25 @@ func (b *Builder) Reset() {
}
func (b *Builder) Push(n Node) Reference {
n.root = &b.tree
b.lastIdx = len(b.tree.nodes)
b.tree.nodes = append(b.tree.nodes, n)
return Reference{
idx: b.lastIdx,
set: true,
}
return Reference(b.lastIdx)
}
func (b *Builder) PushAndChain(n Node) Reference {
n.root = &b.tree
newIdx := len(b.tree.nodes)
b.tree.nodes = append(b.tree.nodes, n)
if b.lastIdx >= 0 {
b.tree.nodes[b.lastIdx].next = newIdx
b.tree.nodes[b.lastIdx].next = newIdx - b.lastIdx
}
b.lastIdx = newIdx
return Reference{
idx: b.lastIdx,
set: true,
}
return Reference(b.lastIdx)
}
func (b *Builder) AttachChild(parent Reference, child Reference) {
b.tree.nodes[parent.idx].child = child.idx
b.tree.nodes[parent].child = int(child) - int(parent)
}
func (b *Builder) Chain(from Reference, to Reference) {
b.tree.nodes[from.idx].next = to.idx
b.tree.nodes[from].next = int(to) - int(from)
}
internal/unsafe/unsafe.go → internal/danger/danger.go
+7 -1
View File
@@ -1,4 +1,4 @@
package unsafe
package danger
import (
"fmt"
@@ -57,3 +57,9 @@ func BytesRange(start []byte, end []byte) []byte {
return start[:l]
}
func Stride(ptr unsafe.Pointer, size uintptr, offset int) unsafe.Pointer {
// TODO: replace with unsafe.Add when Go 1.17 is released
// https://github.com/golang/go/issues/40481
return unsafe.Pointer(uintptr(ptr) + uintptr(int(size)*offset))
}
internal/unsafe/unsafe_test.go → internal/danger/danger_test.go
+19 -9
View File
@@ -1,15 +1,16 @@
package unsafe_test
package danger_test
import (
"testing"
"unsafe"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/pelletier/go-toml/v2/internal/unsafe"
"github.com/pelletier/go-toml/v2/internal/danger"
)
func TestUnsafeSubsliceOffsetValid(t *testing.T) {
func TestSubsliceOffsetValid(t *testing.T) {
examples := []struct {
desc string
test func() ([]byte, []byte)
@@ -28,13 +29,13 @@ func TestUnsafeSubsliceOffsetValid(t *testing.T) {
for _, e := range examples {
t.Run(e.desc, func(t *testing.T) {
d, s := e.test()
offset := unsafe.SubsliceOffset(d, s)
offset := danger.SubsliceOffset(d, s)
assert.Equal(t, e.offset, offset)
})
}
}
func TestUnsafeSubsliceOffsetInvalid(t *testing.T) {
func TestSubsliceOffsetInvalid(t *testing.T) {
examples := []struct {
desc string
test func() ([]byte, []byte)
@@ -72,13 +73,22 @@ func TestUnsafeSubsliceOffsetInvalid(t *testing.T) {
t.Run(e.desc, func(t *testing.T) {
d, s := e.test()
require.Panics(t, func() {
unsafe.SubsliceOffset(d, s)
danger.SubsliceOffset(d, s)
})
})
}
}
func TestUnsafeBytesRange(t *testing.T) {
func TestStride(t *testing.T) {
a := []byte{1, 2, 3, 4}
x := &a[1]
n := (*byte)(danger.Stride(unsafe.Pointer(x), unsafe.Sizeof(byte(0)), 1))
require.Equal(t, &a[2], n)
n = (*byte)(danger.Stride(unsafe.Pointer(x), unsafe.Sizeof(byte(0)), -1))
require.Equal(t, &a[0], n)
}
func TestBytesRange(t *testing.T) {
type fn = func() ([]byte, []byte)
examples := []struct {
desc string
@@ -157,10 +167,10 @@ func TestUnsafeBytesRange(t *testing.T) {
start, end := e.test()
if e.expected == nil {
require.Panics(t, func() {
unsafe.BytesRange(start, end)
danger.BytesRange(start, end)
})
} else {
res := unsafe.BytesRange(start, end)
res := danger.BytesRange(start, end)
require.Equal(t, e.expected, res)
}
})
internal/tracker/key.go
+4 -4
View File
@@ -11,19 +11,19 @@ type KeyTracker struct {
}
// UpdateTable sets the state of the tracker with the AST table node.
func (t *KeyTracker) UpdateTable(node ast.Node) {
func (t *KeyTracker) UpdateTable(node *ast.Node) {
t.reset()
t.Push(node)
}
// UpdateArrayTable sets the state of the tracker with the AST array table node.
func (t *KeyTracker) UpdateArrayTable(node ast.Node) {
func (t *KeyTracker) UpdateArrayTable(node *ast.Node) {
t.reset()
t.Push(node)
}
// Push the given key on the stack.
func (t *KeyTracker) Push(node ast.Node) {
func (t *KeyTracker) Push(node *ast.Node) {
it := node.Key()
for it.Next() {
t.k = append(t.k, string(it.Node().Data))
@@ -31,7 +31,7 @@ func (t *KeyTracker) Push(node ast.Node) {
}
// Pop key from stack.
func (t *KeyTracker) Pop(node ast.Node) {
func (t *KeyTracker) Pop(node *ast.Node) {
it := node.Key()
for it.Next() {
t.k = t.k[:len(t.k)-1]
internal/tracker/seen.go
+4 -4
View File
@@ -104,7 +104,7 @@ func (s *SeenTracker) create(parentIdx int, name []byte, kind keyKind, explicit
// CheckExpression takes a top-level node and checks that it does not contain keys
// that have been seen in previous calls, and validates that types are consistent.
func (s *SeenTracker) CheckExpression(node ast.Node) error {
func (s *SeenTracker) CheckExpression(node *ast.Node) error {
if s.entries == nil {
// s.entries = make([]entry, 0, 8)
// Skip ID = 0 to remove the confusion between nodes whose parent has
@@ -125,7 +125,7 @@ func (s *SeenTracker) CheckExpression(node ast.Node) error {
}
}
func (s *SeenTracker) checkTable(node ast.Node) error {
func (s *SeenTracker) checkTable(node *ast.Node) error {
it := node.Key()
parentIdx := -1
@@ -169,7 +169,7 @@ func (s *SeenTracker) checkTable(node ast.Node) error {
return nil
}
func (s *SeenTracker) checkArrayTable(node ast.Node) error {
func (s *SeenTracker) checkArrayTable(node *ast.Node) error {
it := node.Key()
parentIdx := -1
@@ -207,7 +207,7 @@ func (s *SeenTracker) checkArrayTable(node ast.Node) error {
return nil
}
func (s *SeenTracker) checkKeyValue(node ast.Node) error {
func (s *SeenTracker) checkKeyValue(node *ast.Node) error {
it := node.Key()
parentIdx := s.currentIdx