Skip to content

Commit

Permalink
BAAS-32412: add nil check in mem usage for key and item in builtin_set (
Browse files Browse the repository at this point in the history
  • Loading branch information
Gabri3l authored Jun 26, 2024
1 parent b528e7b commit bb9e63a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 12 deletions.
2 changes: 1 addition & 1 deletion builtin_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func createOrderedMap(vm *Runtime, size int) *orderedMap {
value: value,
}
// These iter items are necessary for testing the mem usage
// estimation since that's who we iterate through the map.
// estimation since that's how we iterate through the map.
if i > 0 {
ht[uint64(i)].iterPrev = ht[uint64(i-1)]
ht[uint64(i-1)].iterNext = ht[uint64(i)]
Expand Down
24 changes: 13 additions & 11 deletions builtin_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,18 +364,20 @@ func (so *setObject) estimateMemUsage(ctx *MemUsageContext) (estimate uint64, er
}
samplesVisited += 1

// We still want to account for both key and value if we return a non-zero value on error.
// This could otherwise skew the estimate when in reality key/value pairs contribute to
// mem usage together.
inc, incErr := item.key.MemUsage(ctx)
memUsage += inc
inc, valErr := item.value.MemUsage(ctx)
memUsage += inc
if valErr != nil {
return computeMemUsageEstimate(memUsage, samplesVisited, totalItems), valErr
if item.key != nil {
inc, err := item.key.MemUsage(ctx)
memUsage += inc
if err != nil {
return computeMemUsageEstimate(memUsage, samplesVisited, totalItems), err
}
}
if incErr != nil {
return computeMemUsageEstimate(memUsage, samplesVisited, totalItems), incErr

if item.value != nil {
inc, err := item.value.MemUsage(ctx)
memUsage += inc
if err != nil {
return computeMemUsageEstimate(memUsage, samplesVisited, totalItems), err
}
}
}

Expand Down
32 changes: 32 additions & 0 deletions builtin_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,28 @@ func TestSetHasFloatVsInt(t *testing.T) {
testScript(SCRIPT, valueTrue, t)
}

func createOrderedMapWithNilValues(size int) *orderedMap {
ht := make(map[uint64]*mapEntry, 0)
for i := 0; i < size; i += 1 {
ht[uint64(i)] = &mapEntry{
key: nil,
value: nil,
}
// These iter items are necessary for testing the mem usage
// estimation since that's how we iterate through the map.
if i > 0 {
ht[uint64(i)].iterPrev = ht[uint64(i-1)]
ht[uint64(i-1)].iterNext = ht[uint64(i)]
}
}
return &orderedMap{
size: size,
iterFirst: ht[uint64(0)],
iterLast: ht[uint64(size-1)],
hashTable: ht,
}
}

func TestSetObjectMemUsage(t *testing.T) {
vm := New()

Expand Down Expand Up @@ -274,6 +296,16 @@ func TestSetObjectMemUsage(t *testing.T) {
(5+SizeString)*20,
errExpected: nil,
},
{
name: "mem above estimate threshold and within memory limit and nil values returns correct mem usage",
mu: NewMemUsageContext(vm, 88, 100, 50, 1, 0.1, TestNativeMemUsageChecker{}),
so: &setObject{
m: createOrderedMapWithNilValues(3),
},
// baseObject
expectedMem: SizeEmptyStruct,
errExpected: nil,
},
{
name: "mem is SizeEmptyStruct given a nil orderedMap object",
mu: NewMemUsageContext(vm, 88, 5000, 50, 50, 0.1, TestNativeMemUsageChecker{}),
Expand Down

0 comments on commit bb9e63a

Please sign in to comment.