-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[None][feat] Enable KVCacheManagerV2 by default for Llama and Llama4 #18342
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 |
|---|---|---|
|
|
@@ -1134,6 +1134,14 @@ def forward( | |
| @register_auto_model("LlamaForCausalLM") | ||
| class LlamaForCausalLM(SpecDecOneEngineForCausalLM[LlamaModel, LlamaConfig]): | ||
|
|
||
| @classmethod | ||
| def get_preferred_kv_cache_manager_version( | ||
| cls, | ||
| pretrained_config: Any = None, | ||
| ) -> Literal["V2"]: | ||
| """Prefer KV cache manager V2 for Llama.""" | ||
| return "V2" | ||
|
|
||
| @classmethod | ||
| def get_preferred_transceiver_runtime( | ||
| cls, | ||
|
|
@@ -1505,6 +1513,22 @@ def call_with_text_prompt( | |
| class Llama4ForConditionalGeneration(SpecDecOneEngineForCausalLM[Llama4Model, | ||
| Llama4Config]): | ||
|
|
||
| @classmethod | ||
| def get_preferred_kv_cache_manager_version( | ||
| cls, | ||
| pretrained_config: Any = None, | ||
| ) -> Literal["V2"]: | ||
| """Prefer KV cache manager V2 for Llama4.""" | ||
| return "V2" | ||
|
|
||
| @classmethod | ||
| def get_preferred_transceiver_runtime( | ||
| cls, | ||
| pretrained_config: Any = None, | ||
| ) -> Optional[Literal["CPP", "PYTHON"]]: | ||
| """Prefer the Python transceiver for Llama4 NIXL disaggregated serving.""" | ||
| return "PYTHON" | ||
|
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. [MAJOR] Llama4 prefers the Python transceiver unconditionally, including when the KV manager resolves to V1
Concrete trigger: a Llama4 NIXL disagg run where the user sets Suggested fix: make the preference conditional on V2 actually being in effect, or return @classmethod
def get_preferred_transceiver_runtime(
cls,
pretrained_config: Any = None,
) -> Optional[Literal["CPP", "PYTHON"]]:
# Only meaningful together with KVCacheManagerV2; leave CPP as-is for V1.
return "PYTHON" if cls.get_preferred_kv_cache_manager_version(
pretrained_config) == "V2" else NoneIf the resolver already couples the two decisions (not visible in this diff), please say so explicitly, and state whether any Llama4 disagg run exercised V1 + Python transceiver. |
||
|
|
||
| def __init__( | ||
| self, | ||
| model_config: ModelConfig[Llama4Config], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,10 +49,18 @@ | |
| "primaryMaxNumBlocks", | ||
| "primaryFreeNumBlocks", | ||
| "primaryUsedNumBlocks", | ||
| "primaryEvictableNumBlocks", | ||
| "primaryPeakFreeNumBlocks", | ||
| "primaryPeakUsedNumBlocks", | ||
| "primaryPeakEvictableNumBlocks", | ||
| # Instantaneous gauges — secondary (host) pool | ||
| "secondaryMaxNumBlocks", | ||
| "secondaryFreeNumBlocks", | ||
| "secondaryUsedNumBlocks", | ||
| "secondaryEvictableNumBlocks", | ||
| "secondaryPeakFreeNumBlocks", | ||
| "secondaryPeakUsedNumBlocks", | ||
| "secondaryPeakEvictableNumBlocks", | ||
| # Per-iteration deltas — context phase | ||
| "iterAllocTotalBlocks", | ||
| "iterAllocNewBlocks", | ||
|
|
@@ -71,8 +79,22 @@ | |
| # Intra-device (GPU → GPU) block copies | ||
| "iterIntraDeviceCopyBlocks", | ||
| "iterIntraDeviceCopyBytes", | ||
| # Host blocks dropped instead of being copied to another cold tier | ||
| "iterHostDroppedBlocks", | ||
| "iterHostDroppedBytes", | ||
| ] | ||
|
|
||
| SECONDARY_FIELDS = { | ||
| "secondaryMaxNumBlocks", | ||
| "secondaryFreeNumBlocks", | ||
| "secondaryUsedNumBlocks", | ||
| "secondaryEvictableNumBlocks", | ||
| "secondaryPeakFreeNumBlocks", | ||
| "secondaryPeakUsedNumBlocks", | ||
| "secondaryPeakEvictableNumBlocks", | ||
| } | ||
| NON_SECONDARY_FIELDS = set(ALL_FIELDS) - SECONDARY_FIELDS | ||
|
|
||
| TEST_NAMES = { | ||
| 1: "Cold start", | ||
| 2: "Partial block reuse", | ||
|
|
@@ -383,26 +405,36 @@ def test_rapid_fire(self, llm_instance, all_collected, request): | |
| assert total_alloc > 0, "iterAllocTotalBlocks = 0 across all entries" | ||
|
|
||
| def test_field_completeness(self, llm_instance, all_collected, request): | ||
| """Field completeness — verify all 18 fields present across all collected stats.""" | ||
| """Field completeness — verify fields in their manager-specific views.""" | ||
| # If running standalone (no prior tests), generate some traffic | ||
| if not all_collected: | ||
| llm_instance.generate(["Hello world"], SamplingParams(max_tokens=16)) | ||
| collect_stats(llm_instance, all_collected) | ||
|
|
||
| entries_with_kv = 0 | ||
| missing_fields = set() | ||
| for s in all_collected: | ||
| ki = s.get("kvCacheIterationStats") | ||
| if ki: | ||
| entries_with_kv += 1 | ||
| for ws, v in ki.items(): | ||
| for field in ALL_FIELDS: | ||
| if field not in v: | ||
| missing_fields.add(field) | ||
| is_v2 = "kvCacheIterationStatsByPoolGroup" in s | ||
|
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. [MINOR] Manager version inferred from a payload key; pool-group view contents never validated
Suggested fix: derive the expectation from the LLM's resolved |
||
| expected_fields = NON_SECONDARY_FIELDS if is_v2 else set(ALL_FIELDS) | ||
|
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. [MAJOR] ALL_FIELDS gains 10 newly-required fields with no serializer change in this PR
No serializer or stats-producer change is present in this PR, so the new names are asserted purely on the assumption that the current stats payload already emits them. Failure mode is concrete: if the V1 (or the V2 hot) view does not emit Please confirm these field names match what the serializer emits today on both arms (and, if they were added by an earlier PR, reference it), or scope the newly-added names to the manager version that produces them. |
||
| for v in ki.values(): | ||
| missing_fields = expected_fields - v.keys() | ||
| assert not missing_fields, ( | ||
| f"Missing kvCacheIterationStats fields: {sorted(missing_fields)}" | ||
| ) | ||
|
|
||
| if is_v2 and "kvCacheIterationStatsByColdPoolGroup" in s: | ||
|
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. [MINOR] Secondary/cold-pool fields are never verified on the V2 arm if the cold view is absent On the V2 arm, Suggested fix: either configure the fixture so a cold pool group is guaranteed (non-zero host cache / forced offload) and then require the key unconditionally, or assert explicitly that the key is expected-absent for this configuration so the skip is visible rather than silent: cold = s.get("kvCacheIterationStatsByColdPoolGroup")
assert cold is not None, "V2 entry has no kvCacheIterationStatsByColdPoolGroup" |
||
| cold_stats = s["kvCacheIterationStatsByColdPoolGroup"] | ||
| for v in cold_stats.values(): | ||
| missing_fields = SECONDARY_FIELDS - v.keys() | ||
| assert not missing_fields, ( | ||
| "Missing kvCacheIterationStatsByColdPoolGroup fields: " | ||
| f"{sorted(missing_fields)}" | ||
| ) | ||
|
|
||
| print(f" Entries with kvCacheIterationStats: {entries_with_kv}/{len(all_collected)}") | ||
| assert entries_with_kv > 0, "no entries contain kvCacheIterationStats" | ||
| assert len(missing_fields) == 0, f"Missing fields: {sorted(missing_fields)}" | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
|
|
||
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.
[MINOR] V2 preference on LlamaForCausalLM is inherited by every subclass and every checkpoint mapping to this architecture
get_preferred_kv_cache_manager_versionis aclassmethodonLlamaForCausalLM, so it applies to (a) every checkpoint whose config declaresarchitectures: ["LlamaForCausalLM"]— far wider than Llama 3.x, including third-party fine-tunes and derivative models — and (b) every Python subclass ofLlamaForCausalLMin the registry, which inherits the hook without opting in. The validation matrix quoted in the description covers Llama3 and Llama4 only, so any derived architecture with a different KV layout silently changes manager on this merge and the only signal would be a post-merge regression.Two concrete asks: (1) enumerate/confirm which registered classes inherit this hook and that none of them need V1, or push the preference down to the concrete classes you validated; (2) the title is
[None]— for a default flip on the most widely used architecture in the repo, a ticket id makes a post-merge revert traceable. Notepretrained_configis accepted but unused, so the hook has no room to opt a variant out today.