Skip to content

Commit 5b641ae

Browse files
committed
perf(go): order the body-node owner candidates once, not per lookup
split_body_node_id sorted every callable id on every call, and _body_source_of calls it once per get_source of a body-node id -- so a bulk get_source over a page's endpoints sorted 5,955 ids per endpoint on nats-server. The order is a property of the application, not of the node being looked up. longest_first() now produces it and the backend caches it in _owner_ids_longest_first. Ties break on the id, so the order is total and a lookup is reproducible rather than dependent on dict iteration. Found evaluating against the analyzer's real-app corpus. Refs #436
1 parent 805a81b commit 5b641ae

2 files changed

Lines changed: 38 additions & 7 deletions

File tree

‎cldk/analysis/go/codeanalyzer/codeanalyzer.py‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
VIA,
6969
GoAnalysisBackend,
7070
)
71-
from cldk.analysis.go.ids import body_node_id, callable_match_names, external_signature, go_value_names, pick_declaration, split_body_node_id
71+
from cldk.analysis.go.ids import body_node_id, callable_match_names, external_signature, go_value_names, longest_first, pick_declaration, split_body_node_id
7272
from cldk.models.go import (
7373
GoAnalysis,
7474
GoApplication,
@@ -372,6 +372,16 @@ def get_source(self, node_id: str) -> str:
372372
self._module_bytes[mod.id] = encoded
373373
return encoded[b0:b1].decode("utf-8")
374374

375+
@cached_property
376+
def _owner_ids_longest_first(self) -> Tuple[str, ...]:
377+
"""Every owner id, longest first — the candidate order a body-node id is split against.
378+
379+
Cached because the order is a property of the application, not of the node being looked up:
380+
computing it inside the lookup sorted 5,955 ids on every ``get_source`` of a body-node id
381+
on nats-server.
382+
"""
383+
return longest_first(self._dataflow_owners)
384+
375385
def _body_source_of(self, node_id: str) -> "tuple | None":
376386
"""``(module, body node)`` for a body-node id, or ``None`` if nothing owns it.
377387
@@ -380,11 +390,15 @@ def _body_source_of(self, node_id: str) -> "tuple | None":
380390
vertex ``@``. A stand-in has no module and so no text, which this reports as "not found"
381391
rather than as a module lookup that would fail further on.
382392
383-
Not indexed eagerly: the body-node ids outnumber the durable ones by a wide margin (3,324
384-
against 319 on the analyzer's own corpus), and ``get_source`` on one is the uncommon case,
393+
Not indexed eagerly: the body-node ids outnumber the durable ones by a wide margin (1.69
394+
**million** against 5,955 on nats-server), and ``get_source`` on one is the uncommon case,
385395
so the prefix walk is paid per call instead of the index being paid per application.
396+
397+
The *candidate order*, though, is paid once — :attr:`_owner_ids_longest_first`. It used to
398+
be sorted inside the lookup, which made a bulk ``get_source`` over a page's endpoints sort
399+
every callable id per endpoint.
386400
"""
387-
hit = split_body_node_id(node_id, self._dataflow_owners)
401+
hit = split_body_node_id(node_id, self._owner_ids_longest_first)
388402
if hit is None:
389403
return None
390404
cid, key = hit

‎cldk/analysis/go/ids.py‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,20 @@ def body_node_id(callable_id: str, body_key: str) -> str:
5252
return f"{callable_id}{body_key}" if body_key.startswith("@") else f"{callable_id}@{body_key}"
5353

5454

55+
def longest_first(callable_ids: Iterable[str]) -> Tuple[str, ...]:
56+
"""``callable_ids`` ordered longest first — the order :func:`split_body_node_id` needs.
57+
58+
Longest first because a callable id can be a proper prefix of a nested closure's
59+
(``…flow.Closure`` and ``…flow.Closure.closure_1``), so the shorter would otherwise claim the
60+
longer's vertices. Ties are broken by the id itself, so the order is total and a lookup is
61+
reproducible rather than dependent on dict iteration.
62+
63+
Separate from the lookup so a backend computes it **once** per application instead of once per
64+
node: see :func:`split_body_node_id`'s ``callable_ids``.
65+
"""
66+
return tuple(sorted(callable_ids, key=lambda cid: (-len(cid), cid)))
67+
68+
5569
def split_body_node_id(node_id: str, callable_ids: Iterable[str]) -> Tuple[str, str] | None:
5670
"""``(callable id, body key)`` for ``node_id``, or ``None`` if no known callable owns it.
5771
@@ -72,15 +86,18 @@ def split_body_node_id(node_id: str, callable_ids: Iterable[str]) -> Tuple[str,
7286
7387
Args:
7488
node_id: A body node's absolute ``can://`` id.
75-
callable_ids: Every callable id in the application, stand-ins included. Order is
76-
irrelevant; this sorts.
89+
callable_ids: Every callable id in the application, stand-ins included, **already ordered
90+
longest first** — :func:`longest_first` is what produces that order, and a backend
91+
should compute it once rather than per call. Sorting here instead cost a sort of every
92+
callable id on every lookup, which on nats-server (5,955 callables) made a bulk
93+
``get_source`` over a page's endpoints quadratic; the eval surfaced it.
7794
7895
Returns:
7996
``(callable_id, body_key)`` where ``body_node_id(*result) == node_id``, or ``None`` when no
8097
candidate owns it — a miss is returned, not raised, because the callers here are describing
8198
a walk's nodes and an unknown node is a node to skip, not an error to propagate.
8299
"""
83-
for cid in sorted(callable_ids, key=len, reverse=True):
100+
for cid in callable_ids:
84101
if not node_id.startswith(cid) or node_id == cid:
85102
continue
86103
rest = node_id[len(cid) :]

0 commit comments

Comments
 (0)