-
Notifications
You must be signed in to change notification settings - Fork 422
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
Support the VK_EXT_pipeline_creation_feedback
extension.
#1968
Support the VK_EXT_pipeline_creation_feedback
extension.
#1968
Conversation
@AntarticCoder Heads up--when you implement |
Thanks I'll keep that in mind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. LGTM. Just one quibble about some repetitive code pasting.
This provides feedback that indicates: * how long it took to compile each shader stage and the pipeline as a whole; * whether or not the pipeline or any shader stage were found in any supplied pipeline cache; and * whether or not any supplied base pipeline were used to accelerate pipeline creation. This is similar to the performance statistics that MoltenVK already collects. Since we don't use any supplied base pipeline at all, this implementation never sets `VK_PIPELINE_CREATION_FEEDBACK_BASE_PIPELINE_ACCELERATION_BIT`. However, I've identified several places where we could probably use the base pipeline to accelerate pipeline creation. One day, I should probably implement that. Likewise, because we don't yet support using `MTLBinaryArchive`s, `VK_PIPELINE_CREATION_FEEDBACK_APPLICATION_PIPELINE_CACHE_HIT_BIT` is never set on the whole pipeline, though it *is* set for individual stages, on the assumption that any shader found in a cache is likely to be found in Metal's own implicit cache. In this implementation, shader stage compilation time includes any time needed to build the `MTLComputePipelineState`s needed for vertex and tessellation control shaders in tessellated pipelines. This patch also changes compilation of the vertex stage `MTLComputePipelineState`s in tessellated pipelines to be eager instead of lazy. We really ought to have been doing this anyway, in order to report pipeline failures at creation time instead of draw time. I'm not happy, though, that we now pay the cost of all three pipeline states all the time, instead of just the ones that are used. This also gets rid of some fields of `MVKGraphicsPipeline` that were only used during pipeline construction, which should save some memory, particularly for apps that create lots of pipelines.
f9241a6
to
561e14b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM.
mvkGetElapsedNanoseconds()
- Nice. Much more appropriate than the name than I had suggested.
This provides feedback that indicates:
This is similar to the performance statistics that MoltenVK already collects.
Since we don't use any supplied base pipeline at all, this implementation never sets
VK_PIPELINE_CREATION_FEEDBACK_BASE_PIPELINE_ACCELERATION_BIT
. However, I've identified several places where we could probably use the base pipeline to accelerate pipeline creation. One day, I should probably implement that.Likewise, because we don't yet support using
MTLBinaryArchive
s,VK_PIPELINE_CREATION_FEEDBACK_APPLICATION_PIPELINE_CACHE_HIT_BIT
is never set on the whole pipeline, though it is set for individual stages, on the assumption that any shader found in a cache is likely to be found in Metal's own implicit cache.In this implementation, shader stage compilation time includes any time needed to build the
MTLComputePipelineState
s needed for vertex and tessellation control shaders in tessellated pipelines.This patch also changes compilation of the vertex stage
MTLComputePipelineState
s in tessellated pipelines to be eager instead of lazy. We really ought to have been doing this anyway, in order to report pipeline failures at creation time instead of draw time. I'm not happy, though, that we now pay the cost of all three pipeline states all the time, instead of just the ones that are used.This also gets rid of some fields of
MVKGraphicsPipeline
that were only used during pipeline construction, which should save some memory, particularly for apps that create lots of pipelines.