Conversation
qyh111
requested review from
FangRun2,
Infinite666,
Tarrei,
harrisonyhq,
mag1c-h and
ygwpz
as code owners
ucm/store/cache/cc/dump_queue.cc
Outdated
| if (!handle.Ready()) { | ||
| auto cacheStream = stream.NextStream(); | ||
| if (task->desc.compute_event_handle != 0) { | ||
| auto s = cacheStream->WaitEvent( |
There was a problem hiding this comment.
A task has only one event, so it is not necessary to execute a wait in each shard.
ucm/store/detail/type/types.h
Outdated
| using vector::vector; /* Inherit all ctors */ | ||
| std::string brief; /* Description of Task */ | ||
| /** Optional: compute-stream event handle for dump. Cache stream waits before D2H. */ | ||
| uintptr_t compute_event_handle{0}; |
There was a problem hiding this comment.
Naming style does not meet requirements
There was a problem hiding this comment.
This parameter represents the handle to the prerequisite events that the task execution depends on. Using prerequisiteHandle might be better.
ucm/integration/vllm/device.py
Outdated
| super().__init__(device_id) | ||
|
|
||
| def init_device(self): | ||
| torch.cuda.set_device(self.device_id) |
There was a problem hiding this comment.
The context has already been set up where this class is called, so it does not need to be initialized again.
|
|
||
| logger = init_logger(__name__) | ||
|
|
||
| class Device(ABC): |
There was a problem hiding this comment.
The code for runtime difference adaptation already exists, it's best to put the new parts together with them.
…ream synchronize()
| /** Wait for compute-stream event before D2H. Event ptr is platform-specific | ||
| * (cudaEvent_t or aclrtEvent). No-op when event is nullptr. */ | ||
| virtual Status WaitEvent(void* event) | ||
| { |
There was a problem hiding this comment.
It might be better to define it as a pure virtual function, emphasizing that all inheritors must implement it.
2 participants
Purpose
Modifications
Test