From 7abcadfce6e0ecb2158806e7dd47aad50126ee6d Mon Sep 17 00:00:00 2001 From: Li Hao Date: Wed, 23 Feb 2022 17:50:35 +0000 Subject: [PATCH] Add toggle to disable the tick->ns conversion of timestamp query Although the error rate of timestamp tick->ns conversion is very small (3e-5), some profiling scenarios, such as CPU/GPU timestamp calibration on Windows, require absolutely accurate timestamps. Add new toggle to resolve timestamps to ticks for those cases where zero error is required. Add an end2end test for GPU timestamp calibration on D3D12 backend. Disable timestamp period calculation on Device when the DisableTimestampQueryConversion is enabled. Bug: dawn:1305 Change-Id: I31ee6b4c1686d5dd2ac29ccb0bd398e650481c26 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/81023 Reviewed-by: Shrek Shao Reviewed-by: Austin Eng Commit-Queue: Austin Eng --- src/dawn/native/CommandEncoder.cpp | 3 +- src/dawn/native/Toggles.cpp | 4 + src/dawn/native/Toggles.h | 1 + src/dawn/native/d3d12/DeviceD3D12.cpp | 3 +- src/dawn/native/metal/DeviceMTL.mm | 3 +- src/dawn/tests/BUILD.gn | 1 + .../D3D12GPUTimestampCalibrationTests.cpp | 118 ++++++++++++++++++ .../white_box/QueryInternalShaderTests.cpp | 9 +- 8 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 src/dawn/tests/white_box/D3D12GPUTimestampCalibrationTests.cpp diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 823e04729d..18b5c63c99 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp @@ -1140,7 +1140,8 @@ namespace dawn::native { cmd->destinationOffset = destinationOffset; // Encode internal compute pipeline for timestamp query - if (querySet->GetQueryType() == wgpu::QueryType::Timestamp) { + if (querySet->GetQueryType() == wgpu::QueryType::Timestamp && + !GetDevice()->IsToggleEnabled(Toggle::DisableTimestampQueryConversion)) { DAWN_TRY(EncodeTimestampsToNanosecondsConversion( this, querySet, firstQuery, queryCount, destination, destinationOffset)); } diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index e65bdf17b0..b9d8387a0a 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -237,6 +237,10 @@ namespace dawn::native { "command queue, and the information includes system time, CPU timestamp, GPU " "timestamp, and their frequency.", "https://crbug.com/dawn/1264"}}, + {Toggle::DisableTimestampQueryConversion, + {"disable_timestamp_query_conversion", + "Resolve timestamp queries into ticks instead of nanoseconds.", + "https://crbug.com/dawn/1305"}}, // Dummy comment to separate the }} so it is clearer what to copy-paste to add a toggle. }}; diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 4a4574dd43..519560bce5 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -62,6 +62,7 @@ namespace dawn::native { UseDummyFragmentInVertexOnlyPipeline, FxcOptimizations, RecordDetailedTimingInTraceEvents, + DisableTimestampQueryConversion, EnumCount, InvalidEnum = EnumCount, diff --git a/src/dawn/native/d3d12/DeviceD3D12.cpp b/src/dawn/native/d3d12/DeviceD3D12.cpp index 918c7c0845..415b486534 100644 --- a/src/dawn/native/d3d12/DeviceD3D12.cpp +++ b/src/dawn/native/d3d12/DeviceD3D12.cpp @@ -78,7 +78,8 @@ namespace dawn::native::d3d12 { CheckHRESULT(mD3d12Device->CreateCommandQueue(&queueDesc, IID_PPV_ARGS(&mCommandQueue)), "D3D12 create command queue")); - if (IsFeatureEnabled(Feature::TimestampQuery)) { + if (IsFeatureEnabled(Feature::TimestampQuery) && + !IsToggleEnabled(Toggle::DisableTimestampQueryConversion)) { // Get GPU timestamp counter frequency (in ticks/second). This fails if the specified // command queue doesn't support timestamps. D3D12_COMMAND_LIST_TYPE_DIRECT queues // always support timestamps except where there are bugs in Windows container and vGPU diff --git a/src/dawn/native/metal/DeviceMTL.mm b/src/dawn/native/metal/DeviceMTL.mm index 55158ea4e7..e654c8be59 100644 --- a/src/dawn/native/metal/DeviceMTL.mm +++ b/src/dawn/native/metal/DeviceMTL.mm @@ -134,7 +134,8 @@ namespace dawn::native::metal { DAWN_TRY(mCommandContext.PrepareNextCommandBuffer(*mCommandQueue)); - if (IsFeatureEnabled(Feature::TimestampQuery)) { + if (IsFeatureEnabled(Feature::TimestampQuery) && + !IsToggleEnabled(Toggle::DisableTimestampQueryConversion)) { // Make a best guess of timestamp period based on device vendor info, and converge it to // an accurate value by the following calculations. mTimestampPeriod = gpu_info::IsIntel(GetAdapter()->GetVendorId()) ? 83.333f : 1.0f; diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index 4f7a559b16..cad12e2f94 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn @@ -533,6 +533,7 @@ source_set("white_box_tests_sources") { if (dawn_enable_d3d12) { sources += [ "white_box/D3D12DescriptorHeapTests.cpp", + "white_box/D3D12GPUTimestampCalibrationTests.cpp", "white_box/D3D12ResidencyTests.cpp", "white_box/D3D12ResourceHeapTests.cpp", ] diff --git a/src/dawn/tests/white_box/D3D12GPUTimestampCalibrationTests.cpp b/src/dawn/tests/white_box/D3D12GPUTimestampCalibrationTests.cpp new file mode 100644 index 0000000000..bff420fb95 --- /dev/null +++ b/src/dawn/tests/white_box/D3D12GPUTimestampCalibrationTests.cpp @@ -0,0 +1,118 @@ +// Copyright 2022 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn/tests/DawnTest.h" + +#include "dawn/native/Buffer.h" +#include "dawn/native/CommandEncoder.h" +#include "dawn/native/d3d12/DeviceD3D12.h" +#include "dawn/utils/WGPUHelpers.h" + +namespace { + class ExpectBetweenTimestamps : public detail::Expectation { + public: + ~ExpectBetweenTimestamps() override = default; + + ExpectBetweenTimestamps(uint64_t value0, uint64_t value1) { + mValue0 = value0; + mValue1 = value1; + } + + // Expect the actual results are between mValue0 and mValue1. + testing::AssertionResult Check(const void* data, size_t size) override { + const uint64_t* actual = static_cast(data); + for (size_t i = 0; i < size / sizeof(uint64_t); ++i) { + if (actual[i] < mValue0 || actual[i] > mValue1) { + return testing::AssertionFailure() + << "Expected data[" << i << "] to be between " << mValue0 << " and " + << mValue1 << ", actual " << actual[i] << std::endl; + } + } + + return testing::AssertionSuccess(); + } + + private: + uint64_t mValue0; + uint64_t mValue1; + }; + +} // anonymous namespace + +using namespace dawn::native::d3d12; + +class D3D12GPUTimestampCalibrationTests : public DawnTest { + protected: + void SetUp() override { + DawnTest::SetUp(); + + DAWN_TEST_UNSUPPORTED_IF(UsesWire()); + // Requires that timestamp query feature is enabled and timestamp query conversion is + // disabled. + DAWN_TEST_UNSUPPORTED_IF(!SupportsFeatures({wgpu::FeatureName::TimestampQuery}) || + !HasToggleEnabled("disable_timestamp_query_conversion")); + } + + std::vector GetRequiredFeatures() override { + std::vector requiredFeatures = {}; + if (SupportsFeatures({wgpu::FeatureName::TimestampQuery})) { + requiredFeatures.push_back(wgpu::FeatureName::TimestampQuery); + } + return requiredFeatures; + } +}; + +// Check that the timestamps got by timestamp query are between the two timestamps from +// GetClockCalibration() after the timestamp conversion is disabled. +TEST_P(D3D12GPUTimestampCalibrationTests, TimestampsInOrder) { + constexpr uint32_t kQueryCount = 2; + + wgpu::QuerySetDescriptor querySetDescriptor; + querySetDescriptor.count = kQueryCount; + querySetDescriptor.type = wgpu::QueryType::Timestamp; + wgpu::QuerySet querySet = device.CreateQuerySet(&querySetDescriptor); + + wgpu::BufferDescriptor bufferDescriptor; + bufferDescriptor.size = kQueryCount * sizeof(uint64_t); + bufferDescriptor.usage = + wgpu::BufferUsage::QueryResolve | wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst; + wgpu::Buffer destination = device.CreateBuffer(&bufferDescriptor); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.WriteTimestamp(querySet, 0); + encoder.WriteTimestamp(querySet, 1); + wgpu::CommandBuffer commands = encoder.Finish(); + + Device* d3DDevice = reinterpret_cast(device.Get()); + uint64_t gpuTimestamp0, gpuTimestamp1; + uint64_t cpuTimestamp0, cpuTimestamp1; + d3DDevice->GetCommandQueue()->GetClockCalibration(&gpuTimestamp0, &cpuTimestamp0); + queue.Submit(1, &commands); + WaitForAllOperations(); + d3DDevice->GetCommandQueue()->GetClockCalibration(&gpuTimestamp1, &cpuTimestamp1); + + // Separate resolve queryset to reduce the execution time of the queue with WriteTimestamp, + // so that the timestamp in the querySet will be closer to both gpuTimestamps from + // GetClockCalibration. + 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 ExpectBetweenTimestamps(gpuTimestamp0, gpuTimestamp1)); +} + +DAWN_INSTANTIATE_TEST(D3D12GPUTimestampCalibrationTests, + D3D12Backend({"disable_timestamp_query_conversion"})); \ No newline at end of file diff --git a/src/dawn/tests/white_box/QueryInternalShaderTests.cpp b/src/dawn/tests/white_box/QueryInternalShaderTests.cpp index 66dac3d32a..96d3b2fe14 100644 --- a/src/dawn/tests/white_box/QueryInternalShaderTests.cpp +++ b/src/dawn/tests/white_box/QueryInternalShaderTests.cpp @@ -84,6 +84,13 @@ constexpr static uint64_t kSentinelValue = ~uint64_t(0u); class QueryInternalShaderTests : public DawnTest { protected: + void SetUp() override { + DawnTest::SetUp(); + + DAWN_TEST_UNSUPPORTED_IF(UsesWire()); + DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("disable_timestamp_query_conversion")); + } + // Original timestamp values in query set for testing const std::vector querySetValues = { kSentinelValue, // garbage data which is not written at beginning @@ -190,8 +197,6 @@ TEST_P(QueryInternalShaderTests, TimestampComputeShader) { // TODO(crbug.com/dawn/741): Test output is wrong with D3D12 + WARP. DAWN_SUPPRESS_TEST_IF(IsD3D12() && IsWARP()); - DAWN_TEST_UNSUPPORTED_IF(UsesWire()); - constexpr std::array kPeriodsToTest = { 1, 7,