Skip to content

Commit

Permalink
deps: cherry-pick 08377af from v8 upstream
Browse files Browse the repository at this point in the history
Orignial commit message:

[crankshaft] No need to rely on the @@hasInstance protector.

In Crankshaft we can actually do an abstract interpretation of the
@@hasInstance lookup when optimizing instanceof and then use the
normal machinery to protect the result instead of relying on the
global @@hasInstance protector cell for optimizations.

This recovers the 100x performance drop in Node.js v7 reported in
#9634. This patch should be
easily back-mergable to Node.js v7.

BUG=v8:5640
R=yangguo@chromium.org,franzih@chromium.org

Committed: https://crrev.com/08377af957b1602396ebf3e11ae000f15ed09333
Cr-Commit-Position: refs/heads/master@{#41059}

PR-URL: #9730
Fixes: #9634
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
fhinkel authored and addaleax committed Dec 5, 2016
1 parent f76bb2a commit 03b1c31
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 19 deletions.
2 changes: 1 addition & 1 deletion deps/v8/include/v8-version.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#define V8_MAJOR_VERSION 5
#define V8_MINOR_VERSION 4
#define V8_BUILD_NUMBER 500
#define V8_PATCH_LEVEL 43
#define V8_PATCH_LEVEL 44

// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/bootstrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
JSObject::kHeaderSize, MaybeHandle<JSObject>(),
Builtins::kFunctionPrototypeHasInstance,
static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY));
native_context()->set_function_has_instance(*has_instance);

// Set the expected parameters for @@hasInstance to 1; required by builtin.
has_instance->shared()->set_internal_formal_parameter_count(1);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ enum ContextLookupFlags {
V(MAP_GET_METHOD_INDEX, JSFunction, map_get) \
V(MAP_HAS_METHOD_INDEX, JSFunction, map_has) \
V(MAP_SET_METHOD_INDEX, JSFunction, map_set) \
V(FUNCTION_HAS_INSTANCE_INDEX, JSFunction, function_has_instance) \
V(OBJECT_VALUE_OF, JSFunction, object_value_of) \
V(OBJECT_TO_STRING, JSFunction, object_to_string) \
V(PROMISE_CATCH_INDEX, JSFunction, promise_catch) \
Expand Down
49 changes: 31 additions & 18 deletions deps/v8/src/crankshaft/hydrogen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11563,24 +11563,37 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
HConstant::cast(right)->handle(isolate())->IsJSFunction()) {
Handle<JSFunction> function =
Handle<JSFunction>::cast(HConstant::cast(right)->handle(isolate()));
// Make sure the prototype of {function} is the %FunctionPrototype%, and
// it already has a meaningful initial map (i.e. we constructed at least
// one instance using the constructor {function}).
// We can only use the fast case if @@hasInstance was not used so far.
if (function->has_initial_map() &&
function->map()->prototype() ==
function->native_context()->closure() &&
!function->map()->has_non_instance_prototype() &&
isolate()->IsHasInstanceLookupChainIntact()) {
Handle<Map> initial_map(function->initial_map(), isolate());
top_info()->dependencies()->AssumeInitialMapCantChange(initial_map);
top_info()->dependencies()->AssumePropertyCell(
isolate()->factory()->has_instance_protector());
HInstruction* prototype =
Add<HConstant>(handle(initial_map->prototype(), isolate()));
HHasInPrototypeChainAndBranch* result =
New<HHasInPrototypeChainAndBranch>(left, prototype);
return ast_context()->ReturnControl(result, expr->id());
// Make sure that the {function} already has a meaningful initial map
// (i.e. we constructed at least one instance using the constructor
// {function}).
if (function->has_initial_map()) {
// Lookup @@hasInstance on the {function}.
Handle<Map> function_map(function->map(), isolate());
PropertyAccessInfo has_instance(
this, LOAD, function_map,
isolate()->factory()->has_instance_symbol());
// Check if we are using the Function.prototype[@@hasInstance].
if (has_instance.CanAccessMonomorphic() &&
has_instance.IsDataConstant() &&
has_instance.constant().is_identical_to(
isolate()->function_has_instance())) {
// Add appropriate receiver map check and prototype chain
// checks to guard the @@hasInstance lookup chain.
AddCheckMap(right, function_map);
if (has_instance.has_holder()) {
Handle<JSObject> prototype(
JSObject::cast(has_instance.map()->prototype()), isolate());
BuildCheckPrototypeMaps(prototype, has_instance.holder());
}
// Perform the prototype chain walk.
Handle<Map> initial_map(function->initial_map(), isolate());
top_info()->dependencies()->AssumeInitialMapCantChange(initial_map);
HInstruction* prototype =
Add<HConstant>(handle(initial_map->prototype(), isolate()));
HHasInPrototypeChainAndBranch* result =
New<HHasInPrototypeChainAndBranch>(left, prototype);
return ast_context()->ReturnControl(result, expr->id());
}
}
}

Expand Down

0 comments on commit 03b1c31

Please sign in to comment.