icd: Remove needless environment override in instance extension enume… - #50
Merged
Merged
Conversation
…ration The instance extensions are enumerated and cached up-front at ICD load time, therefore environment override is not necessary in this call. Furthermore, as it turns out, the loader does not take a lock inside vkEnumerateInstanceExtensionProperties, therefore overriding the environment variables here could cause unexpected side effects in other threads calling into the Vulkan SC loader. Unfortunately, these types of issues are not fixed entirely by this change, as the environment override trick requires "collaboration" from the loader in the sense that the loader itself also mutexes the points where this happens. Fortunately, that is the case for key APIs like vkCreateInstance, vkEnumeratePhysicalDevices, and vkEnumeratePhysicalDeviceGroups, but ICD loading itself is not always mutexed by the loader at the moment, in particularly, exactly in the vkEnumerateInstanceExtensionProperties entry point.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ration
The instance extensions are enumerated and cached up-front at ICD load time, therefore environment override is not necessary in this call.
Furthermore, as it turns out, the loader does not take a lock inside vkEnumerateInstanceExtensionProperties, therefore overriding the environment variables here could cause unexpected side effects in other threads calling into the Vulkan SC loader.
Unfortunately, these types of issues are not fixed entirely by this change, as the environment override trick requires "collaboration" from the loader in the sense that the loader itself also mutexes the points where this happens. Fortunately, that is the case for key APIs like vkCreateInstance, vkEnumeratePhysicalDevices, and vkEnumeratePhysicalDeviceGroups, but ICD loading itself is not always mutexed by the loader at the moment, in particularly, exactly in the vkEnumerateInstanceExtensionProperties entry point.