Skip to content

Removing patching for SGLang versions - #1486

Open
mohbasit wants to merge 7 commits into
mainfrom
feat/remove_patching_tracelens
Open

mohbasit wants to merge 7 commits into
mainfrom
feat/remove_patching_tracelens

Conversation

@mohbasit

@mohbasit mohbasit commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Description: what and why Switches SGLang shape discovery to the no-patch mechanism for SGLang ≥ 0.5.18. Instead of git apply-ing the TraceLens sglang_roofline_patches into the installed SGLang source, the profile path now puts the no-patch kernel_shape_tool on the server PYTHONPATH and sets TRACELENS_SHAPE_DISCOVERY=1, so CPython auto-loads the tool's sitecustomize.py to add Input Dims/Input type/Input Strides to profiler traces. The version gate (resolve_sglang_shape_mode() in _server_patcher.py) selects sitecustomize for ≥ 0.5.18 and legacy patched otherwise. Wiring is mirrored in the single-node (_workload_envs.py, _multi_node_server_lifecycle.py) and pod-side multi-node launchers (launch_multinode.py, launch_infera_node.py, infera.py). Also breaks the CodeQL module-level cyclic import by making the shape-mode imports function-local, and applies ruff format.

Linked issue(s): close/fix refs None.

Tests: added/updated? commands run? No new unit tests. Validated end-to-end on a single-GPU Qwen3-8B SGLang 0.5.18 run: the profile step produced a trace with sglang_profiler:: kernel-shape ops (patchless path active).

Breaking changes: yes/no No. SGLang < 0.5.18 continues to use the existing patched path; only ≥ 0.5.18 switches to the no-patch mechanism.

PR addresses single concern: yes/no Yes — moving SGLang shape discovery off source patching (the cyclic-import and ruff-format fixes are incidental cleanups required to make the same change pass CI).

Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: N/A — this is a Hyperloom-side change. It depends on the TraceLens kernel_shape_tool (no-patch) being present under TRACELENS_ROOT.

@mohbasit
mohbasit requested review from a team, devalshahamd and haofrank as code owners September 11, 2026 03:57
Comment thread src/hyperloom/orchestrator/actions/executors/_workload_envs.py Fixed
Comment thread src/hyperloom/orchestrator/actions/executors/_workload_envs.py Fixed
@mohbasit mohbasit changed the title code to go to patchless sglang Removing patching for SGLang versions Sep 11, 2026
…cyclic import

Move kernel_shape_tool_dir and resolve_sglang_shape_mode out of the
module-level _server_patcher import and into materialize_config_with_envs,
clearing the CodeQL module-level cyclic-import alerts on those lines.
Matches the function-local pattern in _multi_node_server_lifecycle.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mohbasit
mohbasit force-pushed the feat/remove_patching_tracelens branch from 0f0e97c to b6d9de2 Compare September 11, 2026 14:54

@devalshahamd devalshahamd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One minor comment added.

We can also reduce the added comments at certain places to reduce code size, especially when the added comments refer to what used to be there.

Otherwise LGTM.

# ``_workload_envs`` (matches ``_multi_node_server_lifecycle``).
from ._server_patcher import kernel_shape_tool_dir, resolve_sglang_shape_mode

is_sglang = not is_atom and "vllm" not in fw

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we rely on this- if it is not vLLM or ATOM, must be SGLang?

@BaoYunkai

Copy link
Copy Markdown
Collaborator

Hi @mohbasit @devalshahamd
The no-patch / sitecustomize path looks for TraceLens/TraceUtils/kernel_shape_tool under TRACELENS_ROOT.
With TraceLens at 384c362cb0e174ddf4e533e67ae74df30a849dc6 (Hyperloom default TRACELENS_REF from install.sh), that directory isn’t present, so shape discovery stays off (TRACELENS_SHAPE_DISCOVERY=0).

Could you share a SHA you used for E2E with kernel-shape events in the trace?

@mohbasit

Copy link
Copy Markdown
Collaborator Author

@BaoYunkai This is the SHA with which we have tested the code. It will be in the upcoming tracelens release.

commit d5fdb7de85d84d12f443c2a3178336cbe09205d5 (HEAD -> kernel-shape-profiler-registry, origin/kernel-shape-profiler-registry)

@mohbasit

Copy link
Copy Markdown
Collaborator Author

Hi @BaoYunkai , did you try out this SHA?

@BaoYunkai

BaoYunkai commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Hi @mohbasit,

TraceLens main now includes the kernel shape profiler (#1024) with TraceUtils/kernel_shape_tool, which matches the no-patch / sitecustomize path for SGLang ≥0.5.18 in this PR. However, Hyperloom's TRACELENS_REF still points at an older commit, so a default install will not include kernel_shape_tool and shape discovery will stay off.

Before merge, could you please:

  1. Rebase onto latest main and resolve merge conflicts with main.
  2. Update TRACELENS_REF to the current TraceLens main commit SHA(ef45ada46412b78b1aee80e6394b29b64e407054, or any newer main SHA that still includes kernel_shape_tool).
  3. Ensure CI passes.

Thanks!

@mohbasit

Copy link
Copy Markdown
Collaborator Author

@BaoYunkai I have added the latest commit in the TRACELENS_REF and solved conflict. After an E2E run, we should be good to merge

@mohbasit

Copy link
Copy Markdown
Collaborator Author

Run 1 (roofline off): patchless shape tool ran during profile (kernel_shape_profiler enabled…disabled in server.log, no source patch), but trace was never analyzed.
Run 2 (roofline on): tool ran again and fed trace_analyze → produced a 123 KB TraceLens analysis.md (last_trace_analyze=True).
Both confirm the no-patch sitecustomize works on sglang 0.5.19; Run 2 proved it end-to-end.

@BaoYunkai We are good to merge, i have attached my test result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants