Skip to content

Commit 3c6edc5

Browse files
author
ssjia
committed
[ET-VK] Fix pipeline indexing bug in batch create_pipelines
The original code created pipelines for all descriptors but used a separate index counter during cache insertion that only incremented for non-cached entries. This caused incorrect pipeline handles to be associated with keys when some descriptors were already cached, and leaked handles for duplicates. Fix by filtering out already-cached descriptors upfront, ensuring 1:1 correspondence between the created pipelines array and cache insertions. Authored with Claude. Differential Revision: [D92171360](https://our.internmc.facebook.com/intern/diff/D92171360/) [ghstack-poisoned]
1 parent ef49d27 commit 3c6edc5

1 file changed

Lines changed: 21 additions & 10 deletions

File tree

backends/vulkan/runtime/vk_api/Pipeline.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,22 @@ void ComputePipelineCache::create_pipelines(
459459
const std::unordered_set<Key, Hasher>& descriptors) {
460460
std::lock_guard<std::mutex> lock(cache_mutex_);
461461

462-
const auto num_pipelines = descriptors.size();
462+
// Filter out descriptors already in cache to avoid creating duplicate
463+
// pipelines and to ensure correct indexing between created pipelines and
464+
// cache insertion.
465+
std::vector<Key> keys_to_create;
466+
keys_to_create.reserve(descriptors.size());
467+
for (const auto& key : descriptors) {
468+
if (cache_.find(key) == cache_.cend()) {
469+
keys_to_create.push_back(key);
470+
}
471+
}
472+
473+
if (keys_to_create.empty()) {
474+
return;
475+
}
476+
477+
const auto num_pipelines = keys_to_create.size();
463478
std::vector<VkPipeline> pipelines(num_pipelines);
464479

465480
std::vector<std::vector<VkSpecializationMapEntry>> map_entries;
@@ -474,7 +489,7 @@ void ComputePipelineCache::create_pipelines(
474489
std::vector<VkComputePipelineCreateInfo> create_infos;
475490
create_infos.reserve(num_pipelines);
476491

477-
for (auto& key : descriptors) {
492+
for (const auto& key : keys_to_create) {
478493
map_entries.push_back(key.specialization_constants.generate_map_entries());
479494

480495
specialization_infos.push_back(VkSpecializationInfo{
@@ -513,14 +528,10 @@ void ComputePipelineCache::create_pipelines(
513528
nullptr,
514529
pipelines.data()));
515530

516-
uint32_t i = 0;
517-
for (auto& key : descriptors) {
518-
auto it = cache_.find(key);
519-
if (it != cache_.cend()) {
520-
continue;
521-
}
522-
cache_.insert({key, ComputePipelineCache::Value(device_, pipelines[i])});
523-
++i;
531+
for (size_t i = 0; i < keys_to_create.size(); ++i) {
532+
cache_.insert(
533+
{keys_to_create[i],
534+
ComputePipelineCache::Value(device_, pipelines[i])});
524535
}
525536
}
526537

0 commit comments

Comments
 (0)