Skip to content

perf(ckpt): use set lookups for HF state dict keys during import - #6489

Merged
cuichenx merged 1 commit into
NVIDIA-NeMo:mainfrom
jayzuccarelli:perf/statedict-key-set
Oct 17, 2026
Merged

cuichenx merged 1 commit into
NVIDIA-NeMo:mainfrom
jayzuccarelli:perf/statedict-key-set

Conversation

@jayzuccarelli

Copy link
Copy Markdown
Contributor

What does this PR do ?

StateDict membership checks scanned the full key list on every call, so per-key loops during HF import (state[k], k in state) were quadratic in the number of checkpoint keys. This switches them to set lookups.

Changelog

  • StateDict._get_key_set(): a set over source.get_all_keys(), used by __getitem__, __contains__, get, list access and exact-key _match_keys. Both in-tree sources return a cached list, so the set is built once; it is rebuilt only when the source hands back a different list (the Kimi K2.5-VL / K3 bridges temporarily wrap get_all_keys with virtual keys during build_conversion_tasks).
  • SafeTensorsStateSource.load_tensors: cache each shard's key set next to its reused safe_open handle instead of rebuilding set(f.keys()) per call. Dropped together with the handle in _invalidate_handle, so the retry path still re-reads a replaced shard (covered by the existing test_retry_refreshes_incomplete_shard).
  • Test: test_membership_follows_wrapped_get_all_keys checks membership before, during and after wrapping get_all_keys. Passes on main, fails on a build-once cache, passes here.

Benchmark

Synthetic checkpoints with real key names and one-element tensors, so this measures lookup overhead only, not end-to-end import time. CPU, StateDict(SafeTensorsStateSource(path)), keys warmed first.

Checkpoint shape Keys / shards per-key state[k] loop k in state loop
DeepSeek-V3 94,370 / 163 223.2s -> 12.6s 106.5s -> 0.07s
Qwen3-235B-A22B 73,229 / 118 110.8s -> 9.0s 58.8s -> 0.04s

Tests

uv run python -m pytest tests/unit_tests/models/hf_pretrained/: 193 passed, on this branch and with main's state.py (same tests, including the new one). Also tests/unit_tests/models/kimi_vl/test_kimi_k25_vl_bridge.py: 24 passed. Run on a CPU-only box: CPU torch wheel, Megatron-LM submodule installed editable, and transformer_engine replaced by a local import stub so megatron.bridge imports without CUDA. pre-commit passes on the changed files.

Additional Information

AI disclosure: I used Claude Code to profile this and write the change; I reviewed the diff, tests and benchmark.

🤖 Generated with Claude Code

StateDict membership checks (__getitem__, __contains__, get, exact
pattern match) scanned the full key list on every call, so a per-key
import loop was quadratic in the number of checkpoint keys. Cache a key
set, rebuilt only when the source returns a different key list, and
cache each shard's key set next to its reused safe_open handle.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Eugenio "Jay" Zuccarelli <[email protected]>
@copy-pr-bot

copy-pr-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cuichenx cuichenx added feature New capabilities, enhancements, or enablement work area:ckpt Checkpoint conversion, loading, export, and save paths needs-review PR is ready for code review and waiting on a reviewer labels Oct 15, 2026

@cuichenx cuichenx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great optimization, thanks for contributing

@cuichenx

Copy link
Copy Markdown
Contributor

/ok to test f644649

@github-actions

Copy link
Copy Markdown
Contributor

Automatic Claude reviews have been retired. To request a pull-request review, post a comment containing:

/review

Add model=claude to use a Claude reviewer (the default is model=codex). mode=light|strict selects the review depth; for example, /review model=claude mode=strict. Comment /review help for all options.

@cuichenx cuichenx added ready-to-merge PR is approved, current, and only waiting for CI to pass before merge and removed needs-review PR is ready for code review and waiting on a reviewer labels Oct 16, 2026
@cuichenx
cuichenx merged commit 5a87644 into NVIDIA-NeMo:main Oct 17, 2026
80 of 84 checks passed

This branch was previously deployed

1 inactive deployment
test — f6446493 Deployed Oct 16, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #22929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ckpt Checkpoint conversion, loading, export, and save paths community-request feature New capabilities, enhancements, or enablement work ready-to-merge PR is approved, current, and only waiting for CI to pass before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants