Repository navigation
Concurrency / thread safety Issue with Python TrT-LLM example 聽#225
Description
Hello NVIDIA team,
Thank you developing and open sourcing this framework. 馃檹
It seems like current python example for running TensorRT-LLM is not thread-safe and I wanted to bring this up.
In TensorRT guidelines, TensorRT session is thread-safe as long as new ExecutionContext is used for each thread. [1]
[1]: https://docs.nvidia.com/deeplearning/tensorrt/archives/tensorrt-803/best-practices/index.html#thread-safety
However, in out python TensorRT GenerationSession 2, it seems like we are using the same ExecutionContext 3 4 for a single GenerationSession.
This would make calling GenerationSession::decode function in concurrent manner unsafe.
Would there be a new GenerationSession where new ExecutionContext is created for each call to decode?
Please let me know if my understanding is incorrect.
Activity
juney-nvidia commentedon Nov 1, 2023
@hchoi-moveworks Hi, thanks for your interest in our work!
As to your question, currently
GenerationSessionis not thread-safe and to ensure thread safety, differentGenerationSessioninstances need to be created.There are several reasons why it is designed in such a way:
Thanks
June
cody-moveworks commentedon Nov 1, 2023
@juney-nvidia Hi, thanks for your response! I have a couple of questions:
GenerationSessionencapsulates both thetrt.ICudaEngineinstance and thetrt.IExecutionContextinstances used for inference. My understanding is that thetrt.ICudaEngineinstance can be reused across threads, and that thread-safe execution can be accomplished by creating separatetrt.IExecutionContextinstances for each thread. Is that correct?GenerationSessioninstances, then we would also be loading duplicatetrt.ICudaEngineinstances, effectively loading the model weights into GPU memory multiple times. If we are using multi-threading for GPU inference in Python, it seems our best bet would be to create a separateThreadSafeGenerationSessionclass that decouples thetrt.ICudaEngineinstance from thetrt.IExecutionContextinstances so that thetrt.ICudaEngineinstance can be reused across multiple instances of said class. Does this approach seem reasonable?litaotju commentedon Nov 3, 2023
@cody-moveworks
Though, I'd like to understand the usage better here, due to python GIL, I think the multiple threading in python is not real multithread, so what's the benefit here would be? If there is large perf differences here, we can consider to enhance the thread safety here.
litaotju commentedon Dec 5, 2023
I am closing this issue, since no update after my last comment. Pls re-open if you disagree. Thanks.