From e0a588752c827daef74960bbc80d7f5c4f73c4dd Mon Sep 17 00:00:00 2001 From: Hao Li Date: Wed, 10 Feb 2021 14:01:36 +0000 Subject: [PATCH] Fix timestamp writing and resolving from different encoders Currently the queries availability info is only stored on the encoders where they are written, and we need the info in the compute shader. If resolving them from a different encoder, 0s are returned because the queries are not written on resolving encoder. Besides the encoders, we also need to add availability info to query set, and use it in compute shader instead. When resolving query set without any written, we need to reset queries before resolveQuerySet based on the query set availability on Vulkan. Added more end2end tests for these cases. Bug: dawn:645 Change-Id: I09bf1230934aa885587fa4f671925940c1795cd9 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/39740 Reviewed-by: Corentin Wallez Commit-Queue: Hao Li --- src/dawn_native/CommandEncoder.cpp | 17 ++--- src/dawn_native/QuerySet.cpp | 10 +++ src/dawn_native/QuerySet.h | 6 ++ src/dawn_native/vulkan/CommandBufferVk.cpp | 51 ++++++++++++-- src/tests/end2end/QueryTests.cpp | 81 ++++++++++++++++++++++ 5 files changed, 153 insertions(+), 12 deletions(-) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 7ffa1cd7f1..f3c1622531 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -405,13 +405,11 @@ namespace dawn_native { uint64_t destinationOffset) { DeviceBase* device = encoder->GetDevice(); - std::vector availability; - auto it = encoder->GetQueryAvailabilityMap().find(querySet); - if (it != encoder->GetQueryAvailabilityMap().end()) { - availability = {it->second.begin(), it->second.end()}; - } else { - availability.resize(querySet->GetQueryCount()); - } + // The availability got from query set is a reference to vector, need to covert + // bool to uint32_t due to a user input in pipeline must not contain a bool type in + // WGSL. + std::vector availability{querySet->GetQueryAvailability().begin(), + querySet->GetQueryAvailability().end()}; // Timestamp availability storage buffer BufferDescriptor availabilityDesc = {}; @@ -420,7 +418,7 @@ namespace dawn_native { Ref availabilityBuffer = AcquireRef(device->CreateBuffer(&availabilityDesc)); device->GetQueue()->WriteBuffer(availabilityBuffer.Get(), 0, availability.data(), - querySet->GetQueryCount() * sizeof(uint32_t)); + availability.size() * sizeof(uint32_t)); // Timestamp params uniform buffer TimestampParams params = {queryCount, static_cast(destinationOffset), @@ -462,6 +460,9 @@ namespace dawn_native { TrackUsedQuerySet(querySet); } + // Set the query at queryIndex to available for resolving in query set. + querySet->SetQueryAvailability(queryIndex, 1); + // Gets the iterator for that querySet or create a new vector of bool set to false // if the querySet wasn't registered. auto it = mQueryAvailabilityMap.emplace(querySet, querySet->GetQueryCount()).first; diff --git a/src/dawn_native/QuerySet.cpp b/src/dawn_native/QuerySet.cpp index c98c8ca009..d181e36423 100644 --- a/src/dawn_native/QuerySet.cpp +++ b/src/dawn_native/QuerySet.cpp @@ -107,6 +107,8 @@ namespace dawn_native { for (uint32_t i = 0; i < descriptor->pipelineStatisticsCount; i++) { mPipelineStatistics.push_back(descriptor->pipelineStatistics[i]); } + + mQueryAvailability.resize(descriptor->count); } QuerySetBase::QuerySetBase(DeviceBase* device, ObjectBase::ErrorTag tag) @@ -135,6 +137,14 @@ namespace dawn_native { return mPipelineStatistics; } + const std::vector& QuerySetBase::GetQueryAvailability() const { + return mQueryAvailability; + } + + void QuerySetBase::SetQueryAvailability(uint32_t index, bool available) { + mQueryAvailability[index] = available; + } + MaybeError QuerySetBase::ValidateCanUseInSubmitNow() const { ASSERT(!IsError()); if (mState == QuerySetState::Destroyed) { diff --git a/src/dawn_native/QuerySet.h b/src/dawn_native/QuerySet.h index a8f4deace6..df36b2ada4 100644 --- a/src/dawn_native/QuerySet.h +++ b/src/dawn_native/QuerySet.h @@ -35,6 +35,9 @@ namespace dawn_native { uint32_t GetQueryCount() const; const std::vector& GetPipelineStatistics() const; + const std::vector& GetQueryAvailability() const; + void SetQueryAvailability(uint32_t index, bool available); + MaybeError ValidateCanUseInSubmitNow() const; void Destroy(); @@ -56,6 +59,9 @@ namespace dawn_native { enum class QuerySetState { Unavailable, Available, Destroyed }; QuerySetState mState = QuerySetState::Unavailable; + + // Indicates the available queries on the query set for resolving + std::vector mQueryAvailability; }; } // namespace dawn_native diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 1c7cdd28ef..98bfb58dc4 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -34,6 +34,8 @@ #include "dawn_native/vulkan/UtilsVulkan.h" #include "dawn_native/vulkan/VulkanError.h" +#include + namespace dawn_native { namespace vulkan { namespace { @@ -386,6 +388,48 @@ namespace dawn_native { namespace vulkan { device->fn.CmdWriteTimestamp(commands, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, querySet->GetHandle(), cmd->queryIndex); } + + void RecordResolveQuerySetCmd(VkCommandBuffer commands, + Device* device, + QuerySet* querySet, + uint32_t firstQuery, + uint32_t queryCount, + Buffer* destination, + uint64_t destinationOffset) { + const std::vector& availability = querySet->GetQueryAvailability(); + + auto currentIt = availability.begin() + firstQuery; + auto lastIt = availability.begin() + firstQuery + queryCount; + + // Traverse available queries in the range of [firstQuery, firstQuery + queryCount - 1] + while (currentIt != lastIt) { + auto firstTrueIt = std::find(currentIt, lastIt, true); + // No available query found for resolving + if (firstTrueIt == lastIt) { + break; + } + auto nextFalseIt = std::find(firstTrueIt, lastIt, false); + + // The query index of firstTrueIt where the resolving starts + uint32_t resolveQueryIndex = std::distance(availability.begin(), firstTrueIt); + // The queries count between firstTrueIt and nextFalseIt need to be resolved + uint32_t resolveQueryCount = std::distance(firstTrueIt, nextFalseIt); + + // Calculate destinationOffset based on the current resolveQueryIndex and firstQuery + uint32_t resolveDestinationOffset = + destinationOffset + (resolveQueryIndex - firstQuery) * sizeof(uint64_t); + + // Resolve the queries between firstTrueIt and nextFalseIt (which is at most lastIt) + device->fn.CmdCopyQueryPoolResults( + commands, querySet->GetHandle(), resolveQueryIndex, resolveQueryCount, + destination->GetHandle(), resolveDestinationOffset, sizeof(uint64_t), + VK_QUERY_RESULT_64_BIT | VK_QUERY_RESULT_WAIT_BIT); + + // Set current interator to next false + currentIt = nextFalseIt; + } + } + } // anonymous namespace // static @@ -731,10 +775,9 @@ namespace dawn_native { namespace vulkan { cmd->queryCount * sizeof(uint64_t)); destination->TransitionUsageNow(recordingContext, wgpu::BufferUsage::CopyDst); - device->fn.CmdCopyQueryPoolResults( - commands, querySet->GetHandle(), cmd->firstQuery, cmd->queryCount, - destination->GetHandle(), cmd->destinationOffset, sizeof(uint64_t), - VK_QUERY_RESULT_64_BIT | VK_QUERY_RESULT_WAIT_BIT); + RecordResolveQuerySetCmd(commands, device, querySet, cmd->firstQuery, + cmd->queryCount, destination, cmd->destinationOffset); + break; } diff --git a/src/tests/end2end/QueryTests.cpp b/src/tests/end2end/QueryTests.cpp index e78c2f494b..aee3cc7dd9 100644 --- a/src/tests/end2end/QueryTests.cpp +++ b/src/tests/end2end/QueryTests.cpp @@ -404,6 +404,87 @@ TEST_P(TimestampQueryTests, TimestampOnComputePass) { EXPECT_BUFFER(destination, 0, kQueryCount * sizeof(uint64_t), new TimestampExpectation); } +// Test resolving timestamp query from another different encoder +TEST_P(TimestampQueryTests, ResolveFromAnotherEncoder) { + // TODO(hao.x.li@intel.com): Fix queries reset on Vulkan backend, it does not allow to resolve + // unissued queries. Currently we reset the whole query set at the beginning of command buffer + // creation. + DAWN_SKIP_TEST_IF(IsVulkan()); + + constexpr uint32_t kQueryCount = 2; + + wgpu::QuerySet querySet = CreateQuerySetForTimestamp(kQueryCount); + wgpu::Buffer destination = CreateResolveBuffer(kQueryCount * sizeof(uint64_t)); + + wgpu::CommandEncoder timestampEncoder = device.CreateCommandEncoder(); + timestampEncoder.WriteTimestamp(querySet, 0); + timestampEncoder.WriteTimestamp(querySet, 1); + wgpu::CommandBuffer timestampCommands = timestampEncoder.Finish(); + queue.Submit(1, ×tampCommands); + + wgpu::CommandEncoder resolveEncoder = device.CreateCommandEncoder(); + resolveEncoder.ResolveQuerySet(querySet, 0, kQueryCount, destination, 0); + wgpu::CommandBuffer resolveCommands = resolveEncoder.Finish(); + queue.Submit(1, &resolveCommands); + + EXPECT_BUFFER(destination, 0, kQueryCount * sizeof(uint64_t), new TimestampExpectation); +} + +// Test resolving timestamp query correctly if the queries are written sparsely +TEST_P(TimestampQueryTests, ResolveSparseQueries) { + // TODO(hao.x.li@intel.com): Fix queries reset and sparsely resolving on Vulkan backend, + // otherwise its validation layer reports unissued queries resolving error + DAWN_SKIP_TEST_IF(IsVulkan() && IsBackendValidationEnabled()); + + constexpr uint32_t kQueryCount = 4; + constexpr uint64_t kZero = 0; + + wgpu::QuerySet querySet = CreateQuerySetForTimestamp(kQueryCount); + wgpu::Buffer destination = CreateResolveBuffer(kQueryCount * sizeof(uint64_t)); + // Set sentinel values to check the queries are resolved correctly if the queries are + // written sparsely + std::vector sentinelValues{0, kSentinelValue, 0, kSentinelValue}; + queue.WriteBuffer(destination, 0, sentinelValues.data(), kQueryCount * sizeof(uint64_t)); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.WriteTimestamp(querySet, 0); + encoder.WriteTimestamp(querySet, 2); + encoder.ResolveQuerySet(querySet, 0, kQueryCount, destination, 0); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + EXPECT_BUFFER(destination, 0, sizeof(uint64_t), new TimestampExpectation); + // The query with no value written should be resolved to 0. + EXPECT_BUFFER_U64_RANGE_EQ(&kZero, destination, sizeof(uint64_t), 1); + EXPECT_BUFFER(destination, 2 * sizeof(uint64_t), sizeof(uint64_t), new TimestampExpectation); + // The query with no value written should be resolved to 0. + EXPECT_BUFFER_U64_RANGE_EQ(&kZero, destination, 3 * sizeof(uint64_t), 1); +} + +// Test resolving timestamp query to 0 if all queries are not written +TEST_P(TimestampQueryTests, ResolveWithoutWritten) { + // TODO(hao.x.li@intel.com): Fix queries reset and sparsely resolving on Vulkan backend, + // otherwise its validation layer reports unissued queries resolving error + DAWN_SKIP_TEST_IF(IsVulkan() && IsBackendValidationEnabled()); + + constexpr uint32_t kQueryCount = 2; + + wgpu::QuerySet querySet = CreateQuerySetForTimestamp(kQueryCount); + wgpu::Buffer destination = CreateResolveBuffer(kQueryCount * sizeof(uint64_t)); + // Set sentinel values to check 0 is correctly written if resolving query set with no + // query is written + std::vector sentinelValues(kQueryCount, kSentinelValue); + queue.WriteBuffer(destination, 0, sentinelValues.data(), kQueryCount * sizeof(uint64_t)); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.ResolveQuerySet(querySet, 0, kQueryCount, destination, 0); + wgpu::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + std::vector expectedZeros(kQueryCount); + EXPECT_BUFFER_U64_RANGE_EQ(expectedZeros.data(), destination, 0, kQueryCount); +} + // Test resolving timestamp query to one slot in the buffer TEST_P(TimestampQueryTests, ResolveToBufferWithOffset) { // TODO(hao.x.li@intel.com): Fail to resolve query to buffer with offset on Windows Vulkan and