Bug 93431 – [Chromium] Add texture upload stats to CCRenderingStats.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[David Reveman]

Add texture upload stats provided by ThrottledTextureUploader to CCRenderingStats.

[David Reveman]

Created attachment 157105 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Nat Duca]

Comment on attachment 157105 [details]
Patch

Lets also track the number of texels uploaded

I am very uneasy about the fact that this doesn't work without the streaming texture updater or any other updater.

You can track all of these things on the GPU process. Just track it on a per-stub basis (ccameron can help you with ideas on how to do this) and figure out from the render_view which stub corresponds to the compositor context. That will give you the exact same values BUT not be specific to how the compositor does its uploads.

[David Reveman]

(In reply to comment #3)
> (From update of attachment 157105 [details])
> Lets also track the number of texels uploaded
> 
> I am very uneasy about the fact that this doesn't work without the streaming texture updater or any other updater.

Streaming texture updates is not a requirement. The throttled texture uploader is required and that uploader is currently only used by the threaded compositor. We could potentially use this uploader in single thread mode as well.

> 
> You can track all of these things on the GPU process. Just track it on a per-stub basis (ccameron can help you with ideas on how to do this) and figure out from the render_view which stub corresponds to the compositor context. That will give you the exact same values BUT not be specific to how the compositor does its uploads.

I'm interested in doing this as well. I'm not sure this deprecates the current patch though. I think the current patch is useful to determine how accurate the timing reported by GL_occlusion_query is. Maybe I should add "estimated" to the total time field in the current patch?

[Nat Duca]

> I'm interested in doing this as well. I'm not sure this deprecates the current patch though. I think the current patch is useful to determine how accurate the timing reported by GL_occlusion_query is. Maybe I should add "estimated" to the total time field in the current patch?

OK fine, but I would politely request you do the other approach right away rather as well. Ideally, after you have the other in place, this gets removed.

[Adrienne Walker]

Comment on attachment 157105 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157105&action=review

> Source/WebCore/platform/graphics/chromium/ThrottledTextureUploader.cpp:81
> +    m_context->getQueryObjectuivEXT(m_queryId, Extensions3DChromium::QUERY_RESULT_EXT, &result);

Is this synchronous? What does this time represent?

[David Reveman]

(In reply to comment #6)
> (From update of attachment 157105 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157105&action=review
> 
> > Source/WebCore/platform/graphics/chromium/ThrottledTextureUploader.cpp:81
> > +    m_context->getQueryObjectuivEXT(m_queryId, Extensions3DChromium::QUERY_RESULT_EXT, &result);
> 
> Is this synchronous? What does this time represent?

No, it's not synchronous. This would block until the result is available if we didn't first check using QUERY_RESULT_AVAILABLE_EXT.

This is the GPU process time (in microseconds) spent between glBeginQuery and glEndQuery.

[Adrienne Walker]

Comment on attachment 157105 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157105&action=review

>>> Source/WebCore/platform/graphics/chromium/ThrottledTextureUploader.cpp:81
>>> +    m_context->getQueryObjectuivEXT(m_queryId, Extensions3DChromium::QUERY_RESULT_EXT, &result);
>> 
>> Is this synchronous? What does this time represent?
> 
> No, it's not synchronous. This would block until the result is available if we didn't first check using QUERY_RESULT_AVAILABLE_EXT.
> 
> This is the GPU process time (in microseconds) spent between glBeginQuery and glEndQuery.

Ah, thanks.  Why would this time differ from recording it on the GPU process (just the overhead of glBeginQuery/glEndQuery)? I think I agree with nduca that I'd prefer that this worked for all texture uploads and not just the ThrottledTextureUploader.

[David Reveman]

(In reply to comment #8)
> (From update of attachment 157105 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157105&action=review
> 
> >>> Source/WebCore/platform/graphics/chromium/ThrottledTextureUploader.cpp:81
> >>> +    m_context->getQueryObjectuivEXT(m_queryId, Extensions3DChromium::QUERY_RESULT_EXT, &result);
> >> 
> >> Is this synchronous? What does this time represent?
> > 
> > No, it's not synchronous. This would block until the result is available if we didn't first check using QUERY_RESULT_AVAILABLE_EXT.
> > 
> > This is the GPU process time (in microseconds) spent between glBeginQuery and glEndQuery.
> 
> Ah, thanks.  Why would this time differ from recording it on the GPU process (just the overhead of glBeginQuery/glEndQuery)? I think I agree with nduca that I'd prefer that this worked for all texture uploads and not just the ThrottledTextureUploader.

The GPU process might process commands from other channels in-between processing the glBeginQuery and the glEndQuery.

[Nat Duca]

I think we should begin work on the GPU process version of this. You can do it in a way that is exactly equivalent to this. I'm quite sure.

[Eric Seidel (no email)]

Comment on attachment 157105 [details]
Patch

Cleared review? from attachment 157105 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).