Skip to content

Commit 38e50cd

Browse files
perf(hnsw): deduplicate neighbor updates and fix error handling in insertHelper
Deduplicate addNeighbors calls by iterating over inboundEdgesAllLayersMap keys instead of nnUidArray, which could contain duplicate UIDs when the same neighbor appears across multiple layers. Also fix two bugs: (1) err from addNeighbors for the inserted node was checked after the neighbor loop instead of immediately, and (2) searchPersistentLayer errors were silently swallowed by returning the uninitialized layerErr variable instead of err. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 4120211 commit 38e50cd

1 file changed

Lines changed: 7 additions & 14 deletions

File tree

tok/hnsw/persistent_hnsw.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -593,8 +593,6 @@ func (ph *persistentHNSW[T]) insertHelper(ctx context.Context, tc *TxnCache,
593593
// var nns []persistentHeapElement[T]
594594
visited := []persistentHeapElement[T]{}
595595
inLevel := getInsertLayer(ph.maxLevels) // calculate layer to insert node at (randomized every time)
596-
var layerErr error
597-
598596
for level := range inLevel {
599597
// perform insertion for layers [level, max_level) only, when level < inLevel just find better start
600598
err := ph.getVecFromUid(entry, tc, &startVec)
@@ -617,7 +615,6 @@ func (ph *persistentHNSW[T]) insertHelper(ctx context.Context, tc *TxnCache,
617615

618616
var outboundEdgesAllLayers = make([][]uint64, ph.maxLevels)
619617
var inboundEdgesAllLayersMap = make(map[uint64][][]uint64)
620-
nnUidArray := []uint64{}
621618
for level := inLevel; level < ph.maxLevels; level++ {
622619
err := ph.getVecFromUid(entry, tc, &startVec)
623620
if err != nil {
@@ -626,38 +623,34 @@ func (ph *persistentHNSW[T]) insertHelper(ctx context.Context, tc *TxnCache,
626623
layerResult, err := ph.searchPersistentLayer(tc, level, entry, startVec,
627624
inVec, false, ph.efConstruction, index.AcceptAll[T])
628625
if err != nil {
629-
return []persistentHeapElement[T]{}, []*index.KeyValue{}, layerErr
626+
return []persistentHeapElement[T]{}, []*index.KeyValue{}, err
630627
}
631628

632629
entry = layerResult.bestNeighbor().index
633630

634631
nns := layerResult.neighbors
635632
for i := range nns {
636-
nnUidArray = append(nnUidArray, nns[i].index)
637633
if inboundEdgesAllLayersMap[nns[i].index] == nil {
638634
inboundEdgesAllLayersMap[nns[i].index] = make([][]uint64, ph.maxLevels)
639635
}
640636
inboundEdgesAllLayersMap[nns[i].index][level] =
641637
append(inboundEdgesAllLayersMap[nns[i].index][level], inUuid)
642-
// add nn to outboundEdges.
643-
// These should already be correctly ordered.
644638
outboundEdgesAllLayers[level] =
645639
append(outboundEdgesAllLayers[level], nns[i].index)
646640
}
647641
}
648642
edge, err := ph.addNeighbors(ctx, tc, inUuid, outboundEdgesAllLayers)
649-
for i := range nnUidArray {
650-
edge, err := ph.addNeighbors(
651-
ctx, tc, nnUidArray[i], inboundEdgesAllLayersMap[nnUidArray[i]])
643+
if err != nil {
644+
return []persistentHeapElement[T]{}, []*index.KeyValue{}, err
645+
}
646+
edges = append(edges, edge)
647+
for nnUid, inboundEdges := range inboundEdgesAllLayersMap {
648+
edge, err := ph.addNeighbors(ctx, tc, nnUid, inboundEdges)
652649
if err != nil {
653650
return []persistentHeapElement[T]{}, []*index.KeyValue{}, err
654651
}
655652
edges = append(edges, edge)
656653
}
657-
if err != nil {
658-
return []persistentHeapElement[T]{}, []*index.KeyValue{}, err
659-
}
660-
edges = append(edges, edge)
661654

662655
return visited, edges, nil
663656
}

0 commit comments

Comments
 (0)