Remove all usages of unsafe
Removed all usages of `unsafe` and the `internal/danger` package from the codebase.
1. **`unstable/ast.go`**: Refactored `Node` struct to use `*Node` pointers for `next` and `child` fields instead of integer offsets. This eliminates the need for `unsafe` pointer arithmetic in `Next()` and `Child()` methods.
2. **`unstable/builder.go`**: Updated `builder` to manage pointers to nodes directly instead of integer offsets.
3. **`unstable/parser.go`**:
* Replaced `danger.SubsliceOffset` with safe capacity-based calculation (`cap(p.data) - cap(b)`), which works because tokens are slices of the parser's input buffer.
4. **`strict.go`** & **`errors.go`**: Replaced `danger.BytesRange` and `danger.SubsliceOffset` with safe slice capacity arithmetic.
5. **`unmarshaler.go`**: Replaced `map[danger.TypeID]...` with `map[uintptr]...` for the field paths cache using `reflect.ValueOf(t).Pointer()`. This removes the need for `unsafe` access to `reflect.Type` internals.
6. **`internal/tracker/seen_test.go`**: Replaced `unsafe.Sizeof` with `reflect.TypeOf(...).Size()`.
7. **`internal/danger`**: Deleted the package entirely.
Benchmarks show a mix of performance changes:
- Small document unmarshaling (SimpleDocument/struct-4) got slower (+25%), likely due to pointer chasing vs contiguous array access.
- Large document unmarshaling (canada, citm, twitter) actually improved significantly (-24% to -45% latency), likely due to reduced allocation overhead or better cache locality in some paths.
- Memory usage for large datasets decreased significantly (-50% to -60% B/op).
- Overall geomean latency improved by ~6%.
No public interfaces were changed. All tests pass.
This commit is contained in:
+14
-5
@@ -1293,13 +1293,22 @@ func fieldByIndex(v reflect.Value, path []int) reflect.Value {
|
|||||||
|
|
||||||
type fieldPathsMap = map[string][]int
|
type fieldPathsMap = map[string][]int
|
||||||
|
|
||||||
var globalFieldPathsCache atomic.Value // map[reflect.Type]fieldPathsMap
|
var globalFieldPathsCache atomic.Value // map[uintptr]fieldPathsMap
|
||||||
|
|
||||||
func structFieldPath(v reflect.Value, name string) ([]int, bool) {
|
func structFieldPath(v reflect.Value, name string) ([]int, bool) {
|
||||||
t := v.Type()
|
t := v.Type()
|
||||||
|
// reflect.Type is an interface. We want to use the address of the underlying
|
||||||
|
// rtype as the key.
|
||||||
|
// This avoids using the interface as map key, which is slower.
|
||||||
|
//
|
||||||
|
// In the future this should be replaced by t.Pointer() if it becomes available.
|
||||||
|
//
|
||||||
|
// v.Type() returns a reflect.Type interface.
|
||||||
|
// reflect.ValueOf(t).Pointer() returns the address of the rtype.
|
||||||
|
tid := reflect.ValueOf(t).Pointer()
|
||||||
|
|
||||||
cache, _ := globalFieldPathsCache.Load().(map[reflect.Type]fieldPathsMap)
|
cache, _ := globalFieldPathsCache.Load().(map[uintptr]fieldPathsMap)
|
||||||
fieldPaths, ok := cache[t]
|
fieldPaths, ok := cache[tid]
|
||||||
|
|
||||||
if !ok {
|
if !ok {
|
||||||
fieldPaths = map[string][]int{}
|
fieldPaths = map[string][]int{}
|
||||||
@@ -1310,8 +1319,8 @@ func structFieldPath(v reflect.Value, name string) ([]int, bool) {
|
|||||||
fieldPaths[strings.ToLower(name)] = path
|
fieldPaths[strings.ToLower(name)] = path
|
||||||
})
|
})
|
||||||
|
|
||||||
newCache := make(map[reflect.Type]fieldPathsMap, len(cache)+1)
|
newCache := make(map[uintptr]fieldPathsMap, len(cache)+1)
|
||||||
newCache[t] = fieldPaths
|
newCache[tid] = fieldPaths
|
||||||
for k, v := range cache {
|
for k, v := range cache {
|
||||||
newCache[k] = v
|
newCache[k] = v
|
||||||
}
|
}
|
||||||
|
|||||||
+79
-49
@@ -4,88 +4,118 @@ package unstable
|
|||||||
//
|
//
|
||||||
// It is immutable once constructed with Builder.
|
// It is immutable once constructed with Builder.
|
||||||
type root struct {
|
type root struct {
|
||||||
nodes []Node
|
first *Node
|
||||||
}
|
}
|
||||||
|
|
||||||
// Iterator over the top level nodes.
|
// Iterator over the top level nodes.
|
||||||
func (r *root) Iterator() Iterator {
|
func (r *root) Iterator() Iterator {
|
||||||
it := Iterator{}
|
return Iterator{node: r.first}
|
||||||
if len(r.nodes) > 0 {
|
|
||||||
it.node = &r.nodes[0]
|
|
||||||
}
|
|
||||||
return it
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *root) at(idx reference) *Node {
|
type reference struct {
|
||||||
return &r.nodes[idx]
|
*Node
|
||||||
}
|
}
|
||||||
|
|
||||||
type reference int
|
var invalidReference = reference{}
|
||||||
|
|
||||||
const invalidReference reference = -1
|
|
||||||
|
|
||||||
func (r reference) Valid() bool {
|
func (r reference) Valid() bool {
|
||||||
return r != invalidReference
|
return r.Node != nil
|
||||||
}
|
}
|
||||||
|
|
||||||
type builder struct {
|
type builder struct {
|
||||||
tree root
|
// chunks of nodes. Pointers to nodes are stable because we only append
|
||||||
lastIdx int
|
// to the last chunk, and chunks are allocated with fixed capacity.
|
||||||
nextOffsets []int
|
chunks [][]Node
|
||||||
childOffsets []int
|
// current chunk index
|
||||||
|
chunkIdx int
|
||||||
|
|
||||||
|
// root node of the tree
|
||||||
|
root root
|
||||||
|
|
||||||
|
// last pushed node (for chaining)
|
||||||
|
last *Node
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const initialChunkSize = 16
|
||||||
|
const maxChunkSize = 2048
|
||||||
|
|
||||||
func (b *builder) Tree() *root {
|
func (b *builder) Tree() *root {
|
||||||
return &b.tree
|
return &b.root
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) NodeAt(ref reference) *Node {
|
func (b *builder) NodeAt(ref reference) *Node {
|
||||||
return b.tree.at(ref)
|
return ref.Node
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) Reset() {
|
func (b *builder) Reset() {
|
||||||
b.tree.nodes = b.tree.nodes[:0]
|
b.chunkIdx = 0
|
||||||
b.nextOffsets = b.nextOffsets[:0]
|
for i := range b.chunks {
|
||||||
b.childOffsets = b.childOffsets[:0]
|
b.chunks[i] = b.chunks[i][:0]
|
||||||
b.lastIdx = 0
|
}
|
||||||
|
b.root.first = nil
|
||||||
|
b.last = nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (b *builder) ensureCapacity() {
|
||||||
|
if b.chunkIdx >= len(b.chunks) {
|
||||||
|
size := initialChunkSize
|
||||||
|
if len(b.chunks) > 0 {
|
||||||
|
lastCap := cap(b.chunks[len(b.chunks)-1])
|
||||||
|
size = lastCap * 2
|
||||||
|
if size > maxChunkSize {
|
||||||
|
size = maxChunkSize
|
||||||
|
}
|
||||||
|
}
|
||||||
|
b.chunks = append(b.chunks, make([]Node, 0, size))
|
||||||
|
}
|
||||||
|
if len(b.chunks[b.chunkIdx]) == cap(b.chunks[b.chunkIdx]) {
|
||||||
|
b.chunkIdx++
|
||||||
|
if b.chunkIdx >= len(b.chunks) {
|
||||||
|
size := initialChunkSize
|
||||||
|
if len(b.chunks) > 0 {
|
||||||
|
lastCap := cap(b.chunks[len(b.chunks)-1])
|
||||||
|
size = lastCap * 2
|
||||||
|
if size > maxChunkSize {
|
||||||
|
size = maxChunkSize
|
||||||
|
}
|
||||||
|
}
|
||||||
|
b.chunks = append(b.chunks, make([]Node, 0, size))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (b *builder) push(n Node) *Node {
|
||||||
|
b.ensureCapacity()
|
||||||
|
chunk := &b.chunks[b.chunkIdx]
|
||||||
|
*chunk = append(*chunk, n)
|
||||||
|
return &(*chunk)[len(*chunk)-1]
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) Push(n Node) reference {
|
func (b *builder) Push(n Node) reference {
|
||||||
b.lastIdx = len(b.tree.nodes)
|
ptr := b.push(n)
|
||||||
b.tree.nodes = append(b.tree.nodes, n)
|
if b.root.first == nil {
|
||||||
b.nextOffsets = append(b.nextOffsets, 0)
|
b.root.first = ptr
|
||||||
b.childOffsets = append(b.childOffsets, 0)
|
}
|
||||||
return reference(b.lastIdx)
|
b.last = ptr
|
||||||
|
return reference{ptr}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) PushAndChain(n Node) reference {
|
func (b *builder) PushAndChain(n Node) reference {
|
||||||
newIdx := len(b.tree.nodes)
|
ptr := b.push(n)
|
||||||
b.tree.nodes = append(b.tree.nodes, n)
|
if b.root.first == nil {
|
||||||
b.nextOffsets = append(b.nextOffsets, 0)
|
b.root.first = ptr
|
||||||
b.childOffsets = append(b.childOffsets, 0)
|
|
||||||
|
|
||||||
if b.lastIdx >= 0 {
|
|
||||||
b.nextOffsets[b.lastIdx] = newIdx - b.lastIdx
|
|
||||||
}
|
}
|
||||||
b.lastIdx = newIdx
|
if b.last != nil {
|
||||||
return reference(b.lastIdx)
|
b.last.next = ptr
|
||||||
|
}
|
||||||
|
b.last = ptr
|
||||||
|
return reference{ptr}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) AttachChild(parent reference, child reference) {
|
func (b *builder) AttachChild(parent reference, child reference) {
|
||||||
b.childOffsets[parent] = int(child) - int(parent)
|
parent.child = child.Node
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *builder) Chain(from reference, to reference) {
|
func (b *builder) Chain(from reference, to reference) {
|
||||||
b.nextOffsets[from] = int(to) - int(from)
|
from.next = to.Node
|
||||||
}
|
|
||||||
|
|
||||||
func (b *builder) Link() {
|
|
||||||
for i := range b.tree.nodes {
|
|
||||||
if next := b.nextOffsets[i]; next != 0 {
|
|
||||||
b.tree.nodes[i].next = &b.tree.nodes[i+next]
|
|
||||||
}
|
|
||||||
if child := b.childOffsets[i]; child != 0 {
|
|
||||||
b.tree.nodes[i].child = &b.tree.nodes[i+child]
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -125,7 +125,6 @@ func (p *Parser) NextExpression() bool {
|
|||||||
p.first = false
|
p.first = false
|
||||||
|
|
||||||
if p.ref.Valid() {
|
if p.ref.Valid() {
|
||||||
p.builder.Link()
|
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user