Skip to content

Commit 748720e

Browse files
dominykascodebytere
authored andcommitted
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 <[email protected]> Commit-Queue: Toon Verwaest <[email protected]> Cr-Commit-Position: refs/heads/master@{#67537} Refs: v8/v8@548f6c8 PR-URL: #33484 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent c8f79d8 commit 748720e

File tree

5 files changed

+30
-22
lines changed

5 files changed

+30
-22
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.16',
39+
'v8_embedder_string': '-node.17',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/src/json/json-parser.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,7 @@ MaybeHandle<Object> JsonParser<Char>::ParseJsonValue() {
838838
Map maybe_feedback = JSObject::cast(*element_stack.back()).map();
839839
// Don't consume feedback from objects with a map that's detached
840840
// from the transition tree.
841-
if (!maybe_feedback.GetBackPointer().IsUndefined(isolate_)) {
841+
if (!maybe_feedback.IsDetached(isolate_)) {
842842
feedback = handle(maybe_feedback, isolate_);
843843
if (feedback->is_deprecated()) {
844844
feedback = Map::Update(isolate_, feedback);

deps/v8/src/objects/map-inl.h

+10-1
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@ bool Map::CanHaveFastTransitionableElementsKind() const {
123123
return CanHaveFastTransitionableElementsKind(instance_type());
124124
}
125125

126+
bool Map::IsDetached(Isolate* isolate) const {
127+
if (is_prototype_map()) return true;
128+
return instance_type() == JS_OBJECT_TYPE && NumberOfOwnDescriptors() > 0 &&
129+
GetBackPointer().IsUndefined(isolate);
130+
}
131+
126132
// static
127133
void Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
128134
Isolate* isolate, InstanceType instance_type,
@@ -715,7 +721,10 @@ void Map::AppendDescriptor(Isolate* isolate, Descriptor* desc) {
715721

716722
DEF_GETTER(Map, GetBackPointer, HeapObject) {
717723
Object object = constructor_or_backpointer(isolate);
718-
if (object.IsMap(isolate)) {
724+
// This is the equivalent of IsMap() but avoids reading the instance type so
725+
// it can be used concurrently without acquire load.
726+
if (object.IsHeapObject() && HeapObject::cast(object).map(isolate) ==
727+
GetReadOnlyRoots(isolate).meta_map()) {
719728
return Map::cast(object);
720729
}
721730
// Can't use ReadOnlyRoots(isolate) as this isolate could be produced by

deps/v8/src/objects/map.cc

+13-19
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ Map Map::FindRootMap(Isolate* isolate) const {
655655
if (back.IsUndefined(isolate)) {
656656
// Initial map must not contain descriptors in the descriptors array
657657
// that do not belong to the map.
658-
DCHECK_EQ(result.NumberOfOwnDescriptors(),
658+
DCHECK_LE(result.NumberOfOwnDescriptors(),
659659
result.instance_descriptors().number_of_descriptors());
660660
return result;
661661
}
@@ -1205,7 +1205,7 @@ Map Map::FindElementsKindTransitionedMap(Isolate* isolate,
12051205
DisallowHeapAllocation no_allocation;
12061206
DisallowDeoptimization no_deoptimization(isolate);
12071207

1208-
if (is_prototype_map()) return Map();
1208+
if (IsDetached(isolate)) return Map();
12091209

12101210
ElementsKind kind = elements_kind();
12111211
bool packed = IsFastPackedElementsKind(kind);
@@ -1338,7 +1338,7 @@ static Handle<Map> AddMissingElementsTransitions(Isolate* isolate,
13381338

13391339
ElementsKind kind = map->elements_kind();
13401340
TransitionFlag flag;
1341-
if (map->is_prototype_map()) {
1341+
if (map->IsDetached(isolate)) {
13421342
flag = OMIT_TRANSITION;
13431343
} else {
13441344
flag = INSERT_TRANSITION;
@@ -1705,14 +1705,14 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> parent,
17051705
child->may_have_interesting_symbols());
17061706
if (!parent->GetBackPointer().IsUndefined(isolate)) {
17071707
parent->set_owns_descriptors(false);
1708-
} else {
1708+
} else if (!parent->IsDetached(isolate)) {
17091709
// |parent| is initial map and it must not contain descriptors in the
17101710
// descriptors array that do not belong to the map.
17111711
DCHECK_EQ(parent->NumberOfOwnDescriptors(),
17121712
parent->instance_descriptors().number_of_descriptors());
17131713
}
1714-
if (parent->is_prototype_map()) {
1715-
DCHECK(child->is_prototype_map());
1714+
if (parent->IsDetached(isolate)) {
1715+
DCHECK(child->IsDetached(isolate));
17161716
if (FLAG_trace_maps) {
17171717
LOG(isolate, MapEvent("Transition", parent, child, "prototype", name));
17181718
}
@@ -1739,7 +1739,9 @@ Handle<Map> Map::CopyReplaceDescriptors(
17391739
result->set_may_have_interesting_symbols(true);
17401740
}
17411741

1742-
if (!map->is_prototype_map()) {
1742+
if (map->is_prototype_map()) {
1743+
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
1744+
} else {
17431745
if (flag == INSERT_TRANSITION &&
17441746
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
17451747
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
@@ -1750,19 +1752,11 @@ Handle<Map> Map::CopyReplaceDescriptors(
17501752
descriptors->GeneralizeAllFields();
17511753
result->InitializeDescriptors(isolate, *descriptors,
17521754
LayoutDescriptor::FastPointerLayout());
1753-
// If we were trying to insert a transition but failed because there are
1754-
// too many transitions already, mark the object as a prototype to avoid
1755-
// tracking transitions from the detached map.
1756-
if (flag == INSERT_TRANSITION) {
1757-
result->set_is_prototype_map(true);
1758-
}
17591755
}
1760-
} else {
1761-
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
17621756
}
17631757
if (FLAG_trace_maps &&
17641758
// Mirror conditions above that did not call ConnectTransition().
1765-
(map->is_prototype_map() ||
1759+
(map->IsDetached(isolate) ||
17661760
!(flag == INSERT_TRANSITION &&
17671761
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
17681762
LOG(isolate, MapEvent("ReplaceDescriptors", map, result, reason,
@@ -1944,7 +1938,7 @@ Handle<Map> Map::AsLanguageMode(Isolate* isolate, Handle<Map> initial_map,
19441938
}
19451939

19461940
Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
1947-
DCHECK(!map->is_prototype_map());
1941+
DCHECK(!map->IsDetached(isolate));
19481942
Handle<Map> new_map = CopyDropDescriptors(isolate, map);
19491943

19501944
if (map->owns_descriptors()) {
@@ -2145,7 +2139,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
21452139
StoreOrigin store_origin) {
21462140
RuntimeCallTimerScope stats_scope(
21472141
isolate,
2148-
map->is_prototype_map()
2142+
map->IsDetached(isolate)
21492143
? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
21502144
: RuntimeCallCounterId::kMap_TransitionToDataProperty);
21512145

@@ -2259,7 +2253,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
22592253
PropertyAttributes attributes) {
22602254
RuntimeCallTimerScope stats_scope(
22612255
isolate,
2262-
map->is_prototype_map()
2256+
map->IsDetached(isolate)
22632257
? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty
22642258
: RuntimeCallCounterId::kMap_TransitionToAccessorProperty);
22652259

deps/v8/src/objects/map.h

+5
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,11 @@ class Map : public HeapObject {
421421
inline bool has_sealed_elements() const;
422422
inline bool has_frozen_elements() const;
423423

424+
// Weakly checks whether a map is detached from all transition trees. If this
425+
// returns true, the map is guaranteed to be detached. If it returns false,
426+
// there is no guarantee it is attached.
427+
inline bool IsDetached(Isolate* isolate) const;
428+
424429
// Returns true if the current map doesn't have DICTIONARY_ELEMENTS but if a
425430
// map with DICTIONARY_ELEMENTS was found in the prototype chain.
426431
bool DictionaryElementsInPrototypeChainOnly(Isolate* isolate);

0 commit comments

Comments
 (0)