Skip to content

Commit 7b8996e

Browse files
[SYCL] Keep platform_impl's device_impls alive until shutdown (#18251)
After that devices are never destroyed until the SYCL RT library shutdown. In practice, that means that before the change a simple ``` int main() { sycl::device d; } ``` went into `platform` ctor, then queried all the platform's devices to check that it has some, returned from ctor and those `sycl::device`s created on stack were already destroyed. After that, when creating user's `sycl::device d` we were re-creating device hierarchy for the platform at SYCL level again (including some calls to `urDeviceGetInfo` during `device_impl` creation). After the changes, devices created when veryfing that platform isn't empty are preserved inside the `platform_impl` object and this existing SYCL devices hierarchy is used when creating user's device object. A note on the implementation: `device_impl` has an `std::shared_ptr<platform_impl>` inside so we can't rely on automatic resource management just by the nature of `std::shared_ptr` everywhere (and we haven't changed this aspect in #18143). As such, we have to perform some explicit resource release during shutdown procedure (or in `~UrMock()` for unittests).
1 parent 78405de commit 7b8996e

File tree

6 files changed

+23
-9
lines changed

6 files changed

+23
-9
lines changed

sycl/source/detail/global_handler.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,14 @@ std::vector<std::shared_ptr<platform_impl>> &GlobalHandler::getPlatformCache() {
211211
return PlatformCache;
212212
}
213213

214+
void GlobalHandler::clearPlatforms() {
215+
if (!MPlatformCache.Inst)
216+
return;
217+
for (auto &PltSmartPtr : *MPlatformCache.Inst)
218+
PltSmartPtr->MDevices.clear();
219+
MPlatformCache.Inst->clear();
220+
}
221+
214222
std::mutex &GlobalHandler::getPlatformMapMutex() {
215223
static std::mutex &PlatformMapMutex = getOrCreate(MPlatformMapMutex);
216224
return PlatformMapMutex;
@@ -366,6 +374,7 @@ void shutdown_late() {
366374
#endif
367375

368376
// First, release resources, that may access adapters.
377+
Handler->clearPlatforms(); // includes dropping platforms' devices ownership.
369378
Handler->MPlatformCache.Inst.reset(nullptr);
370379
Handler->MScheduler.Inst.reset(nullptr);
371380
Handler->MProgramManager.Inst.reset(nullptr);

sycl/source/detail/global_handler.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ class GlobalHandler {
6161
Sync &getSync();
6262
std::vector<std::shared_ptr<platform_impl>> &getPlatformCache();
6363

64+
void clearPlatforms();
65+
6466
std::unordered_map<platform_impl *, ContextImplPtr> &
6567
getPlatformToDefaultContextCache();
6668

sycl/source/detail/platform_impl.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ platform_impl::getOrMakeDeviceImpl(ur_device_handle_t UrDevice) {
306306
// Otherwise make the impl
307307
Result = std::make_shared<device_impl>(UrDevice, *this,
308308
device_impl::private_tag{});
309-
MDeviceCache.emplace_back(Result);
309+
MDevices.emplace_back(Result);
310310

311311
return Result;
312312
}
@@ -637,11 +637,9 @@ bool platform_impl::has(aspect Aspect) const {
637637

638638
std::shared_ptr<device_impl>
639639
platform_impl::getDeviceImplHelper(ur_device_handle_t UrDevice) {
640-
for (const std::weak_ptr<device_impl> &DeviceWP : MDeviceCache) {
641-
if (std::shared_ptr<device_impl> Device = DeviceWP.lock()) {
642-
if (Device->getHandleRef() == UrDevice)
643-
return Device;
644-
}
640+
for (const std::shared_ptr<device_impl> &Device : MDevices) {
641+
if (Device->getHandleRef() == UrDevice)
642+
return Device;
645643
}
646644
return nullptr;
647645
}

sycl/source/detail/platform_impl.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
217217

218218
AdapterPtr MAdapter;
219219

220-
std::vector<std::weak_ptr<device_impl>> MDeviceCache;
220+
std::vector<std::shared_ptr<device_impl>> MDevices;
221+
friend class GlobalHandler;
221222
std::mutex MDeviceMapMutex;
222223
};
223224

sycl/unittests/context_device/DeviceRefCounter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ static ur_result_t redefinedDeviceReleaseAfter(void *) {
3131
TEST(DevRefCounter, DevRefCounter) {
3232
{
3333
sycl::unittest::UrMock<> Mock;
34-
sycl::platform Plt = sycl::platform();
3534

3635
mock::getCallbacks().set_after_callback("urDeviceGet",
3736
&redefinedDevicesGetAfter);
3837
mock::getCallbacks().set_after_callback("urDeviceRetain",
3938
&redefinedDeviceRetainAfter);
4039
mock::getCallbacks().set_after_callback("urDeviceRelease",
4140
&redefinedDeviceReleaseAfter);
41+
sycl::platform Plt = sycl::platform();
4242

4343
Plt.get_devices();
4444
}

sycl/unittests/helpers/UrMock.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,11 @@ template <sycl::backend Backend = backend::opencl> class UrMock {
612612
// clear platform cache in case subsequent tests want a different backend,
613613
// this forces platforms to be reconstructed (and thus queries about UR
614614
// backend info to be called again)
615-
detail::GlobalHandler::instance().getPlatformCache().clear();
615+
//
616+
// This also erases each platform's devices (normally done in the library
617+
// shutdown) so that platforms/devices' lifetimes could work in unittests
618+
// scenario.
619+
detail::GlobalHandler::instance().clearPlatforms();
616620
mock::getCallbacks().resetCallbacks();
617621
}
618622

0 commit comments

Comments
 (0)