Skip to content

Commit 96adcd5

Browse files
o-V8 LUCI CQ
o-
authored and
V8 LUCI CQ
committed
Revert "[maps] Create map transitions for derived maps with custom proto"
This reverts commit 9b5b6ee. Reason for revert: crbug:1492212 Original change's description: > [maps] Create map transitions for derived maps with custom proto > > Prevents us from creating excessive maps in the case of e.g., > Reflect.construct with a newTarget. > > Measured impact on top 10k websites is an average of 1% reduction in > map allocations. > > Bug: v8:13978 > Change-Id: I324389f0febfa47a3a179ccdb30029723278ba5e > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4750265 > Commit-Queue: Olivier Flückiger <[email protected]> > Reviewed-by: Camillo Bruni <[email protected]> > Auto-Submit: Olivier Flückiger <[email protected]> > Cr-Commit-Position: refs/heads/main@{#89520} Bug: v8:13978 Change-Id: I833f6d167e6afe2fe7a6dd4019c83032644d0069 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4942590 Auto-Submit: Olivier Flückiger <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Reviewed-by: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#90427}
1 parent 8bafba6 commit 96adcd5

6 files changed

Lines changed: 20 additions & 75 deletions

File tree

‎src/objects/js-function.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,13 +1097,13 @@ MaybeHandle<Map> JSFunction::GetDerivedMap(Isolate* isolate,
10971097
isolate);
10981098
prototype = handle(realm_constructor->prototype(), isolate);
10991099
}
1100-
CHECK(IsJSReceiver(*prototype));
1101-
DCHECK_EQ(constructor_initial_map->constructor_or_back_pointer(),
1102-
*constructor);
11031100

1104-
Handle<Map> map = Map::TransitionToDerivedMap(
1105-
isolate, constructor_initial_map, Handle<HeapObject>::cast(prototype));
1106-
DCHECK_EQ(map->constructor_or_back_pointer(), *constructor);
1101+
Handle<Map> map = Map::CopyInitialMap(isolate, constructor_initial_map);
1102+
map->set_new_target_is_base(false);
1103+
CHECK(IsJSReceiver(*prototype));
1104+
if (map->prototype() != *prototype)
1105+
Map::SetPrototype(isolate, map, Handle<HeapObject>::cast(prototype));
1106+
map->SetConstructor(*constructor);
11071107
return map;
11081108
}
11091109

‎src/objects/map.cc‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,31 +2362,13 @@ void Map::StartInobjectSlackTracking() {
23622362

23632363
Handle<Map> Map::TransitionToPrototype(Isolate* isolate, Handle<Map> map,
23642364
Handle<HeapObject> prototype) {
2365-
Handle<Map> new_map = TransitionsAccessor::GetPrototypeTransition(
2366-
isolate, map, prototype, map->new_target_is_base());
2365+
Handle<Map> new_map =
2366+
TransitionsAccessor::GetPrototypeTransition(isolate, map, prototype);
23672367
if (new_map.is_null()) {
23682368
new_map = Copy(isolate, map, "TransitionToPrototype");
23692369
TransitionsAccessor::PutPrototypeTransition(isolate, map, prototype,
23702370
new_map);
2371-
if (*prototype != map->prototype()) {
2372-
Map::SetPrototype(isolate, new_map, prototype);
2373-
}
2374-
}
2375-
return new_map;
2376-
}
2377-
2378-
Handle<Map> Map::TransitionToDerivedMap(Isolate* isolate, Handle<Map> map,
2379-
Handle<HeapObject> prototype) {
2380-
Handle<Map> new_map = TransitionsAccessor::GetPrototypeTransition(
2381-
isolate, map, prototype, /* new_target_is_base */ false);
2382-
if (new_map.is_null()) {
2383-
new_map = CopyInitialMap(isolate, map);
2384-
TransitionsAccessor::PutPrototypeTransition(isolate, map, prototype,
2385-
new_map);
2386-
if (*prototype != map->prototype()) {
2387-
Map::SetPrototype(isolate, new_map, prototype);
2388-
}
2389-
new_map->set_new_target_is_base(false);
2371+
Map::SetPrototype(isolate, new_map, prototype);
23902372
}
23912373
return new_map;
23922374
}

‎src/objects/map.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -870,9 +870,6 @@ class Map : public TorqueGeneratedMap<Map, HeapObject> {
870870
V8_EXPORT_PRIVATE static Handle<Map> TransitionToPrototype(
871871
Isolate* isolate, Handle<Map> map, Handle<HeapObject> prototype);
872872

873-
V8_EXPORT_PRIVATE static Handle<Map> TransitionToDerivedMap(
874-
Isolate* isolate, Handle<Map> map, Handle<HeapObject> prototype);
875-
876873
static Handle<Map> TransitionToImmutableProto(Isolate* isolate,
877874
Handle<Map> map);
878875

‎src/objects/transitions.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,7 @@ void TransitionsAccessor::PutPrototypeTransition(Isolate* isolate,
445445

446446
// static
447447
Handle<Map> TransitionsAccessor::GetPrototypeTransition(
448-
Isolate* isolate, Handle<Map> map, Handle<Object> prototype_handle,
449-
bool new_target_is_base) {
448+
Isolate* isolate, Handle<Map> map, Handle<Object> prototype_handle) {
450449
DisallowGarbageCollection no_gc;
451450
Tagged<Object> prototype = *prototype_handle;
452451
Tagged<WeakFixedArray> cache = GetPrototypeTransitions(isolate, map);
@@ -458,8 +457,7 @@ Handle<Map> TransitionsAccessor::GetPrototypeTransition(
458457
Tagged<HeapObject> heap_object;
459458
if (target.GetHeapObjectIfWeak(&heap_object)) {
460459
Tagged<Map> target_map = Map::cast(heap_object);
461-
if (target_map->prototype() == prototype &&
462-
target_map->new_target_is_base() == new_target_is_base) {
460+
if (target_map->prototype() == prototype) {
463461
return handle(target_map, isolate);
464462
}
465463
}

‎src/objects/transitions.h‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -124,20 +124,19 @@ class V8_EXPORT_PRIVATE TransitionsAccessor {
124124
}
125125

126126
// ===== PROTOTYPE TRANSITIONS =====
127-
// When you set the prototype of an object using the __proto__ accessor, or if
128-
// an unrelated new.target is passed to a constructor you need a new map for
129-
// the object (the prototype is stored in the map). In order not to multiply
130-
// maps unnecessarily we store these as transitions in the original map. That
131-
// way we can transition to the same map if the same prototype is set, rather
132-
// than creating a new map every time. The transitions are in the form of a
133-
// map where the keys are prototype objects and the values are the maps they
134-
// transition to. PutPrototypeTransition can trigger GC.
127+
// When you set the prototype of an object using the __proto__ accessor you
128+
// need a new map for the object (the prototype is stored in the map). In
129+
// order not to multiply maps unnecessarily we store these as transitions in
130+
// the original map. That way we can transition to the same map if the same
131+
// prototype is set, rather than creating a new map every time. The
132+
// transitions are in the form of a map where the keys are prototype objects
133+
// and the values are the maps they transition to.
134+
// PutPrototypeTransition can trigger GC.
135135
static void PutPrototypeTransition(Isolate* isolate, Handle<Map>,
136136
Handle<Object> prototype,
137137
Handle<Map> target_map);
138138
static Handle<Map> GetPrototypeTransition(Isolate* isolate, Handle<Map> map,
139-
Handle<Object> prototype,
140-
bool new_target_is_base);
139+
Handle<Object> prototype);
141140

142141
// During the first-time Map::Update and Map::TryUpdate, the migration target
143142
// map could be cached in the raw_transitions slot of the old map that is

‎test/mjsunit/regress/regress-reflect-construct.js‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)