Skip to content
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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 2, 2024
Merged

[GPU] set output from variable's memory if kv-cache #27658

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4c1aa5
set output from variable's memory if kv-cache
sungeunk Nov 21, 2024
3efd529
add a test-case
sungeunk Nov 25, 2024
eebe767
set outputs for scale/zp
sungeunk Nov 28, 2024
1ebcb33
update naming
sungeunk Nov 28, 2024
9a046c2
fixed cpplint issues
sungeunk Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/intel_gpu/src/graph/primitive_inst.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ void primitive_inst::realloc_if_needed() {
_max_output_layout_count[j] = 0;
}
} else {
_outputs[0] = variable.get_memory();
Copy link
Contributor

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] on realloc_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

Copy link
Contributor

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):

openvino/src/plugins/intel_gpu/include/intel_gpu/plugin/sync_infer_request.hpp

Line 89 in 0f149e3

std::shared_ptr<cldnn::ShapePredictor> m_shape_predictor = nullptr;

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor Author

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().

Copy link
Contributor

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

GPU_DEBUG_TRACE_DETAIL << id() << " : realloc_if_needed: can_be_optimized = false and memories are not being shared" << std::endl;
}
sungeunk marked this conversation as resolved.
Show resolved Hide resolved
} else {
Expand Down
73 changes: 73 additions & 0 deletions src/plugins/intel_gpu/tests/functional/subgraph_tests/dynamic/kv_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,4 +530,77 @@ TEST_F(KVCacheTests, smoke_multipleIterations_stateful_with_set_state) {
this->test_smoke_multipleIterations_stateful(false, true, true, 1, 2, ov::element::f16, 5, 1, true);
}

class KVCacheIssueTests: public ::testing::Test {
public:
void test_smoke_conflicted_memory_for_two_inf_req() {
#if defined(ANDROID)
GTEST_SKIP();
#endif
auto core = ov::test::utils::PluginCache::get().core();

ov::AnyMap properties = {
ov::hint::kv_cache_precision(ov::element::undefined)
};

const size_t n_batch = 1;
const size_t n_heads = 32;
const size_t n_features = 10;
const size_t context_size = 20;
ov::element::Type element_type = ov::element::f16;

const bool stateful = true;

auto model = tests::make_llm_kv_cache_pattern(n_batch,
n_heads,
n_features,
element_type,
2,
stateful,
false,
stateful);
auto compiled_model = core->compile_model(model, ov::test::utils::DEVICE_GPU, properties);

auto input0 = model->get_parameters().at(0);
auto input1 = model->get_parameters().at(1);

auto infer_request1 = compiled_model.create_infer_request();
auto infer_request2 = compiled_model.create_infer_request();

auto tensor1_input1 = ov::test::utils::create_and_fill_tensor_real_distribution(element_type, {n_batch, context_size, n_heads, n_features}, -0.5f, 0.5f, 1);
sungeunk marked this conversation as resolved.
Show resolved Hide resolved
auto tensor1_input2 = ov::test::utils::create_and_fill_tensor_real_distribution(element_type, {n_batch, n_heads, context_size, context_size}, -0.5f, 0.5f, 1);
infer_request1.set_tensor(input0, tensor1_input1);
infer_request1.set_tensor(input1, tensor1_input2);

auto tensor2_input1 = ov::test::utils::create_and_fill_tensor_real_distribution(element_type, {n_batch, context_size + 1, n_heads, n_features}, -0.5f, 0.5f, 555);
auto tensor2_input2 = ov::test::utils::create_and_fill_tensor_real_distribution(element_type, {n_batch, n_heads, context_size + 1, context_size + 1}, -0.5f, 0.5f, 555);
infer_request2.set_tensor(input0, tensor2_input1);
infer_request2.set_tensor(input1, tensor2_input2);

std::stringstream oss1;
std::stringstream oss2;
for (auto&& state : infer_request1.query_state()) {
state.reset();
}
infer_request1.infer();
for (auto&& state : infer_request1.query_state()) {
oss1.write((char*)state.get_state().data(), state.get_state().get_byte_size());
}

for (auto&& state : infer_request2.query_state()) {
state.reset();
}
infer_request2.infer();
for (auto&& state : infer_request1.query_state()) {
oss2.write((char*)state.get_state().data(), state.get_state().get_byte_size());
}

ASSERT_TRUE(oss1.str() == oss2.str());
}
};

TEST_F(KVCacheIssueTests, smoke_issue_cases) {
sungeunk marked this conversation as resolved.
Show resolved Hide resolved
this->test_smoke_conflicted_memory_for_two_inf_req();
}


} // namespace
Loading