Skip to content

fix: avoid spatial initialization during memory queries - #50

Merged
maverick-woo merged 1 commit into
mainfrom
fix/spatial-cold-memory-query
Nov 1, 2026
Merged

maverick-woo merged 1 commit into
mainfrom
fix/spatial-cold-memory-query

Conversation

@maverick-woo

@maverick-woo maverick-woo commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Keep memory queries from initializing spatial compute budgets before the runtime device is ready, fixing early vLLM startup failures on 310P.

Validated with vLLM OPT-125m on 310P: fresh main reproduces the failure; the fix passes single-container and shared-card two-Pod tests.

Follow-up to #40.

Summary by CodeRabbit

  • Bug Fixes
    • Memory information requests now report the current device’s HBM information when HBM limits are enabled. In other cases, requests continue to use the native runtime.
    • Invalid vNPU configurations and requested limits are now rejected before memory information is returned, helping prevent invalid requests from proceeding.

Resolve the per-device memory quota without applying compute budgets. Preserve native memory query errors when no device has been selected.

Signed-off-by: maverick123123 <yuming.wu@dynamia.ai>
@hami-robot
hami-robot Bot requested a review from archlitchi November 1, 2026 09:16
@hami-robot

hami-robot Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maverick-woo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hami-robot hami-robot Bot added the approved label Nov 1, 2026
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8b2ae7a5-2ed1-4629-9af7-90996dbff7ec
Commits

Reviewing files that changed from the base of the PR and between 5cdb4c4 and 2ee327a.

Files selected for processing: 1
  • crates/hook/src/hook.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The rtMemGetInfoEx hook now validates requested vNPU limits when vNPU configuration is requested. It uses HBM information for the current logical device when that device has HBM limits enabled. Otherwise, it forwards the request to the native runtime and retains the spatial guard.

Changes: Device-aware memory query

Layer / File(s) Summary
Validate and resolve memory queries
crates/hook/src/hook.rs
The hook validates requested limits and preserves the spatial guard. It returns HBM information when the current logical device has HBM limits enabled; other cases use the native runtime passthrough. The unused npu_limiter import was removed.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related issues

  • Issue 42 describes the spatial-budget initialization order. This change preserves the spatial guard in the memory-query path.
  • Issue 45 describes per-NPU memory accounting. This change uses the current logical device when checking for HBM limits.

Possibly related PRs

Suggested reviewers: archlitchi, xiguapi-h

Merge Risk: Minimal · up to 2ee32

The change avoids early spatial initialization during memory queries. No merge-blocking risk remains.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing spatial initialization during memory queries.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks the limits first
Then finds the device for the query
HBM details come when limits apply
The native path stays close at hand
Soft paws hop past the spatial guard

@maverick-woo

Copy link
Copy Markdown
Member Author

/approved

@maverick-woo
maverick-woo merged commit 591cfdd into main Nov 1, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant