-
Notifications
You must be signed in to change notification settings - Fork 48
refactor(attn): let the bucketer own the batched decode buckets #810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,25 +96,6 @@ def _record_block(name: str): | |
| _MIN_BATCHED_SEQS = 4 | ||
|
|
||
|
|
||
| def _powers_of_two_up_to(n: int, start: int = 1) -> tuple[int, ...]: | ||
| """Powers of 2 in [start, n], plus n itself if it is not already a power of 2. | ||
|
|
||
| ``start`` is rounded up to a power of 2 first, keeping a pure doubling | ||
| sequence. A ``start`` above ``n`` yields just ``(n,)``. | ||
| """ | ||
| if n < 1: | ||
| return () | ||
| v = 1 | ||
| while v < start: | ||
| v *= 2 | ||
| result = [] | ||
| while v < n: | ||
| result.append(v) | ||
| v *= 2 | ||
| result.append(n) | ||
| return tuple(result) | ||
|
|
||
|
|
||
| def _find_bucket(n: int, buckets: tuple[int, ...]) -> int | None: | ||
| """Smallest bucket >= n, or None when n exceeds the top bucket.""" | ||
| idx = bisect.bisect_left(buckets, n) | ||
|
|
@@ -654,21 +635,15 @@ def __init__( | |
| static_ctx[name] for name in layer_names if name in static_ctx | ||
| ) | ||
|
|
||
| # Buckets for the batched decode fast path. One compiled kernel | ||
| # per bucket. TODO: expose as engine args if configurability is needed. | ||
| max_num_seqs = vllm_config.scheduler_config.max_num_seqs | ||
| max_num_blocks_per_seq = ( | ||
| model_config.max_model_len + self.block_size - 1 | ||
| ) // self.block_size | ||
| self._num_seqs_buckets: tuple[int, ...] = _powers_of_two_up_to(max_num_seqs) | ||
| self._num_blocks_buckets: tuple[int, ...] = _powers_of_two_up_to(max_num_blocks_per_seq) | ||
|
|
||
| # Owned here, not by the recorder, so a bucket build() can emit is | ||
| # always a bucket that was compiled: the warmup recorder reads this | ||
| # same instance back (spyre_model_runner._record_attention_graphs) | ||
| # rather than constructing a second one that could drift. | ||
| self._attn_bucketer = SpyreAttnBucketer(vllm_config) | ||
|
|
||
| self._num_seqs_buckets: tuple[int, ...] = tuple(self._attn_bucketer.num_seqs_buckets) | ||
| self._num_blocks_buckets: tuple[int, ...] = tuple(self._attn_bucketer.num_blocks_buckets) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the line with the real consequence. The decode path now inherits the KV-derived block ladder, so its floor moves with The KV ladder is coarse because the recorded set is a product of both axes — that reason doesn't apply here, since nothing records decode variants. Either keep this ladder independent, or call out the trade-off. |
||
|
|
||
| def _get_zero_tile(self, aligned_query_len: int) -> torch.Tensor: | ||
| """Return (or create) the shared all-zero mask tile for interior blocks. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,21 @@ def _parse_buckets(raw: str | None) -> list[int] | None: | |
| return values | ||
|
|
||
|
|
||
| def _powers_of_two_up_to(n: int, start: int = 1) -> tuple[int, ...]: | ||
| """Powers of 2 in [start, n] (start rounded up to a power of 2), plus n itself.""" | ||
| if n < 1: | ||
| return () | ||
| v = 1 | ||
| while v < start: | ||
| v *= 2 | ||
| result = [] | ||
| while v < n: | ||
| result.append(v) | ||
| v *= 2 | ||
| result.append(n) | ||
| return tuple(result) | ||
|
|
||
|
|
||
| def _resolve_buckets( | ||
| raw: str | None, limit: int, name: str, default: Callable[[], list[int]] | ||
| ) -> list[int]: | ||
|
|
@@ -113,10 +128,6 @@ def __init__(self, vllm_config: VllmConfig) -> None: | |
| max_model_len = vllm_config.model_config.max_model_len | ||
| max_batched = vllm_config.scheduler_config.max_num_batched_tokens | ||
|
|
||
| # Imported at call time, not module scope: spyre_attn imports this | ||
| # module, so a top-level import back into it would be circular. | ||
| from spyre_inference.v1.attention.backends.spyre_attn import _powers_of_two_up_to | ||
|
|
||
| if block_size & (block_size - 1): | ||
| # Not fatal: _powers_of_two_up_to rounds the start up to a power of | ||
| # two, just coarser at the bottom. Reachable because the platform | ||
|
|
@@ -161,6 +172,11 @@ def __init__(self, vllm_config: VllmConfig) -> None: | |
| {(kv + block_size - 1) // block_size for kv in self._kv_buckets} | ||
| ) | ||
|
|
||
| # The batched decode kernel adds a sequence axis, so it needs a second ladder. | ||
| self._num_seqs_buckets: list[int] = list( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Buckets 1 and 2 can never be reached — line 1030 only takes this path when |
||
| _powers_of_two_up_to(vllm_config.scheduler_config.max_num_seqs) | ||
| ) | ||
|
|
||
| logger.info( | ||
| "SpyreAttnBucketer: %d kv buckets [%d..%d], %d query buckets [%d..%d], " | ||
| "max num_blocks=%d", | ||
|
|
@@ -185,6 +201,10 @@ def query_buckets(self) -> list[int]: | |
| def num_blocks_buckets(self) -> list[int]: | ||
| return self._num_blocks_buckets | ||
|
|
||
| @property | ||
| def num_seqs_buckets(self) -> list[int]: | ||
| return self._num_seqs_buckets | ||
|
|
||
| def find_kv_bucket(self, kv_len: int) -> int | None: | ||
| return self._round_up(kv_len, self._kv_buckets) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,20 +21,23 @@ | |
| import pytest | ||
|
|
||
| from spyre_inference import envs | ||
| from spyre_inference.v1.attention.backends.spyre_attn import _powers_of_two_up_to | ||
| from spyre_inference.v1.attention.spyre_attn_bucketer import ( | ||
| SpyreAttnBucketer, | ||
| _parse_buckets, | ||
| _powers_of_two_up_to, | ||
| ) | ||
|
|
||
| BLOCK_SIZE = 64 | ||
|
|
||
|
|
||
| def make_config(max_model_len=2048, max_num_batched_tokens=512, block_size=BLOCK_SIZE): | ||
| def make_config( | ||
| max_model_len=2048, max_num_batched_tokens=512, block_size=BLOCK_SIZE, max_num_seqs=8 | ||
| ): | ||
| config = MagicMock() | ||
| config.cache_config.block_size = block_size | ||
| config.model_config.max_model_len = max_model_len | ||
| config.scheduler_config.max_num_batched_tokens = max_num_batched_tokens | ||
| config.scheduler_config.max_num_seqs = max_num_seqs | ||
| return config | ||
|
|
||
|
|
||
|
|
@@ -189,6 +192,23 @@ def test_count_stays_tractable_at_long_context(self): | |
| b = SpyreAttnBucketer(make_config(32768, 2048)) | ||
| assert len(b.variants()) < 500 | ||
|
|
||
| def test_num_seqs_buckets_are_powers_of_two_to_max_num_seqs(self): | ||
| b = SpyreAttnBucketer(make_config(max_num_seqs=8)) | ||
| assert b.num_seqs_buckets == [1, 2, 4, 8] | ||
|
|
||
| def test_num_seqs_buckets_top_out_at_max_num_seqs(self): | ||
| b = SpyreAttnBucketer(make_config(max_num_seqs=6)) | ||
| assert b.num_seqs_buckets[-1] == 6 | ||
| assert b.num_seqs_buckets == [1, 2, 4, 6] | ||
|
|
||
| def test_num_blocks_buckets_follow_the_kv_buckets(self, monkeypatch): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checks the bucketer, but not the thing the PR fixes. A test that fails before and passes after: with this same override plus |
||
| """A kv override moves the ladder the attention impl dispatches onto, so the | ||
| impl cannot land on a low block count that warmup never recorded.""" | ||
| monkeypatch.setenv("SPYRE_ATTN_KV_BUCKETS", "512,1024,2048") | ||
| envs.clear_env_cache() | ||
| b = SpyreAttnBucketer(make_config()) | ||
| assert b.num_blocks_buckets == [8, 16, 32] | ||
|
|
||
|
|
||
| class TestEnvOverride: | ||
| def test_kv_buckets_override(self, monkeypatch): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two copies exist only to satisfy
_find_bucket's tuple type, and_find_bucketis now the same lookup asSpyreAttnBucketer._round_up, which line 672 already calls on the bucketer's lists. Dropping both attributes and_find_bucket, and calling the bucketer directly at 1037-1038, removes the last place these can drift.