Vulkan icon indicating copy to clipboard operation
Vulkan copied to clipboard

PipelineStatistics Example bugs that might mislead readers

Open Erfan-Ahmadi opened this issue 2 years ago • 1 comments

Issue 1 - The Query count:

https://github.com/SaschaWillems/Vulkan/blob/28397adb20db03efa1b6362075b13707bc70af93/examples/pipelinestatistics/pipelinestatistics.cpp#L125

The query count for PipelineStatistics need to be only 1 here. The result of our only 1 query will write to an array uint64_t's,

Pipeline statistics queries write one integer value for each bit that is enabled in the pipelineStatistics when the pool is created, and the statistics values are written in bit order starting from the least significant bit. Timestamp queries write one integer value.

Also you can see in your own code that you're only getting the result of only 1 query vkGetQueryPoolResults https://github.com/SaschaWillems/Vulkan/blob/28397adb20db03efa1b6362075b13707bc70af93/examples/pipelinestatistics/pipelinestatistics.cpp#L133-L137

Issue 2 - The Stride:

https://github.com/SaschaWillems/Vulkan/blob/28397adb20db03efa1b6362075b13707bc70af93/examples/pipelinestatistics/pipelinestatistics.cpp#L133-L140

When you try to have more than 1 pipelineStatistics query in your example. the stride here in vkGetQueryPoolResults (currently set to sizeof(uint64_t)) will be wrong here.

Here is my test code that works with 2 queries in your example getQueryResults function:

uint32_t each_count = static_cast<uint32_t>(pipelineStatNames.size()); // will be 8 most probably 
uint32_t all_count = static_cast<uint32_t>(pipelineStats.size()); // will be 16 because of 2 queries (8 * 2)
vkGetQueryPoolResults(
	device,
	queryPool,
	0,
	2, // num queries = 2
	all_count * sizeof(uint64_t), // Size of the result of the 2 queries
	pipelineStats.data(),
	each_count * sizeof(uint64_t), // The stride for each query -> query 1 writes to [0..each_count ) and query 2 writes to [each_count ...each_count * 2) indices of pipelineStats
	VK_QUERY_RESULT_64_BIT);

In this code snippet our pipelineStats memory is written in a way that query 1 writes to [0..8) and query 2 writes to [8...16) indices of pipelineStats.

In this case the stride must be equal to 8 * sizeof(uint64_t)

Reminder: 8 is because there is 1 for each: input assm vertex count, Input assembly primitives count, ... , Tess. eval. shader invocations

You didn't encounter any bug because you only have 1 query and thus stride doesn't have any effect here.

If you had 2 queries you would've noticed that the result of query 2 would overwrite a part of query 1 result in the pData

Also I tested all of my assumptions and ran your projects.

Erfan-Ahmadi avatar Sep 02 '21 10:09 Erfan-Ahmadi

Thanks for your detailed breakdown of this issue. I'll take a look at this and provide a fix.

SaschaWillems avatar Sep 04 '21 07:09 SaschaWillems