perf(unique): make verifyUniqueWithinMutation linear - #9822
Open
shiva-istari wants to merge 2 commits into
Open
Conversation
Fixes #9814. The in-request duplicate check for @unique predicates compared every unique-predicate edge against every other, calling dql.TypeValFrom once per pair: O(N^2) time and allocations in the number of edges per mutation. At 8k edges one check took 1.67s and 64M allocations, dominating batched writes on @unique predicates. Replace the nested scan with a single pass over a seen-map keyed on (predicate, value), remembering the first subject that set each value. Semantics are unchanged: duplicate values from the same subject remain allowed, nil ObjectValues are skipped, entries pruned by updateMutations are still ignored, and the error message is identical. Value identity still uses the interface{} produced by TypeValFrom, so type identity participates in the comparison exactly as it did with ==. Measured (M4 Pro, benchstat over 6 runs, all p=0.002): 5.20ms -> 32.6us at 500 edges, 1.67s -> 585us at 8000 edges (-99.96%); allocs/op drops from N^2 (64M at 8k) to ~N (8k). The after curve doubles per doubling of N, i.e. linear.
There was a problem hiding this comment.
🟡 Changes recommended
The new linear algorithm should be backed by targeted unit tests asserting the core within-mutation @unique semantics to guard against regressions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR optimizes the in-request duplicate check for @unique predicates by replacing the previous quadratic pairwise scan with a linear, map-based pass keyed by (predicate, value), reducing CPU and allocations for large batched mutations.
Changes:
- Introduces a
uniqueValueKeyto represent(predicate, value)identity for within-mutation duplicate detection. - Rewrites
verifyUniqueWithinMutationto track first-seen subjects in aseenmap, making the check O(N) in the number of unique edges. - Preserves prior semantics around same-subject duplicates, nil
ObjectValueskipping, and pruned-mutation handling.
File summaries
| File | Description |
|---|---|
edgraph/server.go |
Replaces O(N²) within-mutation @unique duplicate detection with a single-pass seen-map keyed by (predicate, value). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
matthewmcneely
requested changes
Sep 3, 2026
…d tests
Review catch: uniqueValueKey held whatever dql.TypeValFrom returned,
and five of its branches return slice types ([]byte for
bytes/geo/datetime/bigfloat, []float32 for vfloat) - hashing one
panics, there is no recover on the mutation path, and the chunker makes
it reachable from a plain JSON mutation ("[1.0, 2.0]" on a string
@unique predicate parses as Vfloat32Val before the schema is
consulted). Worse than the old code, whose == comparison only ran once
two edges shared a predicate.
Slice values are now keyed by exact byte content
(string(v) / FloatArrayAsBytes), and types.TypeID joins the key so
equal bytes of different types never collide. The previous code
panicked on any two same-predicate slice values, so content equality
replaces a crash rather than changing working behavior.
Tests added as requested, next to the existing bounds checks:
- TestVerifyUniqueWithinMutationSemantics: different-subject duplicate
rejected with the exact established error message; same-subject
repeats, distinct values/predicates/types, nil ObjectValues and
cross-mutation duplicates in one request.
- TestVerifyUniqueWithinMutationNonScalarValues: panic regression
driving the reviewer's JSON repro through the real chunker (guarded
against going vacuous), plus []byte content equality and
string-vs-equal-bytes non-collision. Verified to panic with "hash of
unhashable type: []float32" on the previous commit.
Perf holds: 38us @500 edges to 651us @8k, growth 2.0x per doubling
(linear); still -99.96% vs the O(N^2) code at 8k edges.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9814.
The in-request duplicate check for @unique predicates compared every unique-predicate edge against every other, calling dql.TypeValFrom once per pair: O(N^2) time and allocations in the number of edges per mutation. At 8k edges one check took 1.67s and 64M allocations, dominating batched writes on @unique predicates.
Replace the nested scan with a single pass over a seen-map keyed on (predicate, value), remembering the first subject that set each value. Semantics are unchanged: duplicate values from the same subject remain allowed, nil ObjectValues are skipped, entries pruned by updateMutations are still ignored, and the error message is identical. Value identity still uses the interface{} produced by TypeValFrom, so type identity participates in the comparison exactly as it did with ==.
Measured (M4 Pro, benchstat over 6 runs, all p=0.002): 5.20ms -> 32.6us at 500 edges, 1.67s -> 585us at 8000 edges (-99.96%); allocs/op drops from N^2 (64M at 8k) to ~N (8k).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.