perf(dedup): batch MinHash shingle hashing into one vectorized pass - #3650
abhay-codes07 wants to merge 1 commit into
Conversation
`_make_minhash` folded each character-shingle into the sketch with a separate `MinHash.update` call, and every call ran the 128-wide permutation arithmetic on its own — `(a * hv + b) % MP & MASK` plus a `np.minimum`, all on 128-element arrays. On graphify's own corpus that's ~200k `update` calls for ~3.8k sketches, and the per-call numpy dispatch on tiny arrays dominated the build phase: `MinHash.update` was the single hottest function at ~0.9s self-time. Add `MinHash.update_batch`, which hashes every shingle, stacks the 32-bit values into one array, and computes the permutations once on an `(S, 128)` array before taking the column-wise minimum. The sketch is the element-wise min over all shingles and `min` is associative, so batching changes nothing about the result — and the `uint64` multiply wraps mod 2**64 exactly as the scalar path does (`a*hv` reaches ~2**93), a wraparound broadcasting preserves element-wise, so the hash values are bit-identical to the per-shingle loop. Verified: `update_batch` produces bit-identical `hashvalues` to the `update` loop across 500 randomized trials (including the empty case), and the fully built+deduplicated graph of the 364-file corpus is byte-identical to v8 (12110 nodes, 24897 edges; same 13 merges — 10 exact, 3 fuzzy). `deduplicate_entities` drops from ~1.9s to ~1.1s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds MinHash.update_batch, which folds many byte-strings into the sketch in a single vectorized (S, 128) permutation pass producing hash values bit-identical to looping update per element, and rewires _make_minhash to use it so shingle hashing no longer pays the per-token Python loop cost. Empty input is a no-op.
No blocking issues surfaced. 7 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 472 functions depend on the 77 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 77 callers, 24 callees - new:
build_merge()— 76 callers, 14 callees - new:
build()— 52 callers, 6 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_prune()— 9 callers, 3 callees - new:
_llm_tiebreak()— 1 callers, 10 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 472 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 241 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
18 of 286 test file(s) selected (6%) via static blast radius.
tests/test_build.py— impacttests/test_build_merge_dedup_scope.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_corrupt_graph_json.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.py— impacttests/test_dedup_survivor_richness.py— impacttests/test_global_graph.py— impacttests/test_go_qualified_resolution.py— impacttests/test_issue_3472_source_file_collision.py— impacttests/test_minhash.py— impacttests/test_no_dedup_flag.py— impacttests/test_non_string_node_ids.py— impacttests/test_prune_sweeps_orphans.py— impacttests/test_unverified_semantic_shrink.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_make\_minhash.
The verifier did not have enough to check \_make\_minhash, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
· 7 more finding(s) on lines outside this diff (see the check run).
What
Fuzzy entity dedup builds a 128-permutation MinHash sketch of each entity label's character-shingles.
_make_minhashfolded the shingles in one at a time — aMinHash.updatecall per shingle — and each call ran the full 128-wide permutation arithmetic on its own:(a * hv + b) % MP & MASK, then annp.minimum, all on 128-element arrays.On graphify's own corpus that's ~200k
updatecalls for ~3.8k sketches, and the per-call numpy dispatch on tiny arrays dominated the build phase —MinHash.updatewas the single hottest function in abuildprofile (~0.9s self-time).How
Add
MinHash.update_batch(values)(the same method name datasketch uses): hash every shingle, stack the 32-bit values into one array, compute the permutations once on an(S, 128)array, and take the column-wise minimum._make_minhashnow makes a single batched call.This is a pure speedup, not an approximation. The sketch is defined as the element-wise minimum of every shingle's permuted hash, and
minis associative and commutative — so folding all shingles at once, in any order, gives the same sketch as folding them one by one. The one subtlety is that the scalar path relies onuint64multiply wrapping mod 2⁶⁴ (a * hvreaches ~2⁹³ before the% MP); numpy broadcasting preserves that wraparound element-wise, so the batched math is bit-identical, not merely close.Measured
Build phase of graphify's own 364-file corpus,
build([...]), best-of-3, back-to-back withv8under the same machine load:v8deduplicate_entitiesitself drops from ~1.9s to ~1.1s in a profile.Correctness
update_batchproduces bit-identicalhashvaluesto theupdateloop across 500 randomized trials (varied lengths, unicode, and the empty-input case).v8: 12110 nodes, 24897 edges, the same 13 merges (10 exact, 3 fuzzy).Tests
The dedup / minhash / lsh / build / merge / cluster suites pass (the one failure on this machine —
test_merge_chunks_accepts_unicode_id— fails identically on cleanv8).🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q