Skip setting Vulkan object labels
Setting Vulkan object labels has been identified as a hotspot for
graphite. Change the Vulkan backend to skip setting the labels if the
UseUserDefinedLabelsInBackend toggle isn't enabled. This wasn't done
previously since VVL error messages are mapped back to a specific device
using labels. Now if VVL error fires with UseUserDefinedLabelsInBackend
disabled it will cause a crash.
Bug: 394635413
Change-Id: I2734cc813c6e7c09f570e553db9412eee51e2579
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/224774
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Kyle Charbonneau <kylechar@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index 6d54f00..732d107 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -222,10 +222,9 @@
{"use_user_defined_labels_in_backend",
"Enables setting labels on backend-specific APIs that label objects. The labels used will be "
"those of the corresponding frontend objects if non-empty and default labels otherwise. "
- "Defaults to false. NOTE: On Vulkan, backend labels are currently always set (with default "
- "labels if this toggle is not set). The reason is that Dawn currently uses backend "
- "object labels on Vulkan to map errors back to the device with which the backend objects "
- "included in the error are associated.",
+ "Defaults to false unless backend validation is enabled in which case it defaults to true. "
+ "NOTE: On Vulkan, backend labels are required to map errors back to the device with which "
+ "the backend objects included in the error are associated.",
"https://crbug.com/dawn/840", ToggleStage::Device}},
{Toggle::UsePlaceholderFragmentInVertexOnlyPipeline,
{"use_placeholder_fragment_in_vertex_only_pipeline",
diff --git a/src/dawn/native/vulkan/BackendVk.cpp b/src/dawn/native/vulkan/BackendVk.cpp
index 1f9ed18..fb0046f 100644
--- a/src/dawn/native/vulkan/BackendVk.cpp
+++ b/src/dawn/native/vulkan/BackendVk.cpp
@@ -299,7 +299,7 @@
VKAPI_ATTR VkBool32 VKAPI_CALL
OnDebugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity,
- VkDebugUtilsMessageTypeFlagsEXT /* messageTypes */,
+ VkDebugUtilsMessageTypeFlagsEXT messageTypes,
const VkDebugUtilsMessengerCallbackDataEXT* pCallbackData,
void* pUserData) {
if (!ShouldReportDebugMessage(pCallbackData->pMessageIdName, pCallbackData->pMessage)) {
@@ -331,10 +331,12 @@
}
}
- // We get to this line if no device was associated with the message. Crash so that the failure
- // is loud and makes tests fail in Debug.
+ // We get to this line if no device was associated with the message. If the message is a backend
+ // validation error then crash as there should have been a debug label on the object. The
+ // driver can also produce errors even with backend validation disabled so those errors are
+ // just logged.
LogCallbackData(LogSeverity::Error, pCallbackData);
- DAWN_ASSERT(false);
+ DAWN_CHECK(!(messageTypes & VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT));
return VK_FALSE;
}
diff --git a/src/dawn/native/vulkan/DeviceVk.cpp b/src/dawn/native/vulkan/DeviceVk.cpp
index e072ab6..ce36937 100644
--- a/src/dawn/native/vulkan/DeviceVk.cpp
+++ b/src/dawn/native/vulkan/DeviceVk.cpp
@@ -95,6 +95,15 @@
mDebugPrefix(GetNextDeviceDebugPrefix()) {}
MaybeError Device::Initialize(const UnpackedPtr<DeviceDescriptor>& descriptor) {
+ if (GetInstance()->IsBackendValidationEnabled() &&
+ !IsToggleEnabled(Toggle::UseUserDefinedLabelsInBackend)) {
+ // NOTE: If Vulkan backend validation is enabled then these labels must be set to associate
+ // validation errors with a specific device. Backend validation errors will cause a crash
+ // if labels are not set.
+ EmitLog(WGPULoggingType_Warning,
+ "Backend object labels are required to map Vulkan backend errors to a device.");
+ }
+
// Copy the adapter's device info to the device so that we can change the "knobs"
mDeviceInfo = ToBackend(GetPhysicalDevice())->GetDeviceInfo();
diff --git a/src/dawn/native/vulkan/UtilsVulkan.cpp b/src/dawn/native/vulkan/UtilsVulkan.cpp
index 4634aa5..690a704 100644
--- a/src/dawn/native/vulkan/UtilsVulkan.cpp
+++ b/src/dawn/native/vulkan/UtilsVulkan.cpp
@@ -239,7 +239,11 @@
VkObjectType objectType,
uint64_t objectHandle,
const char* prefix,
- std::string label) {
+ std::string_view label) {
+ if (!device->IsToggleEnabled(Toggle::UseUserDefinedLabelsInBackend)) {
+ return;
+ }
+
if (!objectHandle || !device->GetVkDevice()) {
return;
}
@@ -255,15 +259,10 @@
// Prefix with the device's message ID so that if this label appears in a validation
// message it can be parsed out and the message can be associated with the right device.
objectNameStream << device->GetDebugPrefix() << kDeviceDebugSeparator << prefix;
-
- // NOTE: Whereas other platforms set backend labels *only* if the
- // `UseUserDefinedLabelsInBackend` toggle is enabled, on Vulkan these
- // labels must always be set as they currently provide the only way to
- // map Vulkan errors that include backend objects back to the device
- // with which the backend objects are associated.
- if (!label.empty() && device->IsToggleEnabled(Toggle::UseUserDefinedLabelsInBackend)) {
+ if (!label.empty()) {
objectNameStream << "_" << label;
}
+
std::string objectName = objectNameStream.str();
objectNameInfo.pObjectName = objectName.c_str();
device->fn.SetDebugUtilsObjectNameEXT(device->GetVkDevice(), &objectNameInfo);
diff --git a/src/dawn/native/vulkan/UtilsVulkan.h b/src/dawn/native/vulkan/UtilsVulkan.h
index 31bfac2..8d48319 100644
--- a/src/dawn/native/vulkan/UtilsVulkan.h
+++ b/src/dawn/native/vulkan/UtilsVulkan.h
@@ -29,6 +29,7 @@
#define SRC_DAWN_NATIVE_VULKAN_UTILSVULKAN_H_
#include <string>
+#include <string_view>
#include <vector>
#include "dawn/common/StackAllocated.h"
@@ -135,7 +136,7 @@
VkObjectType objectType,
uint64_t objectHandle,
const char* prefix,
- std::string label);
+ std::string_view label);
// The majority of Vulkan handles are "non-dispatchable". Dawn wraps these by overriding
// VK_DEFINE_NON_DISPATCHABLE_HANDLE to add some capabilities like making null comparisons
@@ -145,7 +146,7 @@
void SetDebugName(Device* device,
detail::VkHandle<Tag, HandleType> objectHandle,
const char* prefix,
- std::string label = "") {
+ std::string_view label = {}) {
uint64_t handle;
if constexpr (std::is_same_v<HandleType, uint64_t>) {
handle = objectHandle.GetHandle();
@@ -163,7 +164,7 @@
VkObjectType objectType,
HandleType objectHandle,
const char* prefix,
- std::string label = "") {
+ std::string_view label = {}) {
SetDebugNameInternal(device, objectType, reinterpret_cast<uint64_t>(objectHandle), prefix,
label);
}
