From 748720e7b663dddf64a294b549b98f6efd37b473 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 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 15bef5bb08e9e2..6fe46076ff8def 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,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.16', + 'v8_embedder_string': '-node.17', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/json/json-parser.cc b/deps/v8/src/json/json-parser.cc index becf21cd4ed58a..da2f60d3209b4a 100644 --- a/deps/v8/src/json/json-parser.cc +++ b/deps/v8/src/json/json-parser.cc @@ -838,7 +838,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_); if (feedback->is_deprecated()) { feedback = Map::Update(isolate_, feedback); diff --git a/deps/v8/src/objects/map-inl.h b/deps/v8/src/objects/map-inl.h index 4317be20c631be..9b17805c6aa118 100644 --- a/deps/v8/src/objects/map-inl.h +++ b/deps/v8/src/objects/map-inl.h @@ -123,6 +123,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, @@ -715,7 +721,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 2dc288628c8ea6..74b2fea2fea853 100644 --- a/deps/v8/src/objects/map.cc +++ b/deps/v8/src/objects/map.cc @@ -655,7 +655,7 @@ Map Map::FindRootMap(Isolate* isolate) const { if (back.IsUndefined(isolate)) { // Initial map must not contain descriptors in the descriptors array // that do not belong to the map. - DCHECK_EQ(result.NumberOfOwnDescriptors(), + DCHECK_LE(result.NumberOfOwnDescriptors(), result.instance_descriptors().number_of_descriptors()); return result; } @@ -1205,7 +1205,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); @@ -1338,7 +1338,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; @@ -1705,14 +1705,14 @@ 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 not contain descriptors in the // descriptors array that do not belong to the map. 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)); } @@ -1739,7 +1739,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); @@ -1750,19 +1752,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, @@ -1944,7 +1938,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()) { @@ -2145,7 +2139,7 @@ Handle Map::TransitionToDataProperty(Isolate* isolate, Handle map, StoreOrigin store_origin) { RuntimeCallTimerScope stats_scope( isolate, - map->is_prototype_map() + map->IsDetached(isolate) ? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty : RuntimeCallCounterId::kMap_TransitionToDataProperty); @@ -2259,7 +2253,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 0b817e3bab6706..a84173188e4db0 100644 --- a/deps/v8/src/objects/map.h +++ b/deps/v8/src/objects/map.h @@ -421,6 +421,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);