fix: various bugs to eliminate crashes when SGLang overlap scheduling is enabled - #19
Conversation
|
Hi @ivanium, as I tested, the same illegal memory access error still happened. I added |
|
Thanks let me take a deeper look |
343a140 to
d636e23
Compare
Benchmark results
With this PR:This PR without
|
|
While I don't know why it gets faster, it is always good to be fast. -:) |
|
I will test this on GCP later this afternoon. |
|
I tested several times on GCP A100, and I didn't get the error anymore. This seems to work well for now. |
…freeing kv cache; suppress memory leak warnings when kvcached is enabled; add linter hints for in-func imports
…figurable arg. It is enabled for sglang by default for now
… tensors are created; ensure python PageAllocator prealloc thread starts after KV tensors are created
…refactor preallocation thread management to use these no-op implementations when preallocation is disabled
dac6e86 to
068ba2b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses thread safety issues and implements overlap scheduling support for the KV cache management system. The changes enable concurrent operations between worker threads handling CUDA kernels and scheduler threads managing memory allocation/deallocation, while ensuring data consistency and preventing illegal memory accesses.
- Synchronizes CUDA operations before unmapping pages to prevent illegal memory access during concurrent operations
- Implements thread-safe FTensorAllocator and KVCacheManager with proper mutex protection
- Adds support for asynchronous scheduling with null block reservation and background initialization
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| kvcached/tp_ipc_util.py | Adds IPC support for checking KV tensor creation status across workers |
| kvcached/kv_cache_manager.py | Major refactoring to add thread safety, async scheduling, and background initialization |
| kvcached/integration/vllm/interfaces.py | Updates interface to support async scheduling parameter |
| kvcached/integration/sglang/interfaces.py | Updates interface with async scheduling and null block reservation support |
| engine_integration/scripts/kvcached-sglang-v0.4.6.post2.patch | Fixes type conversion and enables async scheduling in SGLang integration |
| engine_integration/benchmark/start_server.sh | Removes overlap schedule disable flag to enable the feature |
| csrc/torch_bindings.cpp | Adds GIL release for thread safety and new KV tensor status checking |
| csrc/inc/ftensor.hpp | Updates comment to reflect thread safety status |
| csrc/inc/constants.hpp | Adds start address constant for virtual memory allocation |
| csrc/inc/allocator.hpp | Adds thread safety with mutex and reorganizes method visibility |
| csrc/ftensor.cpp | Implements deterministic virtual memory allocation using atomic offset |
| csrc/allocator.cpp | Implements comprehensive thread safety with mutex protection |
…d free_ftensor_; remove lock from free_tensor_ to keep code consistency
This PR aims to address issue #5
free_indexasList[int]Close #5
We can optimize the performance and reduce the overhead of CUDA synchronization, but that will be done in another separate PR.