mirror of
https://github.com/anchore/syft.git
synced 2026-08-22 10:03:35 +02:00
perf: make orderedIDSet membership checks constant time (#5178)
orderedIDSet.add scanned the entire existing slice for every inserted ID,
so building a Collection was quadratic in the number of packages sharing an
index key. This is most visible in idsByType: package types are few, so that
index accumulates nearly every package in the SBOM into a single set, and
each Add rescans it.
Keep the ordered slice (ordering and dedup semantics are unchanged) and add
a hash index for membership checks. The index is only built once a set grows
beyond a small threshold, so the many single-element sets held by idsByName
and idsByPath do not pay for a map, while the large idsByType sets get
constant-time lookups.
BenchmarkCollectionAdd (Apple M2 Pro), packages sharing one type:
packages before after speedup
1,000 1.37 ms 0.76 ms 1.8x
10,000 100.31 ms 9.80 ms 10.2x
50,000 2,423.79 ms 34.46 ms 70.3x
The tradeoff is memory: sets past the threshold allocate an index map,
about 14% more bytes at 50,000 packages, with allocation counts unchanged.
Signed-off-by: Oleksandr Vodotiiets <61548316+avodotiiets@users.noreply.github.com>
This commit is contained in:
parent
ed76e96749
commit
21cca30e5b
@ -299,19 +299,56 @@ func (c *Collection) Sorted(types ...Type) (pkgs []Package) {
|
||||
return pkgs
|
||||
}
|
||||
|
||||
// orderedIDSetIndexThreshold is the number of elements beyond which an orderedIDSet maintains a hash
|
||||
// index for membership checks. Below this size a linear scan of the contiguous slice is competitive
|
||||
// with a map lookup and avoids allocating a map for the many small (often single-element) sets held
|
||||
// by the name and path indexes. Above it, the linear scan is what makes bulk insertion quadratic.
|
||||
const orderedIDSetIndexThreshold = 16
|
||||
|
||||
type orderedIDSet struct {
|
||||
slice []artifact.ID
|
||||
|
||||
// index is nil until the set grows beyond orderedIDSetIndexThreshold, after which it is kept in
|
||||
// sync with slice and used to answer membership checks in constant time.
|
||||
index map[artifact.ID]struct{}
|
||||
}
|
||||
|
||||
func (s *orderedIDSet) contains(id artifact.ID) bool {
|
||||
if s.index != nil {
|
||||
_, exists := s.index[id]
|
||||
return exists
|
||||
}
|
||||
|
||||
for _, existingID := range s.slice {
|
||||
if existingID == id {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func (s *orderedIDSet) add(ids ...artifact.ID) {
|
||||
loopNewIDs:
|
||||
for _, newID := range ids {
|
||||
for _, existingID := range s.slice {
|
||||
if existingID == newID {
|
||||
continue loopNewIDs
|
||||
}
|
||||
if s.contains(newID) {
|
||||
continue
|
||||
}
|
||||
|
||||
s.slice = append(s.slice, newID)
|
||||
|
||||
switch {
|
||||
case s.index != nil:
|
||||
s.index[newID] = struct{}{}
|
||||
case len(s.slice) > orderedIDSetIndexThreshold:
|
||||
s.buildIndex()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (s *orderedIDSet) buildIndex() {
|
||||
s.index = make(map[artifact.ID]struct{}, len(s.slice))
|
||||
for _, id := range s.slice {
|
||||
s.index[id] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
@ -319,6 +356,7 @@ func (s *orderedIDSet) delete(id artifact.ID) {
|
||||
for i, existingID := range s.slice {
|
||||
if existingID == id {
|
||||
s.slice = append(s.slice[:i], s.slice[i+1:]...)
|
||||
delete(s.index, id) // no-op when the index has not been built
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
@ -2,6 +2,7 @@ package pkg
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/scylladb/go-set/strset"
|
||||
@ -482,3 +483,73 @@ func Test_idOrderedSet_add(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_idOrderedSet_addBeyondIndexThreshold(t *testing.T) {
|
||||
// exercise the indexed path: sets larger than orderedIDSetIndexThreshold must still deduplicate
|
||||
// and retain insertion ordering
|
||||
const count = orderedIDSetIndexThreshold * 4
|
||||
|
||||
var expected []artifact.ID
|
||||
var input []artifact.ID
|
||||
for i := 0; i < count; i++ {
|
||||
id := artifact.ID(fmt.Sprintf("id-%d", i))
|
||||
expected = append(expected, id)
|
||||
input = append(input, id)
|
||||
}
|
||||
// add every element a second time; none of these should be appended
|
||||
input = append(input, expected...)
|
||||
|
||||
var s orderedIDSet
|
||||
s.add(input...)
|
||||
|
||||
assert.Equal(t, expected, s.slice)
|
||||
require.NotNil(t, s.index, "index should be built once the set grows beyond the threshold")
|
||||
assert.Len(t, s.index, len(expected), "index and slice should agree")
|
||||
}
|
||||
|
||||
func Test_idOrderedSet_deleteKeepsIndexInSync(t *testing.T) {
|
||||
const count = orderedIDSetIndexThreshold * 2
|
||||
|
||||
var s orderedIDSet
|
||||
for i := 0; i < count; i++ {
|
||||
s.add(artifact.ID(fmt.Sprintf("id-%d", i)))
|
||||
}
|
||||
require.NotNil(t, s.index)
|
||||
|
||||
removed := artifact.ID("id-0")
|
||||
s.delete(removed)
|
||||
|
||||
assert.Len(t, s.slice, count-1)
|
||||
assert.NotContains(t, s.slice, removed)
|
||||
assert.False(t, s.contains(removed), "deleted element must not remain in the index")
|
||||
|
||||
// re-adding must work (it would be skipped if the index still held a stale entry)
|
||||
s.add(removed)
|
||||
assert.Equal(t, removed, s.slice[len(s.slice)-1])
|
||||
}
|
||||
|
||||
func BenchmarkCollectionAdd(b *testing.B) {
|
||||
// packages sharing a type all land in the same idsByType set, which is the case that previously
|
||||
// made insertion quadratic
|
||||
for _, count := range []int{1_000, 10_000, 50_000} {
|
||||
pkgs := make([]Package, count)
|
||||
for i := range pkgs {
|
||||
pkgs[i] = Package{
|
||||
Name: fmt.Sprintf("package-%d", i),
|
||||
Version: "1.0.0",
|
||||
Type: NpmPkg,
|
||||
}
|
||||
pkgs[i].SetID()
|
||||
}
|
||||
|
||||
b.Run(fmt.Sprintf("packages=%d", count), func(b *testing.B) {
|
||||
b.ReportAllocs()
|
||||
for i := 0; i < b.N; i++ {
|
||||
c := NewCollection()
|
||||
for _, p := range pkgs {
|
||||
c.Add(p)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user