Skip to content

perf(expr): pre-size literal sets - #1812

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:perf/pre-size-literal-sets
Aug 14, 2026
Merged

perf(expr): pre-size literal sets#1812
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:perf/pre-size-literal-sets

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Pre-size the literal set map from the input literal count.
  • Pre-size the typed literal set rebuilt during predicate binding.
  • Add benchmarks for 8, 64, 1,024, and 8,192 value IN sets.

Why

Large IN predicates currently build maps from an empty capacity. The maps grow and rehash while the literals are inserted, and binding builds a second set with the same issue.

Performance

For 8,192 IN values on an Apple M1 Pro, using three 250 ms benchmark runs:

  • Literal set construction: 1.31 MB/op to 656 KB/op, about 50% less memory.
  • Predicate binding: 1.47 MB/op to 819 KB/op, about 44% less memory.
  • Predicate binding was about 3x faster in the local median run.

The exact CPU result depends on the literal count and workload. The allocation reduction is the main benefit.

Tests

  • go test .
  • go test -race .
  • go test ./...

@fallintoplace
fallintoplace marked this pull request as ready for review August 13, 2026 12:09

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Straightforward and correct change — the map capacity hints are pure upper bounds (duplicates collapsing after the To(boundType) cast in createBoundSetPredicate just means slight over-allocation, never incorrect behavior). Switching typedSet in exprs.go:953 from the Set[Literal] interface returned by newLiteralSet() to the concrete literalSet is behaviorally equivalent: all method sets are value receivers, newBoundSetPredicate still receives it as Set[Literal], and the other.(literalSet) assertion in literalSet.Equals still matches since the dynamic type is unchanged. Verified the build, full package tests, and both new benchmarks pass on the PR head; the allocation numbers roughly match the description's claims. Benchmarks are a nice addition for future regression tracking.

@zeroshade
zeroshade merged commit 3a21b19 into apache:main Aug 14, 2026
15 checks passed
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