Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

Commit

Permalink
layers:Streamline descriptor mem access update
Browse files Browse the repository at this point in the history 
Put memory access updates for active descriptors directly into the read
and write vectors.
This kills an extra data structure transformation where we moved the
accesses through sets before putting them into vectors. There was no
benefit to the extra overhead and this update brings the performance of
validation with memory access checks on-par with validation when the
checks are disabled.

Still have the warning callback disabled for now.

The ENABLE_MEMORY_ACCESS_CALLBACK define in vk_layer_config.h can be
un-commented to enable the warning callback, as well as the descriptor
memory access tracking.
  • Loading branch information
@tobine
tobine committed Sep 11, 2017
1 parent e1eb757 commit 658e576
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 31 deletions.
10 changes: 3 additions & 7 deletions layers/core_validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5909,8 +5909,8 @@ static bool ValidateCmdDrawType(layer_data *dev_data, VkCommandBuffer cmd_buffer
skip |= (VK_PIPELINE_BIND_POINT_GRAPHICS == bind_point) ? outsideRenderPass(dev_data, *cb_state, caller, msg_code)
: insideRenderPass(dev_data, *cb_state, caller, msg_code);
#ifndef ENABLE_MEMORY_ACCESS_CALLBACK
// TODO : Early return here to skip the memory access checking below. The checks are functional but cause a perf hit
// and the callback that's used is disabled, so also turning off these checks for now.
// Early return here to skip the memory access checking below.
// The callback that's used is disabled, so also turning off these checks for now.
// To re-enable the checks, just remove this early return
return skip;
#endif
Expand All @@ -5924,11 +5924,7 @@ static bool ValidateCmdDrawType(layer_data *dev_data, VkCommandBuffer cmd_buffer
// Pull the set node
cvdescriptorset::DescriptorSet *descriptor_set = state.boundDescriptorSets[setIndex];
if (descriptor_set) {
// For given active slots record updated images & buffers
descriptor_set->GetReadWriteBuffersAndImages(set_binding_pair.second, &mem_access_group.read_buffers,
&mem_access_group.read_images, &mem_access_group.write_buffers,
&mem_access_group.write_images);
UpdateRWMemoryAccessVectors(dev_data, cmd_type, read_accesses, write_accesses, mem_access_group);
descriptor_set->AddReadWriteBuffersAndImages(set_binding_pair.second, cmd_type, read_accesses, write_accesses);
}
}
skip |= ValidateRWMemoryAccesses(dev_data->report_data, cmd_buffer, (*cb_state)->mem_accesses, read_accesses,
Expand Down
56 changes: 38 additions & 18 deletions layers/descriptor_sets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,12 +552,11 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t,
return true;
}

// For given bindings, place any update buffers or images into the passed-in unordered_sets
uint32_t cvdescriptorset::DescriptorSet::GetReadWriteBuffersAndImages(const std::map<uint32_t, descriptor_req> &bindings,
std::unordered_set<VkBuffer> *read_buffer_set,
std::unordered_set<VkImageView> *read_image_set,
std::unordered_set<VkBuffer> *write_buffer_set,
std::unordered_set<VkImageView> *write_image_set) const {
// For given bindings, place any active buffers or images into the passed-in vectors
// TODO: Currently just adding entire binding as imprecise update, but ideally would limit access based on view
uint32_t cvdescriptorset::DescriptorSet::AddReadWriteBuffersAndImages(const std::map<uint32_t, descriptor_req> &bindings,
CMD_TYPE cmd, std::vector<MemoryAccess> *read_access_vector,
std::vector<MemoryAccess> *write_access_vector) const {
auto num_updates = 0;
for (auto binding_pair : bindings) {
auto binding = binding_pair.first;
Expand All @@ -566,27 +565,41 @@ uint32_t cvdescriptorset::DescriptorSet::GetReadWriteBuffersAndImages(const std:
continue;
}
auto start_idx = p_layout_->GetGlobalStartIndexFromBinding(binding);
auto &buffer_set = read_buffer_set;
auto &image_set = read_image_set;
auto &update_vector = read_access_vector;
auto update_function = &AddReadMemoryAccess;
if (descriptors_[start_idx]->IsStorage()) {
buffer_set = write_buffer_set;
image_set = write_image_set;
update_vector = write_access_vector;
update_function = &AddWriteMemoryAccess;
}
switch (descriptors_[start_idx]->descriptor_class) {
case (Image): {
for (uint32_t i = 0; i < p_layout_->GetDescriptorCountFromBinding(binding); ++i) {
if (descriptors_[start_idx + i]->updated) {
image_set->insert(static_cast<ImageDescriptor *>(descriptors_[start_idx + i].get())->GetImageView());
num_updates++;
auto image_view = static_cast<ImageDescriptor *>(descriptors_[start_idx + i].get())->GetImageView();
auto image_view_state = GetImageViewState(device_data_, image_view);
if (image_view_state) {
auto image_node = GetImageState(device_data_, image_view_state->create_info.image);
if (image_node) {
update_function(cmd, update_vector, image_node->binding, false);
num_updates++;
}
}
}
}
break;
}
case (ImageSampler): {
for (uint32_t i = 0; i < p_layout_->GetDescriptorCountFromBinding(binding); ++i) {
if (descriptors_[start_idx + i]->updated) {
image_set->insert(static_cast<ImageSamplerDescriptor *>(descriptors_[start_idx + i].get())->GetImageView());
num_updates++;
auto image_view = static_cast<ImageSamplerDescriptor *>(descriptors_[start_idx + i].get())->GetImageView();
auto image_view_state = GetImageViewState(device_data_, image_view);
if (image_view_state) {
auto image_node = GetImageState(device_data_, image_view_state->create_info.image);
if (image_node) {
update_function(cmd, update_vector, image_node->binding, false);
num_updates++;
}
}
}
}
break;
Expand All @@ -597,8 +610,11 @@ uint32_t cvdescriptorset::DescriptorSet::GetReadWriteBuffersAndImages(const std:
auto bufferview = static_cast<TexelDescriptor *>(descriptors_[start_idx + i].get())->GetBufferView();
auto bv_state = GetBufferViewState(device_data_, bufferview);
if (bv_state) {
buffer_set->insert(bv_state->create_info.buffer);
num_updates++;
auto buff_state = GetBufferState(device_data_, bv_state->create_info.buffer);
if (buff_state) {
update_function(cmd, update_vector, buff_state->binding, false);
num_updates++;
}
}
}
}
Expand All @@ -607,8 +623,12 @@ uint32_t cvdescriptorset::DescriptorSet::GetReadWriteBuffersAndImages(const std:
case (GeneralBuffer): {
for (uint32_t i = 0; i < p_layout_->GetDescriptorCountFromBinding(binding); ++i) {
if (descriptors_[start_idx + i]->updated) {
buffer_set->insert(static_cast<BufferDescriptor *>(descriptors_[start_idx + i].get())->GetBuffer());
num_updates++;
auto buff_state = GetBufferState(
device_data_, static_cast<BufferDescriptor *>(descriptors_[start_idx + i].get())->GetBuffer());
if (buff_state) {
update_function(cmd, update_vector, buff_state->binding, false);
num_updates++;
}
}
}
break;
Expand Down
10 changes: 4 additions & 6 deletions layers/descriptor_sets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,10 @@ class DescriptorSet : public BASE_NODE {
// For given bindings validate state at time of draw is correct, returning false on error and writing error details into string*
bool ValidateDrawState(const std::map<uint32_t, descriptor_req> &, const std::vector<uint32_t> &, const GLOBAL_CB_NODE *,
const char *caller, std::string *) const;
// For given set of bindings, add any buffers and images that will be updated to their respective unordered_sets & return number
// of objects inserted
uint32_t GetReadWriteBuffersAndImages(const std::map<uint32_t, descriptor_req> &, std::unordered_set<VkBuffer> *read_buffer_set,
std::unordered_set<VkImageView> *read_image_set,
std::unordered_set<VkBuffer> *write_buffer_set,
std::unordered_set<VkImageView> *write_image_set) const;
// For given set of bindings, add memory bindings for buffers and images that will be updated to respective read & write vectors
uint32_t AddReadWriteBuffersAndImages(const std::map<uint32_t, descriptor_req> &bindings, CMD_TYPE cmd,
std::vector<MemoryAccess> *read_access_vector,
std::vector<MemoryAccess> *write_access_vector) const;

// Descriptor Update functions. These functions validate state and perform update separately
// Validate contents of a WriteUpdate
Expand Down
2 changes: 2 additions & 0 deletions layers/vk_layer_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
extern "C" {
#endif

//#define ENABLE_MEMORY_ACCESS_CALLBACK

// Definitions for Debug Actions
typedef enum VkLayerDbgActionBits {
VK_DBG_LAYER_ACTION_IGNORE = 0x00000000,
Expand Down

0 comments on commit 658e576

Please sign in to comment.