[vulkan] Shaders caching #39384
Closed
[vulkan] Shaders caching #39384
Conversation
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
[ghstack-poisoned]
This was referenced Jun 2, 2020
💊 CI failures summary and remediationsAs of commit 8df4055 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 112 times. |
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
This was referenced Jul 14, 2020
Closed
Closed
Closed
AshkanAliabadi
approved these changes
Jul 23, 2020
| return constBuffer; | ||
| } | ||
|
|
||
| ComputeUnitFactory::ComputeUnitFactory(const VkDevice& device) |
Contributor
There was a problem hiding this comment.
Typically opaque handles in Vulkan like VkDevice etc. are just a typedef to an integer or a pointer so there's no need to pass them by reference.
aten/src/ATen/native/vulkan/Vulkan.h
Outdated
|
|
||
| class ComputeUnitFactory { | ||
| public: | ||
| ComputeUnitFactory(const VkDevice& device); |
Contributor
There was a problem hiding this comment.
It's generally considered good practice in C++ to mark single argument constructors as explicit to avoid implicit type conversions unless such behavior (i.e. in this case implicit conversion of VkDevice to ComputeUnitFactory) is desired.
| WorkGroupSize& workGroupSize) { | ||
| std::stringstream ss; | ||
| ss << key << ':' << workGroupSize.x << ':' << workGroupSize.y << ':' | ||
| << workGroupSize.z; |
Contributor
There was a problem hiding this comment.
This kind of looks like an expensive operation to construct a key for lookups if those lookups are frequent but on the other hand it may never end up mattering so I think it's fine to roll with it for now.
aten/src/ATen/native/vulkan/Vulkan.h
Outdated
|
|
||
| VkDevice device_; | ||
| VkPipelineCache pipelineCache_; | ||
| std::map<std::string, std::shared_ptr<ComputeUnit>> computeUnits_; |
Contributor
There was a problem hiding this comment.
Consider using a std::unordered_map (aka hash map) instead of a std::map. std::map is a poor data structure in general for performance: O(log(n)) look up and poor cache performance due to scattered memory accesses which is even more detrimental to performance when dealing with few (say hundreds) of elements like we are here. In those scenarios the best data structure is typically a vector with a good old boring linear search or for slightly bigger arrays (1000s of elements) sorted vector plus a binary search. A hash map is perfectly fine for now, but just as an FYI if you wanted to go down the route of vector you would store [uint64_t, std::shared_ptr] in the vector where uint64_t is the hash key, and you would do the lookup by using hash(string) as key. Collisions should be very rare and could be handled by storing identical keys inline and doing a linear search over them.
Chances are high this never ends up mattering in our usage here so unordered_map is perfectly fine.
Contributor
Author
There was a problem hiding this comment.
Thanks, I did not know the difference between std::map and std::unordered_map, looks like map works as HashTree (balanced binary tree) while unordered_map juts as a hash map without preserving ordering. I will change it to unordered_map.
| return *(computeUnit.get()); | ||
| } | ||
|
|
||
| #ifdef USE_VULKAN_SHADERC_RUNTIME |
Contributor
There was a problem hiding this comment.
One alternate design could have been to encapsulate the difference between source code (null-terminated const char* shader_source) and binary representation ([const uint32_t* code, const uint32_t size] pair) such that the need for multiple code paths could have been avoided. Haven't put much thought into it but I think it should be possible to cheaply abstract that away.
Introducing `ComputeUnitFactory` which is responsible for providing `ComputeUnit`s (Shaders), it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just `std::map<string, std::shared_ptr>` Macro GLSL_SPV changed to have literal name for cache key as a first argument. All constructors of ComputeUnit are changed to use `ComputeUnitFactory` Ownership model: ComputeUnitFactory also owns `vkPipelineCache` that is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html ) `VContext` (global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed before `vkDestryDevice` in `~VContext` => As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory before `vkDestroyDevice`, doing `unique_ptr<ComputeUnitFactory>.reset()` Differential Revision: [D21962430](https://our.internmc.facebook.com/intern/diff/D21962430) [ghstack-poisoned]
Contributor
|
@IvanKobzarev merged this pull request in 6367a9d. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stack from ghstack:
Introducing
ComputeUnitFactorywhich is responsible for providingComputeUnits (Shaders),it caches it, using shader name (glsl file name)+workGroupSize as a cacheKey, just
std::map<string, std::shared_ptr>Macro GLSL_SPV changed to have literal name for cache key as a first argument.
All constructors of ComputeUnit are changed to use
ComputeUnitFactoryOwnership model:
ComputeUnitFactory also owns
vkPipelineCachethat is internal vulkan cache object ( https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPipelineCache.html )VContext(global object) owns ComputeUnitFactory, that owns ComputeUnits, vkPipelineCache, for destruction of them we need valid VkDevice, so it should be destructed beforevkDestryDevicein~VContext=> As members of the class will be destructed only after destructor - forcing destruction of ComputeUnitFactory beforevkDestroyDevice, doingunique_ptr<ComputeUnitFactory>.reset()Differential Revision: D21962430