Skip to content

Commit 548f6c8

Browse files
verwaestCommit Bot
verwaest
authored and
Commit Bot
committed
[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}
1 parent fe8ff5f commit 548f6c8

4 files changed

Lines changed: 29 additions & 21 deletions

File tree

‎src/json/json-parser.cc‎

Lines changed: 1 addition & 1 deletion
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);

‎src/objects/map-inl.h‎

Lines changed: 10 additions & 1 deletion
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

‎src/objects/map.cc‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ Map Map::FindRootMap(Isolate* isolate) const {
658658
if (back.IsUndefined(isolate)) {
659659
// Initial map must not contain descriptors in the descriptors array
660660
// that do not belong to the map.
661-
DCHECK_EQ(result.NumberOfOwnDescriptors(),
661+
DCHECK_LE(result.NumberOfOwnDescriptors(),
662662
result.instance_descriptors().number_of_descriptors());
663663
return result;
664664
}
@@ -1221,7 +1221,7 @@ Map Map::FindElementsKindTransitionedMap(Isolate* isolate,
12211221
DisallowHeapAllocation no_allocation;
12221222
DisallowDeoptimization no_deoptimization(isolate);
12231223

1224-
if (is_prototype_map()) return Map();
1224+
if (IsDetached(isolate)) return Map();
12251225

12261226
ElementsKind kind = elements_kind();
12271227
bool packed = IsFastPackedElementsKind(kind);
@@ -1354,7 +1354,7 @@ static Handle<Map> AddMissingElementsTransitions(Isolate* isolate,
13541354

13551355
ElementsKind kind = map->elements_kind();
13561356
TransitionFlag flag;
1357-
if (map->is_prototype_map()) {
1357+
if (map->IsDetached(isolate)) {
13581358
flag = OMIT_TRANSITION;
13591359
} else {
13601360
flag = INSERT_TRANSITION;
@@ -1721,14 +1721,14 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> parent,
17211721
child->may_have_interesting_symbols());
17221722
if (!parent->GetBackPointer().IsUndefined(isolate)) {
17231723
parent->set_owns_descriptors(false);
1724-
} else {
1724+
} else if (!parent->IsDetached(isolate)) {
17251725
// |parent| is initial map and it must not contain descriptors in the
17261726
// descriptors array that do not belong to the map.
17271727
DCHECK_EQ(parent->NumberOfOwnDescriptors(),
17281728
parent->instance_descriptors().number_of_descriptors());
17291729
}
1730-
if (parent->is_prototype_map()) {
1731-
DCHECK(child->is_prototype_map());
1730+
if (parent->IsDetached(isolate)) {
1731+
DCHECK(child->IsDetached(isolate));
17321732
if (FLAG_trace_maps) {
17331733
LOG(isolate, MapEvent("Transition", parent, child, "prototype", name));
17341734
}
@@ -1755,7 +1755,9 @@ Handle<Map> Map::CopyReplaceDescriptors(
17551755
result->set_may_have_interesting_symbols(true);
17561756
}
17571757

1758-
if (!map->is_prototype_map()) {
1758+
if (map->is_prototype_map()) {
1759+
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
1760+
} else {
17591761
if (flag == INSERT_TRANSITION &&
17601762
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
17611763
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
@@ -1766,19 +1768,11 @@ Handle<Map> Map::CopyReplaceDescriptors(
17661768
descriptors->GeneralizeAllFields();
17671769
result->InitializeDescriptors(isolate, *descriptors,
17681770
LayoutDescriptor::FastPointerLayout());
1769-
// If we were trying to insert a transition but failed because there are
1770-
// too many transitions already, mark the object as a prototype to avoid
1771-
// tracking transitions from the detached map.
1772-
if (flag == INSERT_TRANSITION) {
1773-
result->set_is_prototype_map(true);
1774-
}
17751771
}
1776-
} else {
1777-
result->InitializeDescriptors(isolate, *descriptors, *layout_descriptor);
17781772
}
17791773
if (FLAG_trace_maps &&
17801774
// Mirror conditions above that did not call ConnectTransition().
1781-
(map->is_prototype_map() ||
1775+
(map->IsDetached(isolate) ||
17821776
!(flag == INSERT_TRANSITION &&
17831777
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
17841778
LOG(isolate, MapEvent("ReplaceDescriptors", map, result, reason,
@@ -1960,7 +1954,7 @@ Handle<Map> Map::AsLanguageMode(Isolate* isolate, Handle<Map> initial_map,
19601954
}
19611955

19621956
Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
1963-
DCHECK(!map->is_prototype_map());
1957+
DCHECK(!map->IsDetached(isolate));
19641958
Handle<Map> new_map = CopyDropDescriptors(isolate, map);
19651959

19661960
if (map->owns_descriptors()) {
@@ -2161,7 +2155,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
21612155
StoreOrigin store_origin) {
21622156
RuntimeCallTimerScope stats_scope(
21632157
isolate,
2164-
map->is_prototype_map()
2158+
map->IsDetached(isolate)
21652159
? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
21662160
: RuntimeCallCounterId::kMap_TransitionToDataProperty);
21672161

@@ -2275,7 +2269,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
22752269
PropertyAttributes attributes) {
22762270
RuntimeCallTimerScope stats_scope(
22772271
isolate,
2278-
map->is_prototype_map()
2272+
map->IsDetached(isolate)
22792273
? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty
22802274
: RuntimeCallCounterId::kMap_TransitionToAccessorProperty);
22812275

‎src/objects/map.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,11 @@ class Map : public HeapObject {
422422
inline bool has_sealed_elements() const;
423423
inline bool has_frozen_elements() const;
424424

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

0 commit comments

Comments
 (0)