mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
perf(model): stop sending 42MB of the same string, and stop enumerating for nobody
Two defects the first whole-repository scan made visible. It ran for 23
minutes over Paid Memberships Pro and decided nothing.
**The batch was 97.5% duplication.** `hookCallbacks` describes the REPOSITORY,
not the region, and I put it in every request when the hook half landed. On a
637-file plugin that is 16,270 characters repeated across 2,500 requests:
taint-requests.json 42.5 MB, of which 41.7 MB is one string over again
Joern parsed that with ujson inside a 2GB heap and the batch died in
ForkJoinPool. The driver reported `query_failed` for all 2,500 regions, which
was the honest answer to a question that never should have been asked that
way. Repository-wide fields are now hoisted to a single `--param`, and only if
they are identical across the whole batch -- a field that genuinely varies per
region stays where it belongs.
**Arbitration spent 737 seconds enumerating candidates nobody would read.**
Candidates exist to be put to the judge, and `scan_region` discards them the
moment there is no judge to put them to -- but the driver enumerated them
first regardless, parsing each region's file once per family. With the budget
at zero model calls that is 2,500 parses whose results are dropped on arrival.
build 264.8s query 401.3s arbitrate 737.3s total 1403s
Nobody should pay for a question nobody will ask.
848 tests pass; gates byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
a762860b17
commit
7f83977add
@@ -80,6 +80,15 @@ _FRONTENDS = {
|
||||
_PREFER_FRONTEND = frozenset({"php"})
|
||||
FRONTEND_BUILD_ATTEMPTS = 4
|
||||
|
||||
# Request fields that describe the REPOSITORY rather than the region, and are therefore identical in every
|
||||
# request of a batch. They are hoisted to a single `--param` instead of being repeated thousands of times.
|
||||
#
|
||||
# Not a micro-optimisation. `hookCallbacks` is 16,270 characters on a 637-file plugin, and repeating it
|
||||
# across 2,500 requests made a 42.5MB request file of which 41.7MB was the same string over and over. Joern
|
||||
# parsed that with ujson inside a 2GB heap and the whole batch died in ForkJoinPool, which the driver
|
||||
# correctly reported as `query_failed` for all 2,500 regions -- a whole-repository scan that decided nothing.
|
||||
_SHARED_REQUEST_FIELDS = ("hookCallbacks", "dispatchApply")
|
||||
|
||||
Runner = Callable[..., subprocess.CompletedProcess[str]]
|
||||
|
||||
|
||||
@@ -407,7 +416,9 @@ class JoernBackend:
|
||||
if not script.is_file():
|
||||
logger.warning("no such cpg query: %s", script)
|
||||
return None
|
||||
payload = json.dumps({rid: {k: _render(v) for k, v in req.items()} for rid, req in requests.items()})
|
||||
rendered = {rid: {k: _render(v) for k, v in req.items()} for rid, req in requests.items()}
|
||||
shared = _hoist_shared(rendered)
|
||||
payload = json.dumps(rendered)
|
||||
# Through a FILE, never the command line. Linux caps a single argument at MAX_ARG_STRLEN (128KB)
|
||||
# whatever ARG_MAX says, and a repository blows through that: 2500 taint requests over a 637-file
|
||||
# WordPress plugin is a 1.49MB payload. execve returns E2BIG, `_run` catches the OSError, this returns
|
||||
@@ -430,6 +441,8 @@ class JoernBackend:
|
||||
"--param",
|
||||
f"requestsFile={requests_file}",
|
||||
]
|
||||
for name, value in shared.items():
|
||||
command += ["--param", f"{name}={value}"]
|
||||
completed = self._run(command, timeout=self.query_timeout, cwd=cpg_path.parent)
|
||||
if completed is None or completed.returncode != 0:
|
||||
detail = (completed.stderr or "")[-400:] if completed is not None else "timeout"
|
||||
@@ -513,6 +526,26 @@ def _unparsed_files(completed: subprocess.CompletedProcess[str] | None) -> tuple
|
||||
return tuple(seen)
|
||||
|
||||
|
||||
def _hoist_shared(rendered: dict[str, dict[str, str]]) -> dict[str, str]:
|
||||
"""Pull the repository-wide fields out of every request, returning them once.
|
||||
|
||||
A field only lifts if it is IDENTICAL across the whole batch, so a value that genuinely varies per
|
||||
region stays where it belongs and nothing is silently shared between questions.
|
||||
"""
|
||||
if not rendered:
|
||||
return {}
|
||||
shared: dict[str, str] = {}
|
||||
for name in _SHARED_REQUEST_FIELDS:
|
||||
values = {request.get(name, "") for request in rendered.values()}
|
||||
if len(values) == 1:
|
||||
value = values.pop()
|
||||
if value:
|
||||
shared[name] = value
|
||||
for request in rendered.values():
|
||||
request.pop(name, None)
|
||||
return shared
|
||||
|
||||
|
||||
def _render(value: object) -> str:
|
||||
"""CPGQL parameters are strings; a sequence becomes a comma-separated list the script splits."""
|
||||
if isinstance(value, str):
|
||||
|
||||
@@ -573,8 +573,11 @@
|
||||
field("sources"),
|
||||
field("sinks"),
|
||||
field("sanitizers"),
|
||||
field("hookCallbacks"),
|
||||
field("dispatchApply"),
|
||||
// Repository-wide, so the driver sends them ONCE as a top-level parameter rather than repeating
|
||||
// them in every request. The per-request form is still honoured for the tests and the single
|
||||
// -region path.
|
||||
if (field("hookCallbacks").nonEmpty) field("hookCallbacks") else hookCallbacks,
|
||||
if (field("dispatchApply").nonEmpty) field("dispatchApply") else dispatchApply,
|
||||
field("function"),
|
||||
paramSrc,
|
||||
field("file"),
|
||||
|
||||
@@ -223,15 +223,22 @@ def scan_repository(
|
||||
exhausted = counted is not None and counted.calls >= limits.max_model_calls
|
||||
family = getattr(spec, "family", "")
|
||||
prefetched = CpgResult(cpg_path=cpg.cpg_path, run=_prefetched(rows_by_id.get(rid, [])))
|
||||
judge = None if exhausted else counted
|
||||
try:
|
||||
enumerated = enumerate_candidates(root, region, family, taxonomy=taxonomy)
|
||||
candidates = [{"id": c.id, "text": c.text, "line": c.line} for c in enumerated.candidates]
|
||||
# Candidates exist to be PUT to the judge, and `scan_region` discards them the moment there is
|
||||
# no judge to put them to. Enumerating them anyway costs a parse of the region's file per family
|
||||
# per region: on a 637-file plugin that was 2,500 enumerations and 737 SECONDS of arbitration
|
||||
# over rows that had already been fetched. Nobody should pay for a question nobody will ask.
|
||||
candidates: list[dict[str, object]] = []
|
||||
if judge is not None:
|
||||
enumerated = enumerate_candidates(root, region, family, taxonomy=taxonomy)
|
||||
candidates = [{"id": c.id, "text": c.text, "line": c.line} for c in enumerated.candidates]
|
||||
found = scan_region(
|
||||
prefetched,
|
||||
spec,
|
||||
path=region.path,
|
||||
function=region.function or "",
|
||||
client=None if exhausted else counted,
|
||||
client=judge,
|
||||
model=model,
|
||||
candidates=candidates,
|
||||
# A repository's parameters are not all attacker input -- but an ENTRY POINT's are, by
|
||||
|
||||
Reference in New Issue
Block a user