Vulkan-loader dlopen large overhead about vulkan-loader HOT 5 CLOSED

When the loader was initially designed, the preference indicated by Khronos members was that these calls be stateless in the loader, thus we open and then close things for each call. Part of that was to reduce / eliminate the need for global data (not part of an instance because no instance available).
Considering the cost, it may make sense to cache this info. Maybe that could be enabled by an environment variable or something to limit the impact to those that specifically ask for it.

from vulkan-loader.

This issue has come up in the past. There are reason's for the inefficiencies but I don't think those are compelling enough to avoid improving this. As @courtney-g said, the biggest reason for the behavior is that Vulkan is designed to avoid storing global state. But even as the loader is implemented now there actually is a small amount of global state, but it's basically just mutexes.

I actually started writing up a patch to address this a while back. After seeing this issue, I rebased it onto the latest from master and pushed it up to this repo as the lenny_preload_icds branch here. That change loads the ICDs when the library is loaded to provide a list. Note that the change is not complete. It will preload the ICDs but not actually use the preloaded list. But I was curious if this looks like it would address your concerns. It absolutely will not change the worst case behavior, which is when the ICDs are constantly changing. But that's not a normal case. This would probably improve the average case considerably.

Also, keep in mind that this patch is far from complete. In order to actually make it work, we'll need to change all places where the loader gets ICDs to search that cached list first. But it shouldn't be that tough to do.

from vulkan-loader.

@courtney-g, thanks for the explanation. The amount of overhead the loader introduces by reloading the libs is not small though, so hopefully a solution that improves it for everyone (rather than opt-in) would be adopted.

@lenny-lunarg, that looks like a change in the right track, which should address the ICD part of the problem. Although I would close all the files (except the ICD that is used) right after vkCreateInstance(). There is little point in keeping the other ICDs in memory during the execution of the application (A niche case could be an application opening multiple ICDs simultaneously which would remain inefficient as is).

I don't know if "constantly changing ICDs" is even a problem. As long as you don't close the handle to the libs, they are alive and you can read them as much as you want. I mean, you never need to check if the ICDs have changed.

About the layer part of the problem: why are the layers being loaded once in vkCreateInstance() and again in vkCreateDevice()? If anything, the handle to them can be cached in the instance and the unused ones closed on device creation.

from vulkan-loader.

I would close all the files (except the ICD that is used) right after vkCreateInstance()

You can't do that because ICD selection doesn't happen until device creation. And even then, an application could create a different VkDevice from the same VkInstance on another ICD. An instance must support all physical devices on the system, which means it needs all of the ICDs. You could still release them and then open them again as needed later, but that causes the same problem we want to solve here.

I don't know if "constantly changing ICDs" is even a problem.

I meant "constantly changing" as in ICDs getting added and removed. You don't have to worry about the already loaded ICDs changing, but you could have a different number of them at different points in time. This is mostly a factor because of external GPUs. If you don't reload the ICDs at instance creation time it would be impossible to load a new driver without re-loading the loader.

why are the layers being loaded once in vkCreateInstance() and again in vkCreateDevice()?

I'll have to take a look at that. I agree that layers should only be loaded in vkCreateInstance. If they're being loaded in vkCreateDevice my guess is that would be some older logic from when device layers were separate from instance layers and it's just code that didn't get removed properly. The queries for device layers still exist (vkEnumerateDeviceLayerProperties and a couple of fields in VkDeviceCreateInfo), but those aren't used anymore.

I'll have a better chance to look into this issue more after the weekend.

from vulkan-loader.

Closing the issue as the the merged PR alleviates the overhead of LoadLibrary/dlopen by calling loader_scan_icd() once ahead of time to keep subsequent calls to LoadLibrary/dlopen of the same icds cheap, as most OS's will have multiple library opens only load the data the first time.
With the deprecation and replacement of standard_validation with khronos_validation, the layer loading cost is much less but if it is still a major issue, a subsequent issue should be made.

from vulkan-loader.