Skip to content

perf(dedup): batch MinHash shingle hashing into one vectorized pass - #3650

Open
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/minhash-batch
Open

abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/minhash-batch

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

What

Fuzzy entity dedup builds a 128-permutation MinHash sketch of each entity label's character-shingles. _make_minhash folded the shingles in one at a time — a MinHash.update call per shingle — and each call ran the full 128-wide permutation arithmetic on its own: (a * hv + b) % MP & MASK, then an 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 in a build profile (~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_minhash now 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 min is 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 on uint64 multiply wrapping mod 2⁶⁴ (a * hv reaches ~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 with v8 under the same machine load:

build
v8 1.97s
batched 1.30s (−34%)

deduplicate_entities itself drops from ~1.9s to ~1.1s in a profile.

Correctness

  • update_batch produces bit-identical hashvalues to the update loop across 500 randomized trials (varied lengths, unicode, and the empty-input case).
  • The fully built and deduplicated graph of the corpus is byte-identical to 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 clean v8).

🤖 Generated with Claude Code

https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q

`_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
Copilot AI lite review requested due to automatic review settings September 17, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 — impact
  • tests/test_build_merge_dedup_scope.py — impact
  • tests/test_build_merge_hyperedges_and_prune.py — impact
  • tests/test_build_merge_shrink_guard.py — impact
  • tests/test_carried_hyperedge_remap.py — impact
  • tests/test_corrupt_graph_json.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_dedup.py — impact
  • tests/test_dedup_remaps_hyperedges.py — impact
  • tests/test_dedup_survivor_richness.py — impact
  • tests/test_global_graph.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_issue_3472_source_file_collision.py — impact
  • tests/test_minhash.py — impact
  • tests/test_no_dedup_flag.py — impact
  • tests/test_non_string_node_ids.py — impact
  • tests/test_prune_sweeps_orphans.py — impact
  • tests/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).

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