Revert "d3d: Change D3D references to PhysicalDevice to WeakRef instead of Ref"
This reverts commit 3c05043f3c070f78b33b3f51124b661f97ec6512.
Reason for revert: slows dawn_end2end_tests by 1.4
https://ci.chromium.org/ui/p/chromium/builders/try/win-dawn-rel/34838/overview
dawn_end2end_tests on NVIDIA GPU on Windows (with patch) on Windows-10-18363
Shard runtime (19m 24s) + overhead (10s): 19m 34s
vs.
https://ci.chromium.org/ui/p/chromium/builders/ci/Dawn%20Win10%20x64%20Release%20(NVIDIA)/83040/overview
Shard runtime (13m 58s) + overhead (9s): 14m 7s
Original change's description:
> d3d: Change D3D references to PhysicalDevice to WeakRef instead of Ref
>
> BackendD3D holds strong references to PhysicalDeviceD3D11/12, which
> are created when enumerating adapters. Once adapters are enumerated,
> these are kept alive indefinitely. Upon creation, PhysicalDeviceD3D
> has a reference to the D3D11/12Device, which means that the
> corresponding physical adapter is kept powered on indefinitely.
>
> This CL changes the strong Ref to a WeakRef and relies on the caller
> to keep a strong reference to the PhysicalDevice to keep it alive.
> All other unused devices are cleaned up, which releases the unused
> D3DDevices.
>
> Bug: 342299153
> Change-Id: I4ff6979abb175f9b737fb3ede4b26334d858d6a4
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/189581
> Reviewed-by: Austin Eng <enga@chromium.org>
> Commit-Queue: Patrick To <patrto@microsoft.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 342299153, 346717086
Change-Id: I1c94259031ef2ce834a855ebe0ee961fdf4a406f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/193480
Commit-Queue: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Yuly Novikov <ynovikov@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/PhysicalDevice.h b/src/dawn/native/PhysicalDevice.h
index 301b7d3..dfc5bd4 100644
--- a/src/dawn/native/PhysicalDevice.h
+++ b/src/dawn/native/PhysicalDevice.h
@@ -36,7 +36,6 @@
#include "dawn/common/GPUInfo.h"
#include "dawn/common/Ref.h"
#include "dawn/common/RefCounted.h"
-#include "dawn/common/WeakRefSupport.h"
#include "dawn/common/ityp_span.h"
#include "dawn/native/Device.h"
#include "dawn/native/Error.h"
@@ -68,7 +67,7 @@
std::string errorMessage;
};
-class PhysicalDeviceBase : public RefCounted, public WeakRefSupport<PhysicalDeviceBase> {
+class PhysicalDeviceBase : public RefCounted {
public:
explicit PhysicalDeviceBase(wgpu::BackendType backend);
~PhysicalDeviceBase() override;
diff --git a/src/dawn/native/d3d/BackendD3D.cpp b/src/dawn/native/d3d/BackendD3D.cpp
index 29291d5..478d60f 100644
--- a/src/dawn/native/d3d/BackendD3D.cpp
+++ b/src/dawn/native/d3d/BackendD3D.cpp
@@ -255,17 +255,19 @@
}
ResultOrError<Ref<PhysicalDeviceBase>> Backend::GetOrCreatePhysicalDeviceFromLUID(LUID luid) {
- Ref<PhysicalDeviceBase> physicalDevice = FindPhysicalDevice(luid);
- if (physicalDevice == nullptr) {
- ComPtr<IDXGIAdapter1> dxgiAdapter = nullptr;
- DAWN_TRY(CheckHRESULT(GetFactory()->EnumAdapterByLuid(luid, IID_PPV_ARGS(&dxgiAdapter)),
- "EnumAdapterByLuid"));
-
- DAWN_TRY_ASSIGN(physicalDevice, CreatePhysicalDeviceFromIDXGIAdapter(dxgiAdapter));
- // The LUID may already exist in the map if the previous WeakRef has been invalidated. In
- // that case, `insert_or_assign` will replace the old device with the new one.
- mPhysicalDevices.insert_or_assign(luid, GetWeakRef(physicalDevice));
+ auto it = mPhysicalDevices.find(luid);
+ if (it != mPhysicalDevices.end()) {
+ // If we've already discovered this physical device, return it.
+ return it->second;
}
+
+ ComPtr<IDXGIAdapter1> dxgiAdapter = nullptr;
+ DAWN_TRY(CheckHRESULT(GetFactory()->EnumAdapterByLuid(luid, IID_PPV_ARGS(&dxgiAdapter)),
+ "EnumAdapterByLuid"));
+
+ Ref<PhysicalDeviceBase> physicalDevice;
+ DAWN_TRY_ASSIGN(physicalDevice, CreatePhysicalDeviceFromIDXGIAdapter(dxgiAdapter));
+ mPhysicalDevices.emplace(luid, physicalDevice);
return physicalDevice;
}
@@ -274,24 +276,16 @@
DXGI_ADAPTER_DESC desc;
DAWN_TRY(CheckHRESULT(dxgiAdapter->GetDesc(&desc), "IDXGIAdapter::GetDesc"));
- Ref<PhysicalDeviceBase> physicalDevice = FindPhysicalDevice(desc.AdapterLuid);
- if (physicalDevice == nullptr) {
- DAWN_TRY_ASSIGN(physicalDevice, CreatePhysicalDeviceFromIDXGIAdapter(dxgiAdapter));
- // The LUID may already exist in the map if the previous WeakRef has been invalidated. In
- // that case, `insert_or_assign` will replace the old device with the new one.
- mPhysicalDevices.insert_or_assign(desc.AdapterLuid, GetWeakRef(physicalDevice));
+ auto it = mPhysicalDevices.find(desc.AdapterLuid);
+ if (it != mPhysicalDevices.end()) {
+ // If we've already discovered this physical device, return it.
+ return it->second;
}
- return physicalDevice;
-}
-Ref<PhysicalDeviceBase> Backend::FindPhysicalDevice(const LUID& luid) {
- auto it = mPhysicalDevices.find(luid);
- if (it == mPhysicalDevices.end()) {
- return nullptr;
- }
- // If we've already discovered this physical device, try to Promote the WeakRef to a Ref. If the
- // WeakRef has been invalidated, nullptr is returned.
- return it->second.Promote();
+ Ref<PhysicalDeviceBase> physicalDevice;
+ DAWN_TRY_ASSIGN(physicalDevice, CreatePhysicalDeviceFromIDXGIAdapter(dxgiAdapter));
+ mPhysicalDevices.emplace(desc.AdapterLuid, physicalDevice);
+ return physicalDevice;
}
std::vector<Ref<PhysicalDeviceBase>> Backend::DiscoverPhysicalDevices(
diff --git a/src/dawn/native/d3d/BackendD3D.h b/src/dawn/native/d3d/BackendD3D.h
index 295b699..7ae6fdb 100644
--- a/src/dawn/native/d3d/BackendD3D.h
+++ b/src/dawn/native/d3d/BackendD3D.h
@@ -102,7 +102,6 @@
ResultOrError<Ref<PhysicalDeviceBase>> GetOrCreatePhysicalDeviceFromLUID(LUID luid);
ResultOrError<Ref<PhysicalDeviceBase>> GetOrCreatePhysicalDeviceFromIDXGIAdapter(
ComPtr<IDXGIAdapter> dxgiAdapter);
- Ref<PhysicalDeviceBase> FindPhysicalDevice(const LUID& luid);
// Acquiring DXC version information and store the result in mDxcVersionInfo. This function
// should be called only once, during startup in `Initialize`.
@@ -134,13 +133,9 @@
};
// Map of LUID to physical device.
- // The LUID is guaranteed to be uniquely identify an adapter on the local machine until restart.
- // A WeakRef prevents the PhysicalDeviceBase (and its D3D Device) from being kept alive if there
- // are no longer any external references. Any references to a D3D Device keeps its corresponding
- // physical adapter powered on. Since `DiscoverPhysicalDevices` may enumerate and add all
- // available adapters, we should release the ones that the caller does not take a strong
- // reference on. Otherwise, all adapters on the system will be kept powered on indefinitely.
- absl::flat_hash_map<LUID, WeakRef<PhysicalDeviceBase>, LUIDHashFunc, LUIDEqualFunc>
+ // The LUID is guaranteed to be uniquely identify an adapter on the local
+ // machine until restart.
+ absl::flat_hash_map<LUID, Ref<PhysicalDeviceBase>, LUIDHashFunc, LUIDEqualFunc>
mPhysicalDevices;
};
