[medium] perf: batch per-attribute sightings lookup in DecayingModelBase::computeCurrentScore

[elhoim]

BLUF — Decay scoring issues one sightings query per attribute; this PR fetches the timestamps per batch.

• Problem — When decay scores are requested, the per-attribute pipelines in MispAttribute::fetchAttributes and Event::fetchEvent issue one sightings SELECT per attribute per associated decaying model just to find that attribute's most recent sighting — one query per attribute across a 50000-row batch page.
• Fix — Adds a grouped lookup in Sighting so last-sighting timestamps are fetched once per batch page or per event, and threads them into the $last_sighting_timestamp parameter that DecayingModelBase::computeCurrentScore already declares but no caller ever passed.
• Effect — Instances using includeDecayScore in restSearch, event exports or CSV export see the sightings query count collapse from per-attribute to per-page, with identical scores.

What

When a decay score is requested, the per-attribute pipelines of MispAttribute::fetchAttributes and Event::fetchEvent issue one sightings SELECT per attribute per associated decaying model, purely to find that attribute's most recent sighting. This PR replaces those with one grouped query per batch page (or per event) and threads the result down through the $last_sighting_timestamp parameter that DecayingModelBase::computeCurrentScore already declares but that no caller has ever passed.

Why it is hot

Tier: T1-conditional, and I want to state the gate plainly rather than sell it. The path only runs when includeDecayScore is set and at least one enabled decaying model is associated with the attribute's type. Both halves are opt-in: includeDecayScore defaults to 0 (app/Controller/EventsController.php:49) and decaying_models.enabled defaults to 0 (INSTALL/MYSQL.sql:364). With no associated models, $models is empty (app/Model/DecayingModel.php:628-639) and the loop at :640 never runs, so no query is issued at all.

It is nevertheless a caller-requested per-request export option in the same class as includeCorrelations, reachable from restSearch/index (EventsController.php:1362-1364, :1822-1823), from the CSV export via enable_decaying (app/Lib/Export/CsvExport.php:41), from Event::fetchEvent (app/Model/Event.php:3431) and from DecayingModelController.php:811. When it is requested, the block executes once per attribute across a 50000-row batch page (app/Model/MispAttribute.php:2293 sets $params['limit'] = 50000; the per-attribute foreach opens at :2348 and the decay block sits at :2401-2411), and once per attribute per event in Event::fetchEvent (Event.php:3435).

Mechanism

DecayingModelBase::computeCurrentScore is declared as computeCurrentScore($user, $model, $attribute, $base_score = false, $last_sighting_timestamp = false) (app/Model/DecayingModelsFormulas/Base.php:133), but its only production caller, DecayingModel::getScore, calls $this->Computation->computeCurrentScore($user, $model, $attribute) with three arguments (app/Model/DecayingModel.php:723). The fifth parameter is therefore always false, the guard at Base.php:139 always fires, and Base.php:140 calls Sighting::getLastSightingForAttribute($user, $attribute['id']) — a find('first') with 'order' => ['Sighting.date_sighting DESC'] (app/Model/Sighting.php:1064-1068). One MySQL round trip, per attribute, per model (DecayingModel.php:640 loops the models, :643 calls getScore).

Everything else the function does is pure PHP: computeBasescore (Base.php:102-129) walks the attribute's EventTag/AttributeTag arrays through __getPrioritisedTag (:67-100) and __getRatioScore (:56-63), and Polynomial::computeScore (app/Model/DecayingModelsFormulas/Polynomial.php:11-19) is a single pow(). So the round trip is the whole cost of the block.

The change:

• New Sighting::getLastSightingsForAttributes($user, $attributeIds) runs one SELECT attribute_id, MAX(date_sighting) AS last_sighting ... WHERE attribute_id IN (batch) AND type = 0 [AND org_id IN (...)] GROUP BY attribute_id, reproducing the org_id conditions of getLastSightingForAttribute (Sighting.php:1055-1058) exactly for SIGHTING_POLICY_EVENT_OWNER/HOST_ORG, and no org filter for SIGHTING_POLICY_EVERYONE — same as the original in both cases.
• MispAttribute::fetchAttributes builds the map once per 50000-row batch page and Event::fetchEvent once per event.
• attachScoresToAttribute and DecayingModel::getScore gained a trailing optional $last_sighting_timestamp = false, forwarded as computeCurrentScore($user, $model, $attribute, false, $last_sighting_timestamp).
• ClassRegistry::init('DecayingModel') is hoisted out of the per-attribute loop (MispAttribute.php:2402). It is a registry hit, not a re-instantiation, so this is cleanliness only and is not part of the speedup claim.

Query count for the decay block goes from 50000 * M per batch page to 1.

Why existing caching does not already cover it

I checked the caching that exists on this path; none of it touches the sighting.

1. DecayingModel::$modelCacheForType (DecayingModel.php:20, populated :635/:639) caches the model definitions per attribute type, not the sightings.
2. DecayingModelMapping::$modelCache (DecayingModelMapping.php:28, checked :80-82, stored :117) memoises getAssociatedModels per attribute_type — so the mapping lookup at DecayingModel.php:628 is genuinely not an N+1, but it says nothing about sightings.
3. DecayingModel::$__registered_model_classes (:22, :461-468) memoises the formula class instance.
4. ClassRegistry::init('DecayingModel') at MispAttribute.php:2402 is a registry hit returning the existing instance (app/Lib/cakephp/lib/Cake/Utility/ClassRegistry.php:305-315, returned at :137-141), so the memos above do survive the loop — the per-attribute init was a no-op cost, not the defect.
5. computeCurrentScore itself has no memo of any kind between Base.php:133 and :165.
6. The sightings that includeSightings may already have attached (MispAttribute.php:2371 → $attr['Attribute']['Sighting']) are ignored: getLastSightingForAttribute re-queries by attribute_id.
7. No Redis/Cache/file cache is consulted anywhere in Base.php or in getLastSightingForAttribute.
8. sightings carries only KEY attribute_id (INSTALL/MYSQL.sql:1413), no composite (attribute_id, type, date_sighting), so the ORDER BY ... DESC LIMIT 1 is a sort over the matched rows, not an index-order early stop.

Speedup

MEASURED: 14.73x on the named path (an independent rerun of the same harness measured 18.93x; I report the lower figure).

Benchmark method — bench.php loads the real app/Model/DecayingModelsFormulas/Base.php twice, the pristine 2.5 copy and the patched copy, plus the real Polynomial.php, each under its own PHP namespace (Bench\Orig, Bench\Patched) via class renaming, so the timed code is the actual production formula rather than a paraphrase. (The three source copies were byte-diffed against the pristine and worktree originals to confirm this.) Each namespace defines a local time() returning a frozen timestamp and a DateTime subclass so the two paths are byte-comparable rather than racing the clock, plus a local ClassRegistry handing back a PDO-backed Sighting stub that mirrors app/Model/Sighting.php exactly: getLastSightingForAttribute issues the per-attribute ... WHERE attribute_id = ? AND type = 0 AND org_id IN (?,?) ORDER BY date_sighting DESC LIMIT 1; getLastSightingsForAttributes issues the batched grouped MAX.

Fixture: a bench_sightings table with the INSTALL/MYSQL.sql:1405-1417 column set and only KEY attribute_id (no composite, matching production), seeded with 23268 sightings across 5000 attributes — ~30% of attributes deliberately have no visible sighting, sightings spread over 4 org_ids and both type 0/1 so the org and type filters actually discriminate. Attributes carry randomised AttributeTag/EventTag arrays and a last_seen that is NULL 40% of the time, so all three branches of the fallback are exercised. Timing is the mean of 3 reps of the full 5000-attribute loop each way after a 200-attribute warmup, serialized behind a lock.

Raw output:

seeded 23268 sightings over 5000 attributes (3121 with a visible sighting)
correctness mismatches: 0
score sum orig=3524.639800 patched=3524.639800 (identical: yes)
original : 6.1401 s for 5000 attributes (5000 DB queries)
patched  : 0.4168 s for 5000 attributes (1 DB query)
speedup  : 14.73x

Correctness assertion: 5000 inputs, 0 mismatches, over three independent comparisons — batched map vs per-attribute query result; both vs an independently computed expected maximum; and the full computeCurrentScore return array (score + base_score) orig vs patched compared with strict !==. Score sums identical.

Honest caveats on the number:

• The measured figure is a floor on the mechanism, not a production-path measurement. The "before" side is a re-executed PDO prepared statement, not a full CakePHP find('first') with query building, hydration and _filterResults/afterFind — so the ratio understates the production ratio rather than inflating it.
• It is also latency-flattered by the ~1 ms in-container round trip. Recomputing with a 200 µs round trip still yields roughly 5x.
• This is the speedup of the named path (the per-attribute decay block), not of a whole export request. End-to-end gain is smaller and depends on which other include* options are enabled, since those add untouched per-attribute cost.

DERIVED: >= 5x floor, independently of the measurement. Per attribute the block costs Q + A, where Q = one CakePHP round trip (Sighting.php:1064, times M >= 1 associated models) and A = the pure-PHP score computation. After the change, Q becomes Q/50000 per batch page. Even under the absurd assumption that A costs as much as a full find('first') round trip: before = Q + A = 2 units, after = A + Q/50000 ~= 1 unit, i.e. 2.0x as an absolute floor. A single indexed MySQL round trip through CakePHP is realistically one to two orders of magnitude more expensive than one pow() plus a few array walks over an attribute's handful of tags, which puts the true figure near M * 50x; 5x is the defensible floor with margin.

Risk

Medium-low. The semantics hold only if the batched query reproduces the sightings-policy conditions of Sighting.php:1055-1062, and it does:

• SIGHTING_POLICY_SIGHTING_REPORTER takes a different path (listSightings) and returns a scalar rather than a row, so it must not be batched. getLastSightingsForAttributes returns null under that policy, both call sites then pass false, and computeCurrentScore takes the untouched original per-attribute branch. That policy is bit-for-bit unchanged — which also means it preserves a pre-existing bug: Sighting.php:1059-1062 returns a scalar from a method declared : array which Base.php then array-indexes, and listSightings throws MethodNotAllowedException when the attribute has no sightings. I did not fix that; it is orthogonal and would be a behaviour change smuggled into a perf patch.
• The org_id filter for EVENT_OWNER/HOST_ORG depends on $user['org_id'] and MISP.host_org_id, both constant for the whole request, so it batches cleanly.
• "No sighting" sentinel. The original distinguishes empty from present at Base.php:141. The patch restructures computeCurrentScore so false = "not prefetched, query it" (original behaviour) and null = "known: no sighting", routing into the existing last_seen/timestamp fallback. null rather than 0 is deliberate and strictly more correct than a 0-sentinel would be: a legitimate sightings row with date_sighting = 0 is non-empty in the original and must not fall through to last_seen.
• MAX() vs ORDER BY ... DESC LIMIT 1. date_sighting is bigint(20) NOT NULL (INSTALL/MYSQL.sql:1407), so MAX()-skips-NULL cannot diverge.

Raised in review, all accepted as known and non-blocking:

• One extra query when the gate is on but no model applies. With includeDecayScore set and zero enabled models associated with the batch's attribute types, the original issued zero sighting queries; the patch issues one grouped query per batch page / per event regardless. Perf micro-cost only, no output change, and a strict win whenever any model is associated.
• Unchunked IN list. Up to 50000 ids in a single IN (...) (~400 KB of SQL). Safe under the default max_allowed_packet (16 MB), but there is no chunking guard.
• REQUIRES_SIGHTINGS formulas are untouched. Sightings.php → Base.php:158 → Sighting::listSightings → a nested per-attribute ACL-enforced fetchAttributes (Sighting.php:960) is a larger win still, but it changes listSightings' semantics and shape and is a separate, riskier change. The 14.73x does not depend on it.
• Composite index (attribute_id, type, date_sighting) on sightings deliberately not added. It needs a db_schema.json + Server.php/updateDatabase migration with its own version bump and upgrade path — out of scope for a minimal code-path patch, and the batched query is one round trip either way.
• Bench is a floor, not a production measurement (see the Speedup caveats).

Behaviour-preservation argument in one line: $last_sighting_timestamp === false still means "look it up", so every pre-existing caller — including DecayingModel.php:591, which always passes an int, and DecayingModelController.php:811 — is unaffected; the only new value the parameter can take is null, produced solely by the two batching call sites. Field selection, return shapes, ordering and ACL conditions are untouched.

Testing

• php -l clean on all five changed files. The deprecation notices on DecayingModel.php:164 and Event.php:6351/:6682/:9977 are pre-existing in the pristine tree, not introduced here.
• The benchmark above (5000 attributes, 3 reps each way after warmup, serialized behind a lock).
• The correctness assertion above: 5000 inputs, three independent comparisons, 0 mismatches, identical score sums.
• A separate edge-case harness written during review, covering has-sighting / no-sighting x last_seen NULL / set, date_sighting = 0, a sighting older than the attribute timestamp, and the SIGHTING_POLICY_SIGHTING_REPORTER scalar return including the pathological timestamp = 0 case: original === patched-with-false and original === patched-with-prefetch on every case.
• Confirmed the one hazard raw-PDO benching cannot catch: neither Sighting nor AppModel declares a default public $order, so Cake will not inject an ORDER BY on a non-grouped column into the new grouped find — no ONLY_FULL_GROUP_BY problem. Neither model declares beforeFind/afterFind either, so the partial grouped rows pass through no callback that could touch a missing field.
• The 'MAX(x) AS y' + 'group' pattern has in-repo precedent (Log.php:373, MispAttribute.php:3015/:3020), and the aggregate is read with a $row[0]['last_sighting'] ?? $row['Sighting']['last_sighting'] fallback for the row position.

No live MISP instance was available, so this has had no end-to-end testing — no real export, restSearch or CSV run against a populated instance. All verification above is static analysis plus a standalone harness running the real formula code against a seeded MariaDB fixture. Review of the actual query against a production-shaped sightings table would be welcome.

🤖 Generated with Claude Code