1367919 - Add tracking for data allocation calls in WebGL

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, enhancement, P1)

Comment 1

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Comment 2

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Rebased patch to be independent of other changes (the webgl.max -> webgl.perf.max rename).

Comment 3

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Comment 4

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8871958 [details] [diff] [review]
0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Review of attachment 8871958 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLBuffer.cpp
@@ +153,4 @@
>          }
>      } else {
>          gl->fBufferData(target, size, uploadData, usage);
> +        mContext->OnDataAllocCall();

Why not put this below, outside the if/else, so we only need to write it once?

::: dom/canvas/WebGLContext.cpp
@@ -932,4 @@
>          mBackbufferNeedsClear = true;
>  
>          gl->ResetSyncCallCount("Existing WebGLContext resized.");
> -

Looks like you accidentally removed a line here.

@@ +1596,5 @@
> +
> +void
> +WebGLContext::OnDataAllocCall() const
> +{
> +   mDataAllocGLCallCount++;

Move this to the header for inlining.

@@ +1602,5 @@
> +
> +uint64_t
> +WebGLContext::GetNumGLDataAllocCalls() const
> +{
> +   return mDataAllocGLCallCount;

Move this to the header for inlining.

@@ +1610,5 @@
> +WebGLContext::OnEndOfFrame() const
> +{
> +   if (gfxPrefs::WebGLSpewFrameAllocs()) {
> +      GeneratePerfWarning("[webgl.perf.spew-frame-allocs] %" PRIu64 " Data allocations this frame.\n",
> +                     mDataAllocGLCallCount);

Align this with the previous line's `(`.

@@ +1646,3 @@
>  
>      mShouldPresent = false;
>      gl->ResetSyncCallCount("WebGLContext PresentScreenBuffer");

ResetSyncCallCount should go in OnEndOfFrame.

Comment 5

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8873157 [details] [diff] [review]
0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Review of attachment 8873157 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLBuffer.cpp
@@ +142,4 @@
>  
>          gl::GLContext::LocalErrorScope errorScope(*gl);
>          gl->fBufferData(target, size, uploadData, usage);
> +

Revert this newline.

::: dom/canvas/WebGLContext.cpp
@@ +931,4 @@
>          mResetLayer = true;
>          mBackbufferNeedsClear = true;
>  
> +        gl->ResetSyncCallCount("Existing WebGLContext resized.");

Move this to the proper patch. I think it's from the other bug.

@@ +1595,5 @@
> +void
> +WebGLContext::OnEndOfFrame() const
> +{
> +   if (gfxPrefs::WebGLSpewFrameAllocs()) {
> +      GeneratePerfWarning("[webgl.perf.spew-frame-allocs] %" PRIu64 " Data allocations this frame.\n",

s/Data/data/

No ending newlines for Generate*Warning().

@@ +1599,5 @@
> +      GeneratePerfWarning("[webgl.perf.spew-frame-allocs] %" PRIu64 " Data allocations this frame.\n",
> +                          mDataAllocGLCallCount);
> +   }
> +   mDataAllocGLCallCount = 0;
> +   gl->ResetSyncCallCount("WebGLContext OnEndOfFrame");

Move this to the proper patch. I think it's from the other bug.

Comment 7

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Comment 8

[Samuel Vargas [:svargas]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8947ca6f473bdcf6697f8953bd4be3dad0c28b23

Comment 9

[Kelsey Gilbert [:jgilbert]]

Please rebase on inbound.

Comment 10

[Samuel Vargas [:svargas]]

Attachment: 0001-Bug-1367919-Add-tracking-for-data-allocation-calls-i.patch

Applies cleanly to mozilla-inbound

Comment 11

[Samuel Vargas [:svargas]]

New Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93b438720a7b8878f11b235ded9f53d72e2c7017

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db8a4903fd27
Add tracking for data allocation calls in WebGL. r=jgilbert

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/db8a4903fd27