-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GPU] set output from variable's memory if kv-cache #27658
[GPU] set output from variable's memory if kv-cache #27658
Conversation
@@ -618,6 +618,7 @@ void primitive_inst::realloc_if_needed() { | |||
_max_output_layout_count[j] = 0; | |||
} | |||
} else { | |||
_outputs[0] = variable.get_memory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some questions about this PR:
- This is setting
_outputs[0]
onrealloc_if_needed
. In case realloc does not happen, are we using proper buffer for_outputs[0]
? I'm afraid this should be set before this function. - Does
shape_predictor
work correctly for multi-ireq scenario? From the code, it seems like that the shape_predictor is placed within the network and it may have issue for multi-ireq scenario as it does not receive ireq information. We may need to take ireq information as an argument for that. Could you comment on it? @sshlyapn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isanghao I think there shouldn't be any problems with shape_predictor, since each ireq has unique instance of shape_predictor (which is passed to the network after the network is assigned to ireq):
std::shared_ptr<cldnn::ShapePredictor> m_shape_predictor = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh you already did the work ;) Thanks for confirmation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sungeunk Could you please describe the issue in more detail? Does this mean that the KV-cache from one inference request is being used in another request in some cases? If so, then we probably need to adjust scales and zero points memory buffers as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sshlyapn Memory of variable-state in two infer-requests are shared at this sample code.minicpm_multi_inferreq.cpp
results of query_state() for llmIR are changed when it calls llmIR2.infer().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sungeunk , thank you for the details. In that case, it seems we have to update the scales/zero points buffers as well
dda463c
to
3efd529
Compare
src/plugins/intel_gpu/tests/functional/subgraph_tests/dynamic/kv_cache.cpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_gpu/tests/functional/subgraph_tests/dynamic/kv_cache.cpp
Outdated
Show resolved
Hide resolved
### Tickets: - 157514
Tickets: