Repository navigation
perf(datasets): batch wildcard exclusions in subject set subtraction - #3395
Conversation
github-actions
Bot
added
the
area/tooling
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| } | ||
| return | ||
| } | ||
| // Build the exclusions once instead of copying the growing slice for each subject. |
| for _, otherSubject := range other.AsSlice() { | ||
| bss.Subtract(otherSubject) | ||
| wildcard, hasWildcard := bss.wildcard.get() | ||
| if !hasWildcard || other.wildcard.getOrNil() != nil || len(other.concrete) < 2 { |
There was a problem hiding this comment.
Add a comment explaining here exactly what's being done: BaseSubjectSet is very complex, so we require detailed comments
There was a problem hiding this comment.
Added an explanation of which cases can be batched and why subtracting another wildcard stays on the existing path.
| exclusions := make([]T, 0, len(existingExclusions)+len(other.concrete)) | ||
| matched := make(map[string]struct{}, min(len(existingExclusions), len(other.concrete))) | ||
| for _, exclusion := range existingExclusions { | ||
| if removing, ok := other.concrete[exclusion.GetSubjectId()]; ok { |
There was a problem hiding this comment.
Added a caveat example here, including the unconditional case, and explained why we keep both source subjects.
| return cloned | ||
| } | ||
|
|
||
| func BenchmarkSubjectSetSubtractAll(b *testing.B) { |
There was a problem hiding this comment.
Add some pre-defined, readable unit tests (in their own table-driven test)
There was a problem hiding this comment.
Done. Added a table-driven test with the inputs and expected results written out for each case.
2 participants
LookupSubjectsrepeatedly copies a growing exclusion list when subtracting concrete subjects from a wildcard.SubtractAllnow builds that list once, preserving caveat expressions and resource provenance through the existing constructors. This makes exclusion construction linear in the existing exclusions and removed subjects. Wildcard subtraction and zero/one-subject cases retain the existing path.AsSlicealso allocates its known capacity once.Adds differential tests against sequential subtraction, input-mutation and resource-tracking checks, and benchmarks for subject-set subtraction and dispatched wildcard lookups.
Measured through the public gRPC API with PostgreSQL 16.11, Go 1.26.8, and enabled dispatch caches on an Apple M4:
Values are medians across three runs of 60 requests at 10 requests/second, with a fresh server per case. The first case uses fully consistent reads with concurrent writes; its per-run p95 ranges were 75.5–93.0 ms before and 9.7–13.6 ms after. Allocation includes concurrent writes and background work. The controls showed no consistent latency change. These are synthetic graphs on one shared local machine.
Reproduction commands, fixtures, pinned revisions, and raw measurements.
Validation: 106 unit-test packages passed; race checks passed for datasets, development membership, graph, and dispatch caching; golangci-lint reported no issues in the changed packages. All 96 PostgreSQL fixture documents returned the exact expected subjects. Docker-dependent tests were not run because Docker was unavailable locally.
Fixes #3394.