From 32cfb94797159d8f69ca3dda7224dfec3bd1de8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 15 Nov 2020 20:03:26 +0100 Subject: [PATCH] Vk: enumerate only what's strictly necessary in pickDevice(). This avoids allocating a potentially large array in case just the first device is needed. Originally I did this in a hope to avoid a stall + CPU power management issues due to some bad shit in the AMD driver, but it seems that enumerating even just one device still makes it stall. Sigh. --- src/Magnum/Vk/DeviceProperties.cpp | 53 +++++++++++++++++++++--------- src/Magnum/Vk/DeviceProperties.h | 20 +++++++---- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/Magnum/Vk/DeviceProperties.cpp b/src/Magnum/Vk/DeviceProperties.cpp index 46b95f277..40833869c 100644 --- a/src/Magnum/Vk/DeviceProperties.cpp +++ b/src/Magnum/Vk/DeviceProperties.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -315,23 +316,40 @@ Containers::Optional DeviceProperties::tryPickMemory(const MemoryFl return tryPickMemory(requiredFlags, {}, memories); } -Containers::Array enumerateDevices(Instance& instance) { - /* Retrieve total device count */ - UnsignedInt count; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(instance->EnumeratePhysicalDevices(instance, &count, nullptr)); +/* Can't be inside an anonymous namespace as it's friended to DeviceProperties */ +namespace Implementation { +UnsignedInt enumerateDevicesInto(Instance& instance, Containers::ArrayView out) { /* Allocate memory for the output, fetch the handles into it */ - Containers::Array out{Containers::NoInit, count}; - Containers::ArrayView handles{reinterpret_cast(out.data()), count}; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(instance->EnumeratePhysicalDevices(instance, &count, handles.data())); + Containers::ArrayView handles{reinterpret_cast(out.data()), out.size()}; + UnsignedInt count = out.size(); + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(instance->EnumeratePhysicalDevices(instance, &count, handles.data())); - /* Expect the device count didn't change between calls */ - CORRADE_INTERNAL_ASSERT(count == out.size()); + /* Expect the final count isn't larger than the output array */ + CORRADE_INTERNAL_ASSERT(count <= out.size()); /* Construct actual DeviceProperties instances from these, go backwards so we don't overwrite the not-yet-processed handles */ for(std::size_t i = count; i != 0; --i) new(out.data() + i - 1) DeviceProperties{instance, handles[i - 1]}; + /* Construct the remaining entries so the array destructor doesn't crash */ + for(std::size_t i = count; i != out.size(); ++i) + new(out.data() + i) DeviceProperties{NoCreate}; + + return count; +} + +} + +Containers::Array enumerateDevices(Instance& instance) { + /* Retrieve total device count */ + UnsignedInt count; + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(instance->EnumeratePhysicalDevices(instance, &count, nullptr)); + + /* Fetch device handles, expect the device count didn't change between + calls */ + Containers::Array out{Containers::NoInit, count}; + CORRADE_INTERNAL_ASSERT_OUTPUT(Implementation::enumerateDevicesInto(instance, out) == out.size()); return out; } @@ -340,27 +358,32 @@ Containers::Optional tryPickDevice(Instance& instance) { Utility::Arguments args = Implementation::arguments(); args.parse(instance.state().argc, instance.state().argv); - Containers::Array devices = enumerateDevices(instance); - /* Pick the first by default */ if(args.value("device").empty()) { - if(devices.empty()) { + Containers::Array1 devices{Containers::NoInit}; + if(!Implementation::enumerateDevicesInto(instance, devices)) { Error{} << "Vk::tryPickDevice(): no Vulkan devices found"; return {}; } + return std::move(devices.front()); } /* Pick by ID */ if(args.value("device")[0] >= '0' && args.value("device")[0] <= '9') { - UnsignedInt id = args.value("device"); - if(id >= devices.size()) { - Error{} << "Vk::tryPickDevice(): index" << id << "out of bounds for" << devices.size() << "Vulkan devices"; + const UnsignedInt id = args.value("device"); + Containers::Array devices{Containers::NoInit, id + 1}; + const UnsignedInt count = Implementation::enumerateDevicesInto(instance, devices); + if(id >= count) { + Error{} << "Vk::tryPickDevice(): index" << id << "out of bounds for" << count << "Vulkan devices"; return {}; } + return std::move(devices[id]); } + Containers::Array devices = enumerateDevices(instance); + /* Pick by type */ DeviceType type; if(args.value("device") == "integrated") diff --git a/src/Magnum/Vk/DeviceProperties.h b/src/Magnum/Vk/DeviceProperties.h index ebd5f4165..e934f6bef 100644 --- a/src/Magnum/Vk/DeviceProperties.h +++ b/src/Magnum/Vk/DeviceProperties.h @@ -42,7 +42,10 @@ namespace Magnum { namespace Vk { -namespace Implementation { struct InstanceState; } +namespace Implementation { + struct InstanceState; + UnsignedInt enumerateDevicesInto(Instance& instance, Containers::ArrayView out); +} /** @brief Physical device type @@ -604,7 +607,7 @@ class MAGNUM_VK_EXPORT DeviceProperties { /* The DAMN THING lists this among friends, which is AN IMPLEMENTATION DETAIL */ friend DeviceCreateInfo; - friend MAGNUM_VK_EXPORT Containers::Array enumerateDevices(Instance&); + friend UnsignedInt Implementation::enumerateDevicesInto(Instance&, Containers::ArrayView); #endif explicit DeviceProperties(Instance& instance, VkPhysicalDevice handle); @@ -644,10 +647,15 @@ MAGNUM_VK_EXPORT Containers::Array enumerateDevices(Instance& @brief Pick a physical device @m_since_latest -Calls @ref enumerateDevices() and selects a device based on preferences -specified through the `--magnum-device` @ref Vk-Instance-command-line "command-line option". -If a device is not found, exits. See @ref tryPickDevice() for an alternative -that doesn't exit on failure. See @ref Device for general usage information. +Selects a device based on preferences specified through the `--magnum-device` +@ref Vk-Instance-command-line "command-line option". If a device is not found, +exits. See @ref tryPickDevice() for an alternative that doesn't exit on +failure. See @ref Device for general usage information. + +If `--magnum-device` is not specified or `--magnum-device` specifies a device +index, this function enumerates just the first N devices to satisfy the +request. Otherwise calls @ref enumerateDevices() and picks the first device +matching the criteria in `--magnum-device`. */ MAGNUM_VK_EXPORT DeviceProperties pickDevice(Instance& instance);