Best Practices/Assistant/Performance Layer

[mark-lunarg]

This issue is a place holder for a utility layer which provides application feedback for performance-related checks. For details and to add items, please see/add to the LunarG Github VulkanTools project Wiki or add an issue to this repo or VulkanTools and we'll add to the master list.

The following Github issues have been created and are now tracked by this issue -- completed checks are currently present in the assistant_layer, available in the Github VulkanTools repository, and soon to be in the Vulkan SDK.

• [ ] #2535 Validate getter's structure count
• [ ] #2486 Potentially false validation error if image size > maxResourceSize
• [ ] Add warnings for non-success driver return values
• [ ] Warn for skipping Get calls -- GetBufferMemoryRequirements, GetImageMemoryRequirements, etc.
• [ ] Move validation WARNINGs not explicitly defined by the specification to the assistant layer
• [x] #2248 validation: Add a warning for LOAD_OP_LOAD + LAYOUT_UNDEFINED (note that this check will be present in the core_validation layer).
• [x] #2137 Add warning on mismatch between sharingMode and queueFamilyIndexCount
• [ ] #2089 No error when recreating swapchain while rendering to an existing swapchain image
• [ ] #2034 Need validation layer to add redundancy check for vulkan states.
• [x] #1912 Add warning when a parameter of vkCmdDispatch is 0
• [x] #1807 Give error when requesting device extensions on instance creation and vice versa
• [x] #1696 Performance hint: Warn if application does not use pipeline cache
• [ ] #1628 VK_LAYER_LUNARG_core_validation queue submissions list grows forever and causes crash
• [x] #1401 Performance warning: VK_PIPELINE_STAGE_ALL_COMMANDS_BIT or VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT
• [ ] #1253 Performance warning: Pipeline barriers used for layout transitions instead of Render Passes
• [x] #1155 Performance warning: High number of memory objects
• [ ] #1407 Performance warning: Detect over-synchronization
• [x] #727 Performance warning: when instanceCount = 0 (or drawCount = 0)

Portability checks as suggested by Ecosystem Issue #11

• [ ] RelaxedPrecision / mediump. If you use mediump and only test on desktop, the result might be inaccurate when porting to mobile.

• [ ] VK_ATTACHMENT_LOAD_OP_DONT_CARE, STORE_OP_DONT_CARE: The implementation is free to turn the image into garbage here, but some implementations may simply behave as-if LOAD_OP_LOAD was used. An application using DONT_CARE by mistake can easily run just fine.

• [ ] Transitions from VK_IMAGE_LAYOUT_UNDEFINED to anything. The implementation is free to turn the image into garbage here, but implementations which do not concern themselves with image layouts might preserve the image. I have had bugs in the past where I transitions from UNDEFINED but I actually meant to preserve the image, and didn't see it until running on AMD.

• [ ] pPreserveAttachments in multipass. If you don't use an attachment in a subpass, the implementation is free to thrash the attachment unless pPreserveAttachments is used. An implementation may preserve anyways which could lead to some awkward debugging sessions on other hardware.

• [ ] Binding subpass input attachments. Currently, a shader will declare input_attachment_index as well as set/binding. For implementations which do not need an actual texture bound to a descriptor set to sample from subpassInput, it's easy to forget, and code can happily run. The wrong image can also be bound, which could cause some weird mismatch. This is a case where code can run "just fine" on mobile, but not desktop. I'm not sure if this is a validation error or not.

• [ ] Aliasing VkMemory and memory corruption. Currently if you have aliased optimal images or aliased buffers and optimal images, if you modify one alias, all other aliases are trashed unless they are all in host-visible LINEAR layouts. On some implementations, it might work "just fine" to alias images, but not others.

• [ ] Initial values of VkMemory. Sometimes the memory can just be all zero when allocating it, and some implementations may come to rely on behavior like this. The spec does mention there might be requirements here, so this could be moot if all relevant OSes supporting Vulkan requires this.

  For instance, if an operating system guarantees that data in all its memory
  allocations are set to zero when newly allocated, the Vulkan implementation must
  make the same guarantees for any allocations it controls (e.g. VkDeviceMemory).

• [ ] VkDescriptorPool might not actually be a strict pool and you can keep allocating from it indefinitely.

• [ ] Command buffer pool behavior is interesting. Some apps may rely on vkFreeCommandBuffers reclaiming memory even when not using COMMAND_BUFFER_RESET_BIT. This can happen to work on implementations which do not pool command buffer allocation.

• [ ] More tricky ones like forgetting vkFlushMappedMemoryRanges and vkInvalidateMappedMemoryRange on incoherent memory ranges can "happen" to work, although this is more of a CPU thing. GPU writes can also "happen" to become visible to the CPU without a pipeline barrier using HOST_WRITE_BIT.

[karl-lunarg]

1628 "VK_LAYER_LUNARG_core_validation queue submissions list grows forever and causes crash" might be another candidate condition to check for in this layer.

[RippeR37]

Idea: Maybe this layer could also check if same state is set multiple times in a row which can lead to lower performance?

[mark-lunarg]

From @Tony-LunarG: Assuming that the 99% common workflow is [BeginCommandBuffer; write commands to command buffer; EndCommandBuffer], it would be nice to warn developer if the code is recording two command buffers simultaneously. Would also need to validate assumption about workflow. Note that it needs to be thread smart since we expect multiple threads to be recording multiple command buffers simultaneously

[MarkY-LunarG]

Idea: Detect case of LVL issue #1917 and warn.

[karl-lunarg]

Idea: #2061 - Add a layer that writes a unique ID to a CPU-visible buffer whenever a marker is encountered. After a GPU crash, the buffer can be examined to see the last ID written.

[HansKristian-Work]

Is this supposed to be a generic layer for best practices on Vulkan which collects best practices from a spec POV, or would there be room for IHV specifics as well (which are probably where the most useful/interesting checks will be)?

[mark-lunarg]

@HansKristian-ARM, the Assistant Layer effort is currently focused on the former. However, it is a simple matter to create related assistant-type layers specific to a particular IHV. The aim of the layer framework is to make the creation of these types of layers much more straightforward.

[hrydgard]

Regarding transitions from VK_IMAGE_LAYOUT_UNDEFINED to anything: When initializing textures with known data, I:

• create with VK_IMAGE_LAYOUT_UNDEFINED
• transition to VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL (for the src stage, I use VK_PIPELINE_STAGE_ALL_COMMANDS)
• vkCmdCopyBufferToImage to write the contents

• transition to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL

  As this sequence will trigger a couple of the proposed checks in this issue, what's the recommended way to do this? PREINITIALIZED is no good because I don't want to create a linear image.

It seems to me that it would make sense to allow creating images directly in VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL and copy to them , I don't quite understand why that's not allowed.

[krOoze]

As this sequence will trigger a couple of the proposed checks in this issue, what's the recommended way to do this?

It will? You are making the contents defined in step 3, so the layers should not complain. Your sequence seems fine to me (except perhaps the wasteful VK_PIPELINE_STAGE_ALL_COMMANDS).

creating images directly in VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL and copy to them , I don't quite understand why that's not allowed.

I would assume the same reason why PREINITIALIZED is distinct from GENERAL. It ensures there will be at least one barrier before the first use of the Image by the device.

[hrydgard]

It will?

This one: "Transitions from VK_IMAGE_LAYOUT_UNDEFINED to anything".

wasteful VK_PIPELINE_STAGE_ALL_COMMANDS

What src pipeline stage would you suggest for transitioning from UNDEFINED? Simply BOTTOM_OF_PIPE?

EDIT: Well TOP_OF_PIPE of course, thanks. Keep confusing them.

[krOoze]

It will?

This one: "Transitions from VK_IMAGE_LAYOUT_UNDEFINED to anything".

You need to read the rest of that paragraph. It concerns itself with a situation where someone transitions from UNDEFINED and then reads the contents. The contents become undefined, so (depending on platform) it may contain original data or it may be trashed upon the read.

But you are writing the content, which makes it defined again. So it should not trigger any such checks. Check implemented in a way which interferes with your case would indeed be useless. It is a very common case, and it cannot be done any differently for TILING_OPTIMAL images.

What src pipeline stage would you suggest for transitioning from UNDEFINED? Simply BOTTOM_OF_PIPE?

New Image\Memory does not have any outstanding read\write operations on it to synchronize against. So, assuming you are not merging other barriers to this, srcStage = TOP_OF_PIPE (i.e. no execution dependency) will suffice.