New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634276 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature



Sign in to add a comment

Speed up accessing window.document => treat it special

Project Member Reported by jochen@chromium.org, Aug 4 2016

Issue description

window.document is an accessor property where the getter almost always returns the same value:

if you do something like this:

var w = window.open(location.href);
var d = window.document;
setTimeout(function() { alert(w.document === d)}, 1000);

it should alert false: during the navigation in the new window, the initial document was replaced with the actual document for the new page. However, since from that point on, window.document will always return the same wrapper, and window.document is accessed a lot, we replace the getter with the actual result here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp?rcl=0&l=414

That way, the optimizing compiler will treat document as a constant and avoid looking it up multiple times (and the fact that it's a value also avoids the call to the getter).

This breaks however both the ecmascript spec and the html spec: ecmascript doesn't allow for replacing a non-configurable, non-writable getter with something else, and html says it's supposed to be a getter not a value.

To fix this, we need to make the spec compliant way fast enough. I propose to use an compiled DOM accessor for document so the cost of the call in the baseline compiler is minimal. For TF, we need to find a way to signal that the getter will always return the same value.
 
This makes sense.

I'm just curious, but have you considered implementing an invalidation logic to V8 getter (instead of inlining DOM accesors)? For example, Blink asks V8 to return the same value until Blink invalidates the value. Blink can invalidate window.foo when the foo is replaced with something else in the Blink side.

I guess the invalidation mechanism would be more generic than the Fast DOM accessors since it can be extended to non-trivial DOM accessors.

Components: -Blink>JavaScript Blink>JavaScript>API
Labels: -Type-Bug Type-Feature
Status: Available (was: Untriaged)
Summary: Speed up accessing window.document => treat it special (was: special case window.document)
Cc: fran...@chromium.org

Comment 4 by jochen@chromium.org, Sep 27 2016

Owner: peterssen@google.com
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8f43d748272536117008aa6a1b53ea52126261c1

commit 8f43d748272536117008aa6a1b53ea52126261c1
Author: vogelheim <vogelheim@chromium.org>
Date: Tue Oct 11 08:22:04 2016

Speedup access to global_proxy.* attributes/accessors.

Using a global proxy (e.g. 'window.f', 'w.f' or 'this.f') is considerably slower than evaluating just 'f'. This CL aims to perform the necessary checks at compile time and inline the accesses.

This is a follow-on CL to crrev.com/2369933005:
- The initial upload is crrev.com/2369933005 + a rebase.
- The remaining issues are the fixes requested by the reviewers on that CL.

BUG=chromium:634276

Review-Url: https://codereview.chromium.org/2403003002
Cr-Commit-Position: refs/heads/master@{#40153}

[modify] https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1/src/crankshaft/hydrogen.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/16055d51b4afc82ef60319c67b8d517220ea269d

commit 16055d51b4afc82ef60319c67b8d517220ea269d
Author: machenbach <machenbach@chromium.org>
Date: Tue Oct 11 12:48:40 2016

Revert of Speedup access to global_proxy.* attributes/accessors. (patchset #3 id:80001 of https://codereview.chromium.org/2403003002/ )

Reason for revert:
Blocks roll:
https://codereview.chromium.org/2406213002/

Original issue's description:
> Speedup access to global_proxy.* attributes/accessors.
>
> Using a global proxy (e.g. 'window.f', 'w.f' or 'this.f') is considerably slower than evaluating just 'f'. This CL aims to perform the necessary checks at compile time and inline the accesses.
>
> This is a follow-on CL to crrev.com/2369933005:
> - The initial upload is crrev.com/2369933005 + a rebase.
> - The remaining issues are the fixes requested by the reviewers on that CL.
>
> BUG=chromium:634276
>
> Committed: https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1
> Cr-Commit-Position: refs/heads/master@{#40153}

TBR=jochen@chromium.org,verwaest@chromium.org,vogelheim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:634276

Review-Url: https://codereview.chromium.org/2408133002
Cr-Commit-Position: refs/heads/master@{#40171}

[modify] https://crrev.com/16055d51b4afc82ef60319c67b8d517220ea269d/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/16055d51b4afc82ef60319c67b8d517220ea269d/src/crankshaft/hydrogen.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/041314524952a3c1bc71bd3beafbbb37319f1d22

commit 041314524952a3c1bc71bd3beafbbb37319f1d22
Author: vogelheim <vogelheim@chromium.org>
Date: Mon Oct 17 13:36:10 2016

Speedup access to global_proxy.* attributes/accessors.

Using a global proxy (e.g. 'window.f', 'w.f' or 'this.f') is considerably slower than evaluating just 'f'. This CL aims to perform the necessary checks at compile time and inline the accesses.

This is a follow-on CL to crrev.com/2369933005:
- The initial upload is crrev.com/2369933005 + a rebase.
- The remaining issues are the fixes requested by the reviewers on that CL.

BUG=chromium:634276,  chromium:654716 

Committed: https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1
Review-Url: https://codereview.chromium.org/2403003002
Cr-Original-Commit-Position: refs/heads/master@{#40153}
Cr-Commit-Position: refs/heads/master@{#40365}

[modify] https://crrev.com/041314524952a3c1bc71bd3beafbbb37319f1d22/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/041314524952a3c1bc71bd3beafbbb37319f1d22/src/crankshaft/hydrogen.h

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/36f3f90907bea457591c6c484fb554b737bbaeac

commit 36f3f90907bea457591c6c484fb554b737bbaeac
Author: vogelheim <vogelheim@chromium.org>
Date: Mon Oct 31 14:28:05 2016

Speedup access to global_proxy.* attributes/accessors.

Using a global proxy (e.g. 'window.f', 'w.f' or 'this.f') is considerably slower than evaluating just 'f'. This CL aims to perform the necessary checks at compile time and inline the accesses.

This is a follow-on CL to crrev.com/2369933005:
- The initial upload is crrev.com/2369933005 + a rebase.
- The remaining issues are the fixes requested by the reviewers on that CL.

BUG=chromium:634276,  chromium:654716 , chromium:656959

Committed: https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1
Committed: https://crrev.com/041314524952a3c1bc71bd3beafbbb37319f1d22
Review-Url: https://codereview.chromium.org/2403003002
Cr-Original-Original-Commit-Position: refs/heads/master@{#40153}
Cr-Original-Commit-Position: refs/heads/master@{#40365}
Cr-Commit-Position: refs/heads/master@{#40671}

[modify] https://crrev.com/36f3f90907bea457591c6c484fb554b737bbaeac/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/36f3f90907bea457591c6c484fb554b737bbaeac/src/crankshaft/hydrogen.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/cadcd787cf185f9fb974a1f0c12fdd13c45171e9

commit cadcd787cf185f9fb974a1f0c12fdd13c45171e9
Author: vogelheim <vogelheim@chromium.org>
Date: Fri Nov 04 13:02:40 2016

V8 support for cached accessors.

Some accessors requires little to no computation at all, its result can be
cached in a private property, avoiding the call overhead.
Calls to the getter are translated into a cheap property load.

Follow-on to crrev.com/2347523003, from peterssen@google.com

BUG=chromium:634276, v8:5548

Review-Url: https://codereview.chromium.org/2405213002
Cr-Commit-Position: refs/heads/master@{#40765}

[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/include/v8.h
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/api.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/ic/ic.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/ic/ic.h
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/lookup.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/lookup.h
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/objects-debug.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/objects-inl.h
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/objects-printer.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/objects.cc
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/src/objects.h
[modify] https://crrev.com/cadcd787cf185f9fb974a1f0c12fdd13c45171e9/test/cctest/test-api-accessors.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 8 2016

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

commit 460ec9e9068eb8bf57ce3fae68bb277ca4106f9a
Author: vogelheim <vogelheim@chromium.org>
Date: Tue Nov 08 09:08:45 2016

[CachedAccessor] for window.document.

- Implemend [CachedAccessor] attribute to enable cached accessors.
- Implement a cached accessor for window.document.

This is based on crrev.com/2335203006 by peterssen@google.com, with
only a minor fix added.

Intent to implement & ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/OhIFnre7ytQ

Previous (reviewed) CL:
https://codereview.chromium.org/2335203006/

Depends on V8 CL:
https://codereview.chromium.org/2405213002/

BUG=chromium:634276

Review-Url: https://codereview.chromium.org/2429343004
Cr-Commit-Position: refs/heads/master@{#430551}

[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/LayoutTests/imported/wpt/html/browsers/the-window-object/window-properties-expected.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/LayoutTests/platform/linux/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/IDLExtendedAttributes.txt
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.h
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8SVGTestInterface.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackFunctions.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestException.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceDocument.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceEventInitConstructor.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceNamedConstructor.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceNode.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceOriginTrialEnabled.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestNode.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterfacePartial.cpp
[modify] https://crrev.com/460ec9e9068eb8bf57ce3fae68bb277ca4106f9a/third_party/WebKit/Source/core/frame/Window.idl

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Assigned)
This owner is not able to receive e-mails, please re-triage.
Owner: vogelheim@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a14f22818851558ec51f04f5fa3396e242f10b78

commit a14f22818851558ec51f04f5fa3396e242f10b78
Author: bmeurer <bmeurer@chromium.org>
Date: Tue Jan 31 06:47:01 2017

[turbofan] Support fast access to the current global object.

This is essentially a port of http://crrev.com/2403003002 to TurboFan,
adding support for fast access to JSGlobalObject properties through the
current native contexts' JSGlobalProxy.

It's a slightly bigger change, since JSNativeContextSpecialization and
JSGlobalObjectSpecialization needs merging for this to work, as due to
different type feedback layout we cannot just turn a JSLoadNamed into
JSLoadGlobal operator (and same for JSStoreNamed vs. JSStoreGlobal).
This part of the change is mostly mechanical.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng
R=ishell@chromium.org, jochen@chromium.org
BUG=chromium:634276,v8:5267

Review-Url: https://codereview.chromium.org/2664853002
Cr-Commit-Position: refs/heads/master@{#42805}

[modify] https://crrev.com/a14f22818851558ec51f04f5fa3396e242f10b78/BUILD.gn
[delete] https://crrev.com/daad4319c960b714c270b2c603b1f03bbc6ade14/src/compiler/js-global-object-specialization.cc
[delete] https://crrev.com/daad4319c960b714c270b2c603b1f03bbc6ade14/src/compiler/js-global-object-specialization.h
[modify] https://crrev.com/a14f22818851558ec51f04f5fa3396e242f10b78/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/a14f22818851558ec51f04f5fa3396e242f10b78/src/compiler/js-native-context-specialization.h
[modify] https://crrev.com/a14f22818851558ec51f04f5fa3396e242f10b78/src/compiler/pipeline.cc
[modify] https://crrev.com/a14f22818851558ec51f04f5fa3396e242f10b78/src/v8.gyp

Sign in to add a comment