Sitelet https://github.com/gpuweb/gpuweb/pull/691
Skip to content

Query API: Pipeline Statistics Query - #691

Merged
kvark merged 4 commits into
gpuweb:masterfrom
haoxli:pipeline-statistics-idl
May 8, 2020
Merged

kvark merged 4 commits into
gpuweb:masterfrom
haoxli:pipeline-statistics-idl

Conversation

@haoxli

@haoxli haoxli commented Apr 7, 2020 •

Copy link
Copy Markdown
Contributor

Add extension and entrypoints for pipeline statistics query according to #614.


Preview | Diff

@Kangz Kangz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM although we should have a plan for what form the pipeline statistics are returned as, and a plan for extensibility of the pipeline statistics (what if we add tessellation and want to return the number of patches processed?)

Also I'm not 100% sure how used this feature is in practice. We can have it for now and figure out later down the line if it is useful in WebGPU's MVP or not.

@haoxli

haoxli commented Apr 10, 2020

Copy link
Copy Markdown
Contributor Author

The results returned by pipeline statistics query will be the number of vertex shader invocations, the number of primitives sent to the clip stage, the number of primitives out by the clip stage, the number of fragment shader invocations, the number of compute shader invocations in order.

Because all query data stored in the QuerySet will be resolved into a buffer, we just copy the common parts supported by all backends to the destination buffer, so if we want to expand the results, just add the corresponding copy items.

For usage scenarios, we can use these pipeline statistics results to measure the relative complexity of different parts of applications, which can help to find bottlenecks while performance tuning.

@Kangz

Kangz commented Apr 10, 2020

Copy link
Copy Markdown
Contributor

The results returned by pipeline statistics query will be the number of vertex shader invocations, the number of primitives sent to the clip stage, the number of primitives out by the clip stage, the number of fragment shader invocations, the number of compute shader invocations in order.

Because all query data stored in the QuerySet will be resolved into a buffer, we just copy the common parts supported by all backends to the destination buffer, so if we want to expand the results, just add the corresponding copy items.

Yes, the question to know in which order they are written in the buffer when you resolve the query, and with what format (u64, u32). The other question is that right now only a few, say 4, of the pipeline statistics can be non-zero. But tomorrow, for example when WebGPU gain tessellation shaders, where are we going to write the additional data? It will be weird to users if without doing anything the amount of data written increases.

@haoxli

haoxli commented Apr 17, 2020

Copy link
Copy Markdown
Contributor Author

On Vulkan, it allows users to decide which data of pipeline statistics to query by setting VkQueryPoolCreateInfo::pipelineStatistics when creating query sets (always querying all pipeline statistics data on D3D12 and Metal), maybe we can add a GPUPipelineStatisticFlags in GPUQuerySetDescriptor such as:

dictionary GPUQuerySetDescriptor : GPUObjectDescriptorBase {
    required GPUQueryType type;
    required GPUSize32 count;
    GPUPipelineStatisticFlags pipelineStatistics;
};

typedef [EnforceRange] unsigned long GPUPipelineStatisticFlags;
interface GPUPipelineStatisticBit {
    const GPUPipelineStatisticFlags VERTEX_SHADER_INVOCATIONS   = 0x01;
    const GPUPipelineStatisticFlags CLIPPER_INVOCATIONS         = 0x02;
    const GPUPipelineStatisticFlags CLIPPER_PRIMITIVES_OUT      = 0x04;
    const GPUPipelineStatisticFlags FRAGMENT_SHADER_INVOCATIONS = 0x08;
    const GPUPipelineStatisticFlags COMPUTE_SHADER_INVOCATIONS  = 0x10;
};

@Kangz

Kangz commented Apr 17, 2020

Copy link
Copy Markdown
Contributor

That addresses part of the question, which is how we could deal with extensibility. But the other part is, can you describe what format and in which order will the different statistics be returned depending on that flag?

For example if I ask a pipeline statistic query with VERTEX_SHADER_INVOCATIONS | FRAGMENT_SHADER_INVOCATIONS, which of these formats will be the one I can use to read data in shaders from the resolve buffer?

struct Statistics {
    vertexShaderInvocations : u32; // or u64?
    fragmentShaderInvocations : u32;
}
struct Statistics {
    fragmentShaderInvocations : u32;
    vertexShaderInvocations : u32;
}
struct Statistics {
    vertexShaderInvocations : u32;
    clipperInvocations : u32; // unused
    clipperPrimitives : u32; // unused
    fragmentShaderInvocations : u32;
    computeShaderInvocations : u32; // unused
}

@haoxli

haoxli commented Apr 20, 2020

Copy link
Copy Markdown
Contributor Author

For example if I ask a pipeline statistic query with VERTEX_SHADER_INVOCATIONS | FRAGMENT_SHADER_INVOCATIONS, which of these formats will be the one I can use to read data in shaders from the resolve buffer?

The results in the resolve buffer will be stpred in the first format:

struct Statistics {
    vertexShaderInvocations : u64
    fragmentShaderInvocations : u64;
}

Only the pipeline statistics of the specified flags will be wrote to resolve buffer, and the order of results is not affected by the order of flags, it will always write in this order:

struct Statistics {
    vertexShaderInvocations : u64;
    clipperInvocations : u64; 
    clipperPrimitives : u64;
    fragmentShaderInvocations : u64;
    computeShaderInvocations : u64;
}

which does the same with Vulkan.
If we set GPUQuerySetDescriptor.pipelineStatistics=0, only a zero value is wrote to the buffer.

Do I need to add these notes to the PR now?

@Kangz

Kangz commented Apr 20, 2020

Copy link
Copy Markdown
Contributor

Thanks, we need to have at least the IDL for GPUPipelineStatisticBit and an explanation of what's in the resolve buffer, even if it isn't a proper spec. Other parts of the API are mostly self-explanatory for people who know other APIs, but not this, so it's worth having the description as soon as the IDL is introduced.

Comment thread spec/index.bs Outdated

@kvark kvark left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Kangz Kangz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

Comment thread spec/index.bs Outdated
Comment thread spec/index.bs Outdated
Comment thread spec/index.bs Outdated
@Kangz

Kangz commented May 6, 2020

Copy link
Copy Markdown
Contributor

@kainino0x @kvark @JusSn, @haoxli addressed all comments, can this be merged?

@kvark

kvark commented May 6, 2020

Copy link
Copy Markdown
Contributor

Would be nice to get a thumbs up from Apple or Microsoft.

@haoxli

haoxli commented May 8, 2020

Copy link
Copy Markdown
Contributor Author

@litherum, @RafaelCintron Do you have any comments about pipeline statistics queries?

@kvark
kvark merged commit d5e25c9 into gpuweb:master May 8, 2020
JusSn pushed a commit to JusSn/gpuweb that referenced this pull request Jun 8, 2020
* Query API: Pipeline Statistics Query

* Add GPUPipelineStatisticBit and more instructions

* Use array of enum insead of bitfield in pipeline statistics

* Fix nits
JusSn pushed a commit to JusSn/gpuweb that referenced this pull request Jun 8, 2020
* Query API: Pipeline Statistics Query

* Add GPUPipelineStatisticBit and more instructions

* Use array of enum insead of bitfield in pipeline statistics

* Fix nits
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
If the texture format is depth/stencil format, it must be a full
copy for copies resource(s).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants