From af95bd70bd8ddb723107f6b2777423711f2e98ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Wed, 20 May 2020 16:19:06 +0300 Subject: [PATCH] deps: V8: cherry-pick 548f6c81d424 Original commit message: [runtime] Don't track transitions for certainly detached maps Previously such maps were marked as prototype, but that has bad performance / memory characteristics if objects are used as dictionaries. Bug: b:148346655, v8:10339 Change-Id: I287c5664c8b7799a084669aaaffe3affcf73e95f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2179322 Reviewed-by: Igor Sheludko Commit-Queue: Toon Verwaest Cr-Commit-Position: refs/heads/master@{#67537} Refs: https://github.com/v8/v8/commit/548f6c81d4246736a7feafd7995fdf6f24ed1149 PR-URL: https://github.com/nodejs/node/pull/33484 Backport-PR-URL: https://github.com/nodejs/node/pull/33553 Reviewed-By: Anna Henningsen Reviewed-By: Matheus Marchini Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- common.gypi | 2 +- deps/v8/src/json/json-parser.cc | 2 +- deps/v8/src/objects/map-inl.h | 11 ++++++++++- deps/v8/src/objects/map.cc | 32 +++++++++++++------------------- deps/v8/src/objects/map.h | 5 +++++ 5 files changed, 30 insertions(+), 22 deletions(-) diff --git a/common.gypi b/common.gypi index 7b97fbebe2deac..3b9cc3e885b254 100644 --- a/common.gypi +++ b/common.gypi @@ -34,7 +34,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.37', + 'v8_embedder_string': '-node.38', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/json/json-parser.cc b/deps/v8/src/json/json-parser.cc index e49775704db86c..ae120f54bd1bc7 100644 --- a/deps/v8/src/json/json-parser.cc +++ b/deps/v8/src/json/json-parser.cc @@ -828,7 +828,7 @@ MaybeHandle JsonParser::ParseJsonValue() { Map maybe_feedback = JSObject::cast(*element_stack.back()).map(); // Don't consume feedback from objects with a map that's detached // from the transition tree. - if (!maybe_feedback.GetBackPointer().IsUndefined(isolate_)) { + if (!maybe_feedback.IsDetached(isolate_)) { feedback = handle(maybe_feedback, isolate_); } } diff --git a/deps/v8/src/objects/map-inl.h b/deps/v8/src/objects/map-inl.h index 48bb86e2dab454..6204a5043bbf8c 100644 --- a/deps/v8/src/objects/map-inl.h +++ b/deps/v8/src/objects/map-inl.h @@ -119,6 +119,12 @@ bool Map::CanHaveFastTransitionableElementsKind() const { return CanHaveFastTransitionableElementsKind(instance_type()); } +bool Map::IsDetached(Isolate* isolate) const { + if (is_prototype_map()) return true; + return instance_type() == JS_OBJECT_TYPE && NumberOfOwnDescriptors() > 0 && + GetBackPointer().IsUndefined(isolate); +} + // static void Map::GeneralizeIfCanHaveTransitionableFastElementsKind( Isolate* isolate, InstanceType instance_type, @@ -697,7 +703,10 @@ void Map::AppendDescriptor(Isolate* isolate, Descriptor* desc) { DEF_GETTER(Map, GetBackPointer, HeapObject) { Object object = constructor_or_backpointer(isolate); - if (object.IsMap(isolate)) { + // This is the equivalent of IsMap() but avoids reading the instance type so + // it can be used concurrently without acquire load. + if (object.IsHeapObject() && HeapObject::cast(object).map(isolate) == + GetReadOnlyRoots(isolate).meta_map()) { return Map::cast(object); } // Can't use ReadOnlyRoots(isolate) as this isolate could be produced by diff --git a/deps/v8/src/objects/map.cc b/deps/v8/src/objects/map.cc index a672d6580a0837..e5594ae78ccfa0 100644 --- a/deps/v8/src/objects/map.cc +++ b/deps/v8/src/objects/map.cc @@ -646,7 +646,7 @@ Map Map::FindRootMap(Isolate* isolate) const { // Initial map always owns descriptors and doesn't have unused entries // in the descriptor array. DCHECK(result.owns_descriptors()); - DCHECK_EQ(result.NumberOfOwnDescriptors(), + DCHECK_LE(result.NumberOfOwnDescriptors(), result.instance_descriptors().number_of_descriptors()); return result; } @@ -1192,7 +1192,7 @@ Map Map::FindElementsKindTransitionedMap(Isolate* isolate, DisallowHeapAllocation no_allocation; DisallowDeoptimization no_deoptimization(isolate); - if (is_prototype_map()) return Map(); + if (IsDetached(isolate)) return Map(); ElementsKind kind = elements_kind(); bool packed = IsFastPackedElementsKind(kind); @@ -1325,7 +1325,7 @@ static Handle AddMissingElementsTransitions(Isolate* isolate, ElementsKind kind = map->elements_kind(); TransitionFlag flag; - if (map->is_prototype_map()) { + if (map->IsDetached(isolate)) { flag = OMIT_TRANSITION; } else { flag = INSERT_TRANSITION; @@ -1687,15 +1687,15 @@ void Map::ConnectTransition(Isolate* isolate, Handle parent, child->may_have_interesting_symbols()); if (!parent->GetBackPointer().IsUndefined(isolate)) { parent->set_owns_descriptors(false); - } else { + } else if (!parent->IsDetached(isolate)) { // |parent| is initial map and it must keep the ownership, there must be no // descriptors in the descriptors array that do not belong to the map. DCHECK(parent->owns_descriptors()); DCHECK_EQ(parent->NumberOfOwnDescriptors(), parent->instance_descriptors().number_of_descriptors()); } - if (parent->is_prototype_map()) { - DCHECK(child->is_prototype_map()); + if (parent->IsDetached(isolate)) { + DCHECK(child->IsDetached(isolate)); if (FLAG_trace_maps) { LOG(isolate, MapEvent("Transition", *parent, *child, "prototype", *name)); } @@ -1722,7 +1722,9 @@ Handle Map::CopyReplaceDescriptors( result->set_may_have_interesting_symbols(true); } - if (!map->is_prototype_map()) { + if (map->is_prototype_map()) { + result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor); + } else { if (flag == INSERT_TRANSITION && TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) { result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor); @@ -1733,19 +1735,11 @@ Handle Map::CopyReplaceDescriptors( descriptors->GeneralizeAllFields(); result->InitializeDescriptors(isolate, *descriptors, LayoutDescriptor::FastPointerLayout()); - // If we were trying to insert a transition but failed because there are - // too many transitions already, mark the object as a prototype to avoid - // tracking transitions from the detached map. - if (flag == INSERT_TRANSITION) { - result->set_is_prototype_map(true); - } } - } else { - result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor); } if (FLAG_trace_maps && // Mirror conditions above that did not call ConnectTransition(). - (map->is_prototype_map() || + (map->IsDetached(isolate) || !(flag == INSERT_TRANSITION && TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) { LOG(isolate, MapEvent("ReplaceDescriptors", *map, *result, reason, @@ -1926,7 +1920,7 @@ Handle Map::AsLanguageMode(Isolate* isolate, Handle initial_map, } Handle Map::CopyForElementsTransition(Isolate* isolate, Handle map) { - DCHECK(!map->is_prototype_map()); + DCHECK(!map->IsDetached(isolate)); Handle new_map = CopyDropDescriptors(isolate, map); if (map->owns_descriptors()) { @@ -2126,7 +2120,7 @@ Handle Map::TransitionToDataProperty(Isolate* isolate, Handle map, StoreOrigin store_origin) { RuntimeCallTimerScope stats_scope( isolate, *map, - map->is_prototype_map() + map->IsDetached(isolate) ? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty : RuntimeCallCounterId::kMap_TransitionToDataProperty); @@ -2238,7 +2232,7 @@ Handle Map::TransitionToAccessorProperty(Isolate* isolate, Handle map, PropertyAttributes attributes) { RuntimeCallTimerScope stats_scope( isolate, - map->is_prototype_map() + map->IsDetached(isolate) ? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty : RuntimeCallCounterId::kMap_TransitionToAccessorProperty); diff --git a/deps/v8/src/objects/map.h b/deps/v8/src/objects/map.h index ef16019685f1c2..f87dcd06f9b1ab 100644 --- a/deps/v8/src/objects/map.h +++ b/deps/v8/src/objects/map.h @@ -428,6 +428,11 @@ class Map : public HeapObject { inline bool has_sealed_elements() const; inline bool has_frozen_elements() const; + // Weakly checks whether a map is detached from all transition trees. If this + // returns true, the map is guaranteed to be detached. If it returns false, + // there is no guarantee it is attached. + inline bool IsDetached(Isolate* isolate) const; + // Returns true if the current map doesn't have DICTIONARY_ELEMENTS but if a // map with DICTIONARY_ELEMENTS was found in the prototype chain. bool DictionaryElementsInPrototypeChainOnly(Isolate* isolate);