New issue
Advanced search Search tips

Issue 870959 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

`WebGLRenderingContext` constant access is slower than expected

Reported by j...@joshgroves.com, Aug 4

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3511.0 Safari/537.36

Steps to reproduce the problem:
1. Run benchmark at https://jsperf.com/literal-vs-const-vs-member/1 (sourced from discussion at https://stackoverflow.com/a/50532589/) to test performance of access to WebGL constants
2. Compare with manually inlined or frozen objects results

What is the expected behavior?
Ideally, access to constants on `WebGLRenderingContext` and `WebGL2RenderingContext` should perform comparably to manually inlining the values.

What went wrong?
When accessing these constants through `window.{WebGLRenderingContext|WebGL2RenderingContext}`, presumably {`WebGLRenderingContext`|`WebGL2RenderingContext`} access have to be checked (i.e. whether they've been replaced on `window`). However, because this is not typically the case, ideally these accesses could be faster than other globals attached to `window`.

Did this work before? N/A 

Chrome version: 70.0.3511.0  Channel: canary
OS Version: 10.0
Flash Version: 

It appears that applications are currently using workarounds in order to force inlining or faster access, such as redefining all relevant WebGL constants. It would be useful to avoid this to reduce web application size and use the constants provided by the WebGL API.
 
Components: -Blink Blink>Bindings Blink>WebGL
Labels: Needs-Triage-M70
Labels: -OS-Windows
Status: Untriaged (was: Unconfirmed)
Based on an experiment I added here:
https://jsperf.com/literal-vs-const-vs-member/2

it looks like while WebGLRenderingContext.TEXTURE_2D is slow, gl.TEXTURE_2D (that is, accessing the enum via an actual WebGL context instance) is reasonably fast.

This would seem to support the hypothesis that it's the indirection via `window.` that is causing the slowness.

Leaving this as untriaged for the bindings team to decide the priority. For context, I think most (but certainly not all) apps access the GL enum constants via a WebGL context instance.
(FYI, though, look at the stack overflow link for an example of why an app might prefer to use WebGLRenderingContext.TEXTURE_2D over gl.TEXTURE_2D.)
It gets about 10x faster if you just do this:

window.WebGLRenderingKontext = window.WebGLRenderingContext;

and use the new name. A profile shows that we spend basically all of our time in the code to call a custom getter that we have registered. If we can get away with turning this into a plain data property, this should get dramatically faster.
In fact we do have that optimization for full interfaces (using the V8 lazy data attribute feature). Perhaps it just doesn't exist yet for partial interfaces.
Scratch that -- we do, but it doesn't apply to constructors guarded by RuntimeEnabledFeatures. So putting this behind an (always-on!) runtime-enabled flag for OffscreenCanvas caused this to regress.
Owner: jbroman@chromium.org
Status: Started (was: Untriaged)
self-assigning since I've started looking into this now
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ef4726cf9fd7906bad8258041bf27b86cdc6b21

commit 2ef4726cf9fd7906bad8258041bf27b86cdc6b21
Author: Jeremy Roman <jbroman@chromium.org>
Date: Fri Aug 10 15:25:17 2018

Support lazy attributes for runtime-enabled constructors.

Lack of this caused us to use a non-lazy attribute, which is much slower.
Notably access to the "WebGLRenderingContext" global in Window regressed when
it was conditionally exposed on Worker (for OffscreenCanvas).

To avoid further code duplication, this CL moves this behavior into a bit on
V8DOMConfiguration::AttributeConfiguration, and makes the bindings generator
use that for attributes both lazy and not.

Bug:  870959 
Change-Id: If8d68ef452b384479202d79be9defe2ecac72993
Reviewed-on: https://chromium-review.googlesource.com/1167955
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582172}
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-log-side-effects-expected.txt
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/core/v8/v8_dom_configuration.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/core/v8/v8_dom_configuration.h
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/scripts/v8_interface.py
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/templates/attributes.cpp.tmpl
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/templates/interface.cpp.tmpl
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_svg_test_interface.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_attributes.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_callback_functions.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_integer_indexed.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_integer_indexed_global.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_integer_indexed_primary_global.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_2.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_3.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_check_security.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_conditional_secure_context.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_document.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_event_init_constructor.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_named_constructor.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_node.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_origin_trial_enabled.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_secure_context.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_node.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/core/v8_test_typedefs.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/modules/v8_test_inherited_legacy_unenumerable_named_properties.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/modules/v8_test_interface_5.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/modules/v8_test_interface_partial.cc
[modify] https://crrev.com/2ef4726cf9fd7906bad8258041bf27b86cdc6b21/third_party/blink/renderer/bindings/tests/results/modules/v8_test_sub_object.cc

Status: Fixed (was: Started)

Sign in to add a comment