1363436 - Remove WebGLContext::ValidateBufferFetching() for improving the performance

Status: RESOLVED WORKSFORME

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

Description

[Daosheng Mu[:daoshengmu]]

According to Bug 1358977 Comment 7, we have a performance issue at WebGLContext::ValidateBufferFetching, and it seems to be unnecessary for us.

Comment 1

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1363436 - Simplify WebGLContext::ValidateBufferFetching() for improving the performance;

Review commit: https://reviewboard.mozilla.org/r/137520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137520/

Comment 2

[Daosheng Mu[:daoshengmu]]

After applying this patch, the FPS will be increased to 59 from 53. It gains 11.3% improvement.

Comment 3

[Peter Chang[:pchang]]

Daosheng, there is a 'override' type[1] of gfx preferences in gfxPrefs.h.
I think you can create a validationbypass pref that depends on another gfxPrefs::OverrideBase_ANGLE(). Then we can bypass validation only when ANGLE is enabled.

Jeff, do you have any concern about this approach? I think it doesn't add overhead to maintain validation flow and we can gain extra fps in comment 2 easily.

[1]http://searchfox.org/mozilla-central/source/gfx/thebes/gfxPrefs.h#497

Comment 4

[Kelsey Gilbert [:jgilbert]]

We should first try to redo this validation so that we don't need to skip it. It's really hard to tell when ANGLE changes something, and some of these validations have poor test coverage. We also need this validation to be fast on all non-ANGLE platforms, so there's value to first trying to do validation fast everywhere.

For ANGLE, we're actually more interested in skipping ANGLE's validation, not our own. It looks like ANGLE has support for this, but it doesn't seem to be being used by us yet. I thought we tried to opt into this, but at the very least, it's not being done presently.

If someone could take a look at opting in to ANGLE's skip-validation mode, that would likely be more worth our time.

Comment 5

[Michael Leu[:Lenzak](UTC+8)]

ANGLE does have some mechanism which can bypass part of its validations without changing it, if we pushed (0x31b3, LOCAL_GL_TRUE) in  attribute map when we are creating the context, ANGLE will bypass some of its validation.

However, it can only bypass some validation which are not that significant, result in little performance gain.

Do we need to modify AGNLE? I think it would need more effort when we are integrating newer ANGLE.

In addition, WebGLContext::ValidateBufferFetching is not needed only when we are using ANGLE on Windows, maybe we should try just bypass it only if we are using ANGLE on Windows, I think it should be easier to implement.

Comment 6

[Michael Leu[:Lenzak](UTC+8)]

Attachment: Bug 1363436 - New pref to disable BufferFetchingValidation under some conditions

Review commit: https://reviewboard.mozilla.org/r/138264/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/138264/

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Michael Leu[:Lenzak](UTC+8) from comment #5)
> ANGLE does have some mechanism which can bypass part of its validations
> without changing it, if we pushed (0x31b3, LOCAL_GL_TRUE) in  attribute map
> when we are creating the context, ANGLE will bypass some of its validation.
> 
> However, it can only bypass some validation which are not that significant,
> result in little performance gain.
> 
> Do we need to modify AGNLE? I think it would need more effort when we are
> integrating newer ANGLE.
> 
> In addition, WebGLContext::ValidateBufferFetching is not needed only when we
> are using ANGLE on Windows, maybe we should try just bypass it only if we
> are using ANGLE on Windows, I think it should be easier to implement.

Again, we should optimize our buffer fetching validation, not disable it. We need to run it on all platforms, and it should therefore be fast regardless of if we can skip it on Windows.

Comment 8

[Michael Leu[:Lenzak](UTC+8)]

I have tested the running time difference between bypass or not bypass WebGLContext::ValidateBufferFetching().

The test benchmark is Epic ZenGarden-wasm, and my GFX card is GTX1070.

w/o bypassing: 82210ms
w/  bypassing: 81097ms

The performance gain is not significant.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Let's WONTFIX unless we see this showing up hot in profiles.

Comment 10

[Kelsey Gilbert [:jgilbert]]

WORKSFORME is better than WONTFIX for "this doesn't seem like a problem".
We can always re-open bugs!