Skip to content

Commit 445dc8e

Browse files
fhinkelJonathan Darling
authored and
Jonathan Darling
committed
deps: cherry-pick 08377af from v8 upstream
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 nodejs#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@{nodejs#41059} PR-URL: nodejs#9730 Fixes: nodejs#9634 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent dd6a1e5 commit 445dc8e

File tree

4 files changed

+34
-19
lines changed

4 files changed

+34
-19
lines changed

deps/v8/include/v8-version.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#define V8_MAJOR_VERSION 5
1212
#define V8_MINOR_VERSION 4
1313
#define V8_BUILD_NUMBER 500
14-
#define V8_PATCH_LEVEL 43
14+
#define V8_PATCH_LEVEL 44
1515

1616
// Use 1 for candidates and 0 otherwise.
1717
// (Boolean macro values are not supported by all preprocessors.)

deps/v8/src/bootstrapper.cc

+1
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
12061206
JSObject::kHeaderSize, MaybeHandle<JSObject>(),
12071207
Builtins::kFunctionPrototypeHasInstance,
12081208
static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY));
1209+
native_context()->set_function_has_instance(*has_instance);
12091210

12101211
// Set the expected parameters for @@hasInstance to 1; required by builtin.
12111212
has_instance->shared()->set_internal_formal_parameter_count(1);

deps/v8/src/contexts.h

+1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ enum ContextLookupFlags {
7878
V(MAP_GET_METHOD_INDEX, JSFunction, map_get) \
7979
V(MAP_HAS_METHOD_INDEX, JSFunction, map_has) \
8080
V(MAP_SET_METHOD_INDEX, JSFunction, map_set) \
81+
V(FUNCTION_HAS_INSTANCE_INDEX, JSFunction, function_has_instance) \
8182
V(OBJECT_VALUE_OF, JSFunction, object_value_of) \
8283
V(OBJECT_TO_STRING, JSFunction, object_to_string) \
8384
V(PROMISE_CATCH_INDEX, JSFunction, promise_catch) \

deps/v8/src/crankshaft/hydrogen.cc

+31-18
Original file line numberDiff line numberDiff line change
@@ -11563,24 +11563,37 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
1156311563
HConstant::cast(right)->handle(isolate())->IsJSFunction()) {
1156411564
Handle<JSFunction> function =
1156511565
Handle<JSFunction>::cast(HConstant::cast(right)->handle(isolate()));
11566-
// Make sure the prototype of {function} is the %FunctionPrototype%, and
11567-
// it already has a meaningful initial map (i.e. we constructed at least
11568-
// one instance using the constructor {function}).
11569-
// We can only use the fast case if @@hasInstance was not used so far.
11570-
if (function->has_initial_map() &&
11571-
function->map()->prototype() ==
11572-
function->native_context()->closure() &&
11573-
!function->map()->has_non_instance_prototype() &&
11574-
isolate()->IsHasInstanceLookupChainIntact()) {
11575-
Handle<Map> initial_map(function->initial_map(), isolate());
11576-
top_info()->dependencies()->AssumeInitialMapCantChange(initial_map);
11577-
top_info()->dependencies()->AssumePropertyCell(
11578-
isolate()->factory()->has_instance_protector());
11579-
HInstruction* prototype =
11580-
Add<HConstant>(handle(initial_map->prototype(), isolate()));
11581-
HHasInPrototypeChainAndBranch* result =
11582-
New<HHasInPrototypeChainAndBranch>(left, prototype);
11583-
return ast_context()->ReturnControl(result, expr->id());
11566+
// Make sure that the {function} already has a meaningful initial map
11567+
// (i.e. we constructed at least one instance using the constructor
11568+
// {function}).
11569+
if (function->has_initial_map()) {
11570+
// Lookup @@hasInstance on the {function}.
11571+
Handle<Map> function_map(function->map(), isolate());
11572+
PropertyAccessInfo has_instance(
11573+
this, LOAD, function_map,
11574+
isolate()->factory()->has_instance_symbol());
11575+
// Check if we are using the Function.prototype[@@hasInstance].
11576+
if (has_instance.CanAccessMonomorphic() &&
11577+
has_instance.IsDataConstant() &&
11578+
has_instance.constant().is_identical_to(
11579+
isolate()->function_has_instance())) {
11580+
// Add appropriate receiver map check and prototype chain
11581+
// checks to guard the @@hasInstance lookup chain.
11582+
AddCheckMap(right, function_map);
11583+
if (has_instance.has_holder()) {
11584+
Handle<JSObject> prototype(
11585+
JSObject::cast(has_instance.map()->prototype()), isolate());
11586+
BuildCheckPrototypeMaps(prototype, has_instance.holder());
11587+
}
11588+
// Perform the prototype chain walk.
11589+
Handle<Map> initial_map(function->initial_map(), isolate());
11590+
top_info()->dependencies()->AssumeInitialMapCantChange(initial_map);
11591+
HInstruction* prototype =
11592+
Add<HConstant>(handle(initial_map->prototype(), isolate()));
11593+
HHasInPrototypeChainAndBranch* result =
11594+
New<HHasInPrototypeChainAndBranch>(left, prototype);
11595+
return ast_context()->ReturnControl(result, expr->id());
11596+
}
1158411597
}
1158511598
}
1158611599

0 commit comments

Comments
 (0)