Answer getUniqueBlocks from an inverted picblockhash index (92s -> 0.11s)

[danielplohmann]

getUniqueBlocks decides "is this block hash unique to the requested samples?" by reading every function in the database that carries picblockhashes. On this Malpedia-sized instance (8,691 samples / 11,669,208 functions) that is 9,088,495 documents and 51,364,030 block entries streamed into Python, ~92 s per call — and the cost does not depend on how many samples were asked about, because it scans the corpus either way.

This adds a picblockhashes collection and answers the same question by reading one document per candidate hash.

{_id: "<block hash>", sample_ids: [<sample_id>, ...]}

Measured, on the live corpus, identical results both ways

request	candidates	scan (today)	index
1 sample (win.citadel)	4,095	92–97 s	0.11 s	1 unique block
5 samples	155,849	107 s	2.12 s	59,977 unique blocks

12,139,968 distinct hashes, 0.47 GB + 0.25 GB for the _id index, 245 s to build.

The decisions, since they are the reviewable part

Why this shape. The sample list is the whole question. Function ids and offsets would multiply the document count by the ~5.6 blocks each function averages, and getUniqueBlocks already holds them from the candidate side. Family ids would make a relabel invalidate the index for no gain.

Why not the existing _picblockhashes.hash index instead. It answers the same query and needs no new collection — I measured it as the cheaper option first. But its $in fan-out pulls whole 708-byte function documents, so it degrades as the request grows: 12.7x at one sample, 2.3x at five, against 407x and 37x here, on the same runs. The sample-set case is the one that matters — fkie-cad/mcritweb#140 is about to give Unique Blocks its own page taking a set of samples, where today the only caller passes a list of one.

Why maintaining this on the write path is safe, when #149 and #151 are both bugs in exactly that. The difference is idempotency:

• add uses $addToSet with upsert, which has set semantics — a retried or replayed insert converges instead of drifting. Family statistics drift from the collections they summarise, and /status reports the drifted values as the corpus size #151's family counters use $inc, where a lost or repeated update is permanent and needs a recount to find.
• delete scopes the $pull by _id, taking the hashes from the sample's own functions in one indexed read. So no multikey index on sample_ids is needed — that would have cost 0.24 GB over 12.1M documents. The consequence is that the hook must run before the function documents are deleted, which is why it sits where it does in deleteSample.
• emptied posting lists are deleted rather than left. They would actually read correctly (an empty list means "held by nobody", so the candidate survives), but deleteSample leaves empty band documents behind, and its upsert can create them #149 is that same residue in the band index and there is no reason to repeat it.
• a family reassignment is a no-op, pinned by a test so that a future change adding family_id here has to confront it.
• recalculateAllPicHashes rewrites block hashes wholesale, so it marks the index stale rather than trying to diff old against new per function — which is what the rebuild does anyway, in one pass.

Why a completeness flag rather than "the collection exists". A half-filled index reports blocks as unique that are not — a wrong answer, not a slow one. So the index is read only when a settings flag vouches for it. A database with no functions gets the flag for free (an empty index over an empty corpus is complete), so fresh instances maintain it from the first submit and never need a rebuild. An existing database upgrading into this does not get the flag and keeps using the scan — which is retained, not deleted — until an operator calls the rebuild. Upgrading changes performance, never results. While the flag is unset the write hooks skip their work, since there is no point maintaining an index nothing reads.

clearStorage drops the collection. Leaving it would keep asserting that hashes are held by samples that no longer exist.

Exposure

Worker.rebuildPicBlockHashIndex (@Remote(progress=True), so MinHashIndex forwards it automatically), a GET /rebuild_picblockhash_index route, and McritClient.rebuildPicBlockHashIndex — mirroring rebuild_index / recalculate_pichashes. MemoryStorage gets a return 0 implementation with a comment: it holds every function in a dict already, so its elimination has nothing to index, and the backends stay interchangeable.

Tests

Five new mongo-backed tests, all using two samples built from the same report so they carry identical block hashes:

• index and scan agree — the same getUniqueBlocks call answered both ways, for one sample and for two
• maintained on add and delete, including that a delete leaves no emptied posting lists
• the rebuild reproduces exactly what the write hooks built
• family reassignment leaves the index untouched
• nothing is written while the index is incomplete, and the rebuild recovers it

208 passed, 34 subtests passed in 85.72s
All checks passed!            # ruff check
136 files already formatted   # ruff format --check

What I did not do, and what I could not verify here

• No version bump or changelog entry, to avoid conflicting with the other open PRs — fold into whichever release suits.
• The $out rebuild replaces the collection atomically but holds no lock against concurrent writes; a submit landing mid-rebuild could be missed. It sets the flag afterwards, so the window is "rebuild then submit" and the fix would be to re-run it. Worth a maintainer's opinion on whether that needs more than a docstring.
• Maintenance costs were measured per-operation on a scratch copy (add 0.52 s/sample, delete 0.40 s/sample), not under concurrent load through the real write paths.
• The scale figures come from this one corpus on one machine.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GwSfzAD1ZwEoWY3eWvbYvX

[danielplohmann]

Rebased onto main after #171/#172/#173/#174/#180 landed. All three gates clean on the rebase: 260 passed, ruff check + format, ty 0.0.74.

One conflict needed a judgement rather than a merge. My side of deleteSample was the pre-#174 body, which main has since rewritten twice — #174 made it decrement family stats by what delete_many/delete_one actually removed, and #180 factored the band pull into _pullBandEntries. Taking my side back would have silently reverted both. So I kept main's deleteSample and inserted only the one line this PR needs:

function_minhashes = self._getFunctionMinHashesBySampleId(sample_id)
self._pullBandEntries(function_minhashes)
# drop this sample from the picblockhash index while its functions still exist to be read
self._removeSampleFromPicBlockHashIndex(sample_id)

The ordering constraint from the original PR still holds and is why the hook sits there: it reads the sample's own functions, so it has to run before they are deleted.

The other three conflicts were adjacent-edit collisions where both sides were needed — rebuildPicBlockHashIndex next to #180's repairMinHashes and #174's recomputeFamilyStats in McritClient/StatusResource, and both fake-collection helpers in testMongoDbStorageDeleteSample.

🤖 Generated with Claude Code