Repository navigation
refactor: make time_stats msgpack-native - #30005
oleksii-tumanov wants to merge 4 commits into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
90b5d76 to
288639a
Compare
oleksii-tumanov
changed the title
288639a to
532f045
Compare
| self.__dict__.update(state) | ||
|
|
||
| struct_fields = {field.name for field in msgspec.structs.fields(type(self))} | ||
| for key, value in state.items(): |
There was a problem hiding this comment.
It seems we could handle both timestamp conversion and setattr in a single iteration to avoid additional performance overhead.
There was a problem hiding this comment.
Implemented in 33f91c8df. setstate now converts timestamps and assigns struct fields in one pass without mutating the input state.
| return TraceNullContext() | ||
|
|
||
| @staticmethod | ||
| def _encode_trace_ctx_ids(trace_ctx_state: Dict[str, Any]) -> Dict[str, Any]: |
There was a problem hiding this comment.
Operations related to trace_ctx would be better implemented in python/sglang/srt/observability/trace.py, by directly modifying TraceReqContext.__getstate__() and __setstate__().
There was a problem hiding this comment.
thanks. I agree, TraceReqContext should be an owner for it and it should be backward compatible with old IDs. It will do a PR a bit bigger.
There was a problem hiding this comment.
Implemented in 33f91c8df. TraceReqContext now owns fixed-width ID serialization and accepts legacy integer IDs; pickle, msgpack, and multimodal JSON round trips are covered.
532f045 to
33f91c8
Compare
33f91c8 to
1040792
Compare
|
Hi @sufeng-buaa, I addressed your comments and rebased the PR onto the latest main. Could you please take another look when you have time? Thanks! |
OK, let me run some key tests locally. |
|
/tag-and-rerun-ci |
|
@sufeng-buaa |
It seems the main branch currently has some bugs, and so does my PR. We need to wait a while for others to fix the bug in the main branch, and then we can re-merge main. |
|
thank you for updates! |
This branch has not been deployed
2 participants
Motivation
Part of #29465. Task 2
Makes request time_stats msgpack-native by removing their remaining PickleWrapper usage.
Modifications
Scheduler IPC snapshot with metrics enabled:
Accuracy Tests
Not applicable. This changes internal IPC serialization and does not affect model outputs.
Speed Tests and Profiling
No end-to-end inference benchmark was run because model execution is unchanged. The representative scheduler snapshot above verifies that default fields are omitted from the serialized payload.
Testing
PYTHONPATH=python python3 -m pytest test/registered/unit/observability/test_trace.py test/registered/unit/managers/test_io_struct.py test/registered/unit/managers/test_multi_tokenizer_mixin.py python/sglang/multimodal_gen/test/unit/test_disagg_trace.py -q(97 passed, 16 subtests passed)pre-commit run --all-files(passed)git diff --check(passed)Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ci.CI States
Latest PR Test (Base): ❌ Run #29791248995
Latest PR Test (Extra): ❌ Run #29791248876