From b5453ee7b855787156ffbaad851ba8c3727b16af Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Mon, 21 Nov 2016 15:44:03 +0100 Subject: [PATCH] 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 https://github.com/nodejs/node/issues/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: https://github.com/nodejs/node/pull/9730 Fixes: https://github.com/nodejs/node/issues/9634 Reviewed-By: Franziska Hinkelmann Reviewed-By: Matteo Collina --- deps/v8/include/v8-version.h | 2 +- deps/v8/src/bootstrapper.cc | 1 + deps/v8/src/contexts.h | 1 + deps/v8/src/crankshaft/hydrogen.cc | 49 +++++++++++++++++++----------- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index beb8042d37..df187b8307 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -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.) diff --git a/deps/v8/src/bootstrapper.cc b/deps/v8/src/bootstrapper.cc index 5142817986..cfea3208c9 100644 --- a/deps/v8/src/bootstrapper.cc +++ b/deps/v8/src/bootstrapper.cc @@ -1206,6 +1206,7 @@ void Genesis::InitializeGlobal(Handle global_object, JSObject::kHeaderSize, MaybeHandle(), Builtins::kFunctionPrototypeHasInstance, static_cast(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); diff --git a/deps/v8/src/contexts.h b/deps/v8/src/contexts.h index d73135f7a4..fd5b006192 100644 --- a/deps/v8/src/contexts.h +++ b/deps/v8/src/contexts.h @@ -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) \ diff --git a/deps/v8/src/crankshaft/hydrogen.cc b/deps/v8/src/crankshaft/hydrogen.cc index a33d2a6120..f40337e645 100644 --- a/deps/v8/src/crankshaft/hydrogen.cc +++ b/deps/v8/src/crankshaft/hydrogen.cc @@ -11563,24 +11563,37 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { HConstant::cast(right)->handle(isolate())->IsJSFunction()) { Handle function = Handle::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 initial_map(function->initial_map(), isolate()); - top_info()->dependencies()->AssumeInitialMapCantChange(initial_map); - top_info()->dependencies()->AssumePropertyCell( - isolate()->factory()->has_instance_protector()); - HInstruction* prototype = - Add(handle(initial_map->prototype(), isolate())); - HHasInPrototypeChainAndBranch* result = - New(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 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 prototype( + JSObject::cast(has_instance.map()->prototype()), isolate()); + BuildCheckPrototypeMaps(prototype, has_instance.holder()); + } + // Perform the prototype chain walk. + Handle initial_map(function->initial_map(), isolate()); + top_info()->dependencies()->AssumeInitialMapCantChange(initial_map); + HInstruction* prototype = + Add(handle(initial_map->prototype(), isolate())); + HHasInPrototypeChainAndBranch* result = + New(left, prototype); + return ast_context()->ReturnControl(result, expr->id()); + } } }