Return this in Context::native_context if receiver is a native context. (issue 1214903017 by verwaest@chromium.org)

33 views
Skip to first unread message

verw...@chromium.org

unread,
Jul 1, 2015, 9:30:11 AM7/1/15
to jkum...@chromium.org, v8-...@googlegroups.com
Reviewers: Jakob,

Message:
ptal

Description:
Return this in Context::native_context if receiver is a native context.

Checking for native context is faster than checking for global object.
Additionally it speeds up the case were it actually is the native context,
while
not slowing down the alternative case. The bootstrapper only needs to
access the
native context from the native context, so this avoids the expensive
fallback.

BUG=

Please review this at https://codereview.chromium.org/1214903017/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+3, -13 lines):
M src/contexts.cc


Index: src/contexts.cc
diff --git a/src/contexts.cc b/src/contexts.cc
index
61b83c2c4e546b7a37d25007ee0ea26d1de6ef4f..d434cf3a6d0bd6d0dbc1590d44f8d178767f58bc
100644
--- a/src/contexts.cc
+++ b/src/contexts.cc
@@ -85,22 +85,12 @@ Context* Context::script_context() {


Context* Context::native_context() {
+ if (IsNativeContext()) return this;
// Fast case: the global object for this context has been set. In
// that case, the global object has a direct pointer to the global
// context.
- if (global_object()->IsGlobalObject()) {
- return global_object()->native_context();
- }
-
- // During bootstrapping, the global object might not be set and we
- // have to search the context chain to find the native context.
- DCHECK(this->GetIsolate()->bootstrapper()->IsActive());
- Context* current = this;
- while (!current->IsNativeContext()) {
- JSFunction* closure = JSFunction::cast(current->closure());
- current = Context::cast(closure->context());
- }
- return current;
+ DCHECK(global_object()->IsGlobalObject());
+ return global_object()->native_context();
}




jkum...@chromium.org

unread,
Jul 1, 2015, 10:14:28 AM7/1/15
to verw...@chromium.org, v8-...@googlegroups.com
LGTM with nit.


https://codereview.chromium.org/1214903017/diff/1/src/contexts.cc
File src/contexts.cc (right):

https://codereview.chromium.org/1214903017/diff/1/src/contexts.cc#newcode89
src/contexts.cc:89: // Fast case: the global object for this context has
been set. In
nit: outdated comment.

https://codereview.chromium.org/1214903017/

verw...@chromium.org

unread,
Jul 1, 2015, 10:21:42 AM7/1/15
to jkum...@chromium.org, v8-...@googlegroups.com

commit-bot@chromium.org via codereview.chromium.org

unread,
Jul 1, 2015, 10:22:39 AM7/1/15
to verw...@chromium.org, jkum...@chromium.org, commi...@chromium.org, v8-...@googlegroups.com

commit-bot@chromium.org via codereview.chromium.org

unread,
Jul 1, 2015, 11:34:24 AM7/1/15
to verw...@chromium.org, jkum...@chromium.org, commi...@chromium.org, v8-...@googlegroups.com
Committed patchset #2 (id:20001)

https://codereview.chromium.org/1214903017/

commit-bot@chromium.org via codereview.chromium.org

unread,
Jul 1, 2015, 11:34:44 AM7/1/15
to verw...@chromium.org, jkum...@chromium.org, commi...@chromium.org, v8-...@googlegroups.com
Patchset 2 (id:??) landed as
https://crrev.com/fdc5c1343cb5786fa0a21feefa34a472a64369f1
Cr-Commit-Position: refs/heads/master@{#29423}

https://codereview.chromium.org/1214903017/
Reply all
Reply to author
Forward
0 new messages