Skip to content

Support aggregate size computations over list, maps, and structs - #1404

Open
TristonianJones wants to merge 1 commit into
cel-expr:masterfrom
TristonianJones:aggregate-size
Open

Support aggregate size computations over list, maps, and structs#1404
TristonianJones wants to merge 1 commit into
cel-expr:masterfrom
TristonianJones:aggregate-size

Conversation

@TristonianJones

@TristonianJones TristonianJones commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

To support best cost tracking, introduce version-aware AggregateSize methods
on list, map, and object types.

The implementations currently memoize aggregate size computations
for immutable objects. The aggregate size is capped at uint32 to minimize
the allocation overhead associated with the book-keeping fields on these
objects.

As far as possible, the allocation cost associated computing the size
list, maps, and structs has been reduced to a minimum. The purpose of
having different, or versioned, AggregateSize implementations is to allow
users to tailor which kinds of costs they're willing to consider before giving
up and assigning a maximum size.

  1. Amortized / Repeated Calculation (Memoized vs Unmemoized $O(N)$ Traversal)
Value Type Implementation $N = 10$ $N = 100$ $N = 1000$ Complexity Class
builtin_list Built-in (baseList) 4.81 ns 4.82 ns 4.81 ns $O(1)$ Constant
builtin_map Built-in (baseMap) 5.01 ns 5.02 ns 5.02 ns $O(1)$ Constant
custom_list Custom (traits.Lister) 211.20 ns 1,963.00 ns 19,812.00 ns $O(N)$ Linear
custom_map Custom (traits.Mapper) 1,079.00 ns 10,884.00 ns 127,415.00 ns $O(N)$ Linear
  1. First-Time / Uncached Construction & Calculation
Value Type $N = 10$ $N = 100$ $N = 1000$
builtin_list 222.0 ns (5 allocs) 1,958.0 ns (5 allocs) 19,830.0 ns (5 allocs)
custom_list 416.7 ns (7 allocs) 3,913.0 ns (7 allocs) 37,500.0 ns (7 allocs)
builtin_map 1,043.0 ns (14 allocs) 12,953.0 ns (138 allocs) 145,100.0 ns (1,338 allocs)
custom_map 2,094.0 ns (29 allocs) 23,677.0 ns (288 allocs) 277,112.0 ns (2,838 allocs)
  1. Tree Nesting Complexity ($W^D$ Total Nodes)
Test Case Nesting Depth & Width Total Node Count Time per Op Memory Allocs
builtin_nested_list Depth 2, Width 10 110 nodes 4.81 ns 0 B/op, 0 allocs
custom_nested_list Depth 2, Width 10 110 nodes 2,364.00 ns 704 B/op, 22 allocs
builtin_nested_list Depth 3, Width 10 1,110 nodes 4.81 ns 0 B/op, 0 allocs
custom_nested_list Depth 3, Width 10 1,110 nodes 24,773.00 ns 7,744 B/op, 242 allocs

Comment thread common/types/overflow.go
return uint32(n)
}

func safeUint32FromBoxedInt(v Int) uint32 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you want to clamp the negatives? (-1 will return MaxUint32)

@TristonianJones TristonianJones Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure if you pass in a negative it's not intentional, and I should flip it to max as there's probably a bug. Thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should at least make this consistent one way or the other (safeUint32FromInt above clamps negatives).

As a general utility -- I'd think clamping to 0 is the common behavior but returning max uint32 is just fine to signal fail-closed for incorrect cost computation.

Comment thread common/types/util.go
case Bytes:
return safeUint32FromInt(len(v))
case traits.Sizer:
return safeUint32FromBoxedInt(v.Size().(Int))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to check for int type first? (Sizer's Size() returns ref.Val).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size is always an Int. At some point I was worried about Unknown and Err types, but the function guards prevent these values from ever being extracted from objects now -- they didn't in the early days and by then it was too late to change the interface.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually wondering about custom structs implementing Sizer and downstreams accidentally returning Uint (this is pretty contrived, feel free to just ack)

Comment thread common/types/list.go
Comment thread common/types/util.go
return total
}

func getReflectValueAggregateSize(s AggregateSizer, fieldVal reflect.Value) uint32 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about adding a guard for cyclic structs? (In case if the host application passes one in)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding a budget and depth make a lot of sense. I have this drafted, to a degree, in a memory limit PR, but let me move it over here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants