-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider properly reference counting and managing loader objects #1784
Comments
My two cents: we should should reconsider using the loader handler wrappers altogether. They are massive PITA. Sometime ago I even added a task for myself to look into better approaches, but haven't had the chance to work on that yet. But I did think on it, and came to the conclusion that we should just require adapters to have a pointer to the DDI in the first 8 bytes of any handle (except for native ones). Which is I believe what OpenCL does. |
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. A lot of changes were also made to fix use-after-free issues, specifically: * The `release` functions now no longer use the handle after freeing it. * `urDeviceTest` no longer frees devices that it dosen't own. * Some tests for reference counting now explicitly retain an extra copy before releasing them. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. A lot of changes were also made to fix use-after-free issues, specifically: * The `release` functions now no longer use the handle after freeing it. * `urDeviceTest` no longer frees devices that it dosen't own. * Some tests for reference counting now explicitly retain an extra copy before releasing them. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
Previously the factories used by ur_ldrddi (used when there are multiple backends) would add newly created objects to a map, but never release them. This patch adds reference counting semantics to the allocation, retention and release methods. A lot of changes were also made to fix use-after-free issues, specifically: * The `release` functions now no longer use the handle after freeing it. * `urDeviceTest` no longer frees devices that it dosen't own. * Some tests for reference counting now explicitly retain an extra copy before releasing them. No tests were added; this should be covered by tests for urThingRetain. Closes: oneapi-src#1784 .
This PR adding [release] tags to entry points that decrement handle reference counts revived a seemingly long dead mechanism in loader code: generating calls to the object factory's release method, which removes dead handles from the loader/platform handle map.
We inherited this mechanism from level zero, which doesn't have
Retain
entry points or any other way to increment a handle's reference count. As a result there isn't a reference counting mechanism in the loader to make sure we're only erasing handles from the map when they're actually invalid, the first call toRelease
a given handle would always delete it from the map if we allowed this code to generate as it was originally designed.Introducing a reference count to the loader objects does mean a slight additional overhead, and until now we haven't noticed the absence of this mechanism which only really serves to keep the size of the factory object maps down. This could also have weird consequences for objects created from native handles, it's a change we'll need to carefully consider and design properly if the benefits seem worth it.
For now I've disabled generating these release calls in #1720, effectively making it a NFC
The text was updated successfully, but these errors were encountered: