Skip to content

Commit ff3fe65

Browse files
harsh543claude
andcommitted
Replace O(N*L^2) grouping with a trie -- fixes scaling finding from review
Addresses a scaling finding from @raravind007's review on this PR: groups_by_prefix built a tuple(chain[:depth]) key for every depth of every request's chain, which is O(chain_length^2) per request. That lands hardest on exactly the workload this tool is meant to highlight -- a long shared system prompt or tool schema reused across many requests (their example: an 8k-token shared prefix at block_size=16 is ~512 blocks; at 10k requests that's ~2.6B tuple-element operations for grouping alone). Replaced with a trie built in one O(total blocks) pass (each chain step is a single dict lookup/insert), where a shared prefix is identified by shared trie nodes instead of by re-slicing and re-hashing a growing tuple at every depth. A winning group's actual block-hash tuple is only reconstructed (via parent pointers) for the at-most-top_k_groups nodes that survive selection, not for every node in the trie. Verified equivalence against the old implementation with a 300-trial fuzz test comparing both algorithms' output on random synthetic chain trees (not included in this commit -- exploratory verification only); 6 of 300 trials initially disagreed, all on groups tied exactly on (depth, count) competing for the last top_k slot. Neither the old nor new code had a defined tiebreak for that case -- it fell out of incidental dict/stack iteration order in both -- so this also adds an explicit deterministic tiebreak (smallest request_id in the group) instead of leaving output order-dependent. With that tiebreak applied to both, all 300 trials matched exactly. Measured ~8.7x speedup at a modest N=2000/L=200 shape; the gap widens quadratically with chain length, so it's much larger at the review's N=10000/L=512 example. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: harsh543 <harsh.rocks@hotmail.com>
1 parent aac06a7 commit ff3fe65

2 files changed

Lines changed: 153 additions & 41 deletions

File tree

tests/entrypoints/unit_tests/test_prefix_cache_analysis.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from vllm.entrypoints.prefix_cache_analysis import (
99
PromptRecord,
10+
_group_requests_by_shared_prefix,
1011
_reusable_tokens_from_chains,
1112
analyze,
1213
load_plain_prompt_jsonl,
@@ -107,6 +108,46 @@ def test_reusable_tokens_does_not_double_count_overlapping_groups():
107108
assert _reusable_tokens_from_chains(chains, block_size=16) == 112
108109

109110

111+
def test_group_requests_by_shared_prefix_finds_maximal_distinct_groups():
112+
"""Regression test for the trie-based rewrite of the O(N*L^2)
113+
tuple-slicing grouping (flagged in PR review as quadratic in chain
114+
length -- see _group_requests_by_shared_prefix's docstring). Reuses the
115+
same a/b/c fixture as the reusable-tokens test: {a,b} share a 5-block
116+
prefix, {a,b,c} share a shorter 2-block prefix within that. Both are
117+
real, distinct-membership groups and must both be reported.
118+
"""
119+
chains = {
120+
"a": [BlockHash(h) for h in (b"h0", b"h1", b"h2", b"h3", b"h4", b"hA_only")],
121+
"b": [BlockHash(h) for h in (b"h0", b"h1", b"h2", b"h3", b"h4", b"hB_only")],
122+
"c": [BlockHash(h) for h in (b"h0", b"h1", b"hC_diverge")],
123+
}
124+
groups = _group_requests_by_shared_prefix(chains, top_k_groups=10)
125+
126+
by_members = {tuple(sorted(g.request_ids)): g for g in groups}
127+
assert set(by_members) == {("a", "b"), ("a", "b", "c")}
128+
assert len(by_members[("a", "b")].shared_block_hashes) == 5
129+
assert len(by_members[("a", "b", "c")].shared_block_hashes) == 2
130+
131+
132+
def test_group_requests_by_shared_prefix_breaks_ties_deterministically():
133+
"""Two groups can tie exactly on (depth, request count) -- e.g. two
134+
unrelated pairs each sharing a single block. Without an explicit
135+
tiebreak, which one wins the last top_k slot depends on incidental
136+
dict/stack iteration order. Assert it's actually deterministic (smallest
137+
request_id in the group wins) rather than order-dependent.
138+
"""
139+
chains = {
140+
"a": [BlockHash(b"hA")],
141+
"b": [BlockHash(b"hA")],
142+
"y": [BlockHash(b"hY")],
143+
"z": [BlockHash(b"hY")],
144+
}
145+
groups = _group_requests_by_shared_prefix(chains, top_k_groups=1)
146+
147+
assert len(groups) == 1
148+
assert set(groups[0].request_ids) == {"a", "b"}
149+
150+
110151
def test_analyze_rejects_duplicate_request_ids():
111152
"""Regression test: without this check, chains[record.request_id] = chain
112153
silently overwrites the earlier record's chain on a duplicate id, but

vllm/entrypoints/prefix_cache_analysis.py

Lines changed: 112 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,117 @@ def _full_block_hash_chain(
165165
return chain
166166

167167

168+
class _TrieNode:
169+
"""One node in the shared-prefix trie built by
170+
``_group_requests_by_shared_prefix``. ``block_hash``/``parent`` identify
171+
the edge this node was reached by, so a winning node's full prefix can be
172+
reconstructed by walking parents back to the root -- without needing
173+
every node to carry its own path.
174+
"""
175+
176+
__slots__ = ("children", "request_ids", "parent", "block_hash")
177+
178+
def __init__(
179+
self,
180+
parent: _TrieNode | None = None,
181+
block_hash: BlockHash | None = None,
182+
) -> None:
183+
self.children: dict[BlockHash, _TrieNode] = {}
184+
self.request_ids: list[str] = []
185+
self.parent = parent
186+
self.block_hash = block_hash
187+
188+
189+
def _path_to_node(node: _TrieNode) -> tuple[BlockHash, ...]:
190+
path: list[BlockHash] = []
191+
while node.parent is not None:
192+
assert node.block_hash is not None
193+
path.append(node.block_hash)
194+
node = node.parent
195+
path.reverse()
196+
return tuple(path)
197+
198+
199+
def _group_requests_by_shared_prefix(
200+
chains: dict[str, list[BlockHash]], top_k_groups: int
201+
) -> list[PrefixGroup]:
202+
"""Find the top-K largest shared hash-chain prefixes across requests.
203+
204+
Builds a trie over the chains in a single O(total blocks) pass -- each
205+
chain step is one dict lookup/insert -- instead of materializing a
206+
``tuple(chain[:depth])`` key for every depth of every chain, which is
207+
O(chain_length ** 2) per request. That quadratic cost lands hardest on
208+
exactly the workload this tool exists to highlight: a long shared system
209+
prompt or tool schema reused across many requests (e.g. an 8k-token
210+
shared prefix at block_size=16 is ~512 blocks; at 10k requests the old
211+
approach did on the order of 10_000 * 512**2 ~= 2.6B tuple-element
212+
operations for grouping alone).
213+
214+
A winning group's actual ``shared_block_hashes`` tuple is only
215+
reconstructed (via ``_path_to_node``) for the at-most-``top_k_groups``
216+
nodes that survive selection, not for every trie node.
217+
"""
218+
root = _TrieNode()
219+
for request_id, chain in chains.items():
220+
node = root
221+
for block_hash in chain:
222+
child = node.children.get(block_hash)
223+
if child is None:
224+
child = _TrieNode(parent=node, block_hash=block_hash)
225+
node.children[block_hash] = child
226+
node = child
227+
node.request_ids.append(request_id)
228+
229+
# Depth is tracked as a plain counter alongside each stack entry, not by
230+
# materializing a path, so this collection pass is O(total trie nodes)
231+
# <= O(total blocks).
232+
candidates: list[tuple[int, _TrieNode]] = []
233+
stack: list[tuple[_TrieNode, int]] = [(root, 0)]
234+
while stack:
235+
node, depth = stack.pop()
236+
# Each request_id is appended at most once per node (chains cannot
237+
# revisit a node -- block hashes are chained forward-only), so
238+
# request_ids is already duplicate-free here.
239+
if depth > 0 and len(node.request_ids) > 1:
240+
candidates.append((depth, node))
241+
for child in node.children.values():
242+
stack.append((child, depth + 1))
243+
244+
# Prefer longer shared prefixes first, then more requests sharing them.
245+
# Ties (equal depth and count) are broken by the lexicographically
246+
# smallest request_id in the group, so selection is a deterministic
247+
# function of the input and doesn't depend on dict/stack iteration
248+
# order -- two runs on the same input always report the same winners.
249+
candidates.sort(
250+
key=lambda c: (-c[0], -len(c[1].request_ids), min(c[1].request_ids))
251+
)
252+
253+
# Deduplicate so a longer prefix's requests aren't also reported at
254+
# every shorter prefix depth -- keep only maximal groups. This is a
255+
# display-only reduction: a request set can legitimately appear in more
256+
# than one *distinct* group (e.g. {a,b} at depth 5 and {a,b,c} at depth
257+
# 2 are both real, different-membership sharing clusters), so this only
258+
# collapses redundant same-membership entries at shorter depths.
259+
top_groups: list[PrefixGroup] = []
260+
covered: set[tuple[str, ...]] = set()
261+
for depth, node in candidates:
262+
request_ids = sorted(node.request_ids)
263+
key = tuple(request_ids)
264+
if key in covered:
265+
continue
266+
covered.add(key)
267+
top_groups.append(
268+
PrefixGroup(
269+
shared_block_hashes=_path_to_node(node),
270+
request_ids=request_ids,
271+
)
272+
)
273+
if len(top_groups) >= top_k_groups:
274+
break
275+
276+
return top_groups
277+
278+
168279
def _reusable_tokens_from_chains(
169280
chains: dict[str, list[BlockHash]], block_size: int
170281
) -> int:
@@ -227,47 +338,7 @@ def analyze(
227338
total_full_block_tokens += len(chain) * block_size
228339
chains[record.request_id] = chain
229340

230-
# Group requests by their longest shared prefix of block hashes.
231-
# A group key is the hash-chain prefix itself; every request that
232-
# shares that exact prefix (or extends it) counts toward the group
233-
# at that depth.
234-
groups_by_prefix: dict[tuple[BlockHash, ...], list[str]] = defaultdict(list)
235-
for request_id, chain in chains.items():
236-
for depth in range(1, len(chain) + 1):
237-
prefix = tuple(chain[:depth])
238-
groups_by_prefix[prefix].append(request_id)
239-
240-
# Keep only prefixes shared by more than one request -- a prefix
241-
# unique to a single request contributes nothing to reuse.
242-
shared_groups = [
243-
PrefixGroup(shared_block_hashes=prefix, request_ids=sorted(set(ids)))
244-
for prefix, ids in groups_by_prefix.items()
245-
if len(set(ids)) > 1
246-
]
247-
248-
# Prefer longer shared prefixes first, then more requests sharing them.
249-
shared_groups.sort(
250-
key=lambda g: (len(g.shared_block_hashes), len(g.request_ids)),
251-
reverse=True,
252-
)
253-
254-
# Deduplicate so a longer prefix's requests aren't also reported at
255-
# every shorter prefix depth -- keep only maximal groups. This is a
256-
# display-only reduction: a request set can legitimately appear in more
257-
# than one *distinct* group (e.g. {a,b} at depth 5 and {a,b,c} at depth
258-
# 2 are both real, different-membership sharing clusters), so this only
259-
# collapses redundant same-membership entries at shorter depths.
260-
top_groups: list[PrefixGroup] = []
261-
covered: set[tuple[str, ...]] = set()
262-
for group in shared_groups:
263-
key = tuple(group.request_ids)
264-
if key in covered:
265-
continue
266-
covered.add(key)
267-
top_groups.append(group)
268-
if len(top_groups) >= top_k_groups:
269-
break
270-
341+
top_groups = _group_requests_by_shared_prefix(chains, top_k_groups)
271342
reusable_tokens = _reusable_tokens_from_chains(chains, block_size)
272343

273344
return AnalysisReport(

0 commit comments

Comments
 (0)