oneapi-src / unified-runtime

https://oneapi-src.github.io/unified-runtime/
Other
39 stars 117 forks source link

Consider properly reference counting and managing loader objects #1784

Open aarongreig opened 5 months ago

aarongreig commented 5 months ago

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 to Release 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

pbalcer commented 5 months ago

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. Yes, it would make it a tiny bit harder to do inheritance (objects with vptr's would need wrappers), but that's a small price to pay for eliminating the back-and-forth translation of handles.