Skip to content

Commit 9b5b6ee

Browse files
o-V8 LUCI CQ
o-
authored and
V8 LUCI CQ
committed
[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}
1 parent 54885fe commit 9b5b6ee

6 files changed

Lines changed: 76 additions & 20 deletions

File tree

‎src/objects/js-function.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,13 +1083,13 @@ MaybeHandle<Map> JSFunction::GetDerivedMap(Isolate* isolate,
10831083
isolate);
10841084
prototype = handle(realm_constructor->prototype(), isolate);
10851085
}
1086-
1087-
Handle<Map> map = Map::CopyInitialMap(isolate, constructor_initial_map);
1088-
map->set_new_target_is_base(false);
10891086
CHECK(IsJSReceiver(*prototype));
1090-
if (map->prototype() != *prototype)
1091-
Map::SetPrototype(isolate, map, Handle<HeapObject>::cast(prototype));
1092-
map->SetConstructor(*constructor);
1087+
DCHECK_EQ(constructor_initial_map->constructor_or_back_pointer(),
1088+
*constructor);
1089+
1090+
Handle<Map> map = Map::TransitionToDerivedMap(
1091+
isolate, constructor_initial_map, Handle<HeapObject>::cast(prototype));
1092+
DCHECK_EQ(map->constructor_or_back_pointer(), *constructor);
10931093
return map;
10941094
}
10951095

‎src/objects/map.cc‎

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

23342334
Handle<Map> Map::TransitionToPrototype(Isolate* isolate, Handle<Map> map,
23352335
Handle<HeapObject> prototype) {
2336-
Handle<Map> new_map =
2337-
TransitionsAccessor::GetPrototypeTransition(isolate, map, prototype);
2336+
Handle<Map> new_map = TransitionsAccessor::GetPrototypeTransition(
2337+
isolate, map, prototype, map->new_target_is_base());
23382338
if (new_map.is_null()) {
23392339
new_map = Copy(isolate, map, "TransitionToPrototype");
23402340
TransitionsAccessor::PutPrototypeTransition(isolate, map, prototype,
23412341
new_map);
2342-
Map::SetPrototype(isolate, new_map, prototype);
2342+
if (*prototype != map->prototype()) {
2343+
Map::SetPrototype(isolate, new_map, prototype);
2344+
}
2345+
}
2346+
return new_map;
2347+
}
2348+
2349+
Handle<Map> Map::TransitionToDerivedMap(Isolate* isolate, Handle<Map> map,
2350+
Handle<HeapObject> prototype) {
2351+
Handle<Map> new_map = TransitionsAccessor::GetPrototypeTransition(
2352+
isolate, map, prototype, /* new_target_is_base */ false);
2353+
if (new_map.is_null()) {
2354+
new_map = CopyInitialMap(isolate, map);
2355+
TransitionsAccessor::PutPrototypeTransition(isolate, map, prototype,
2356+
new_map);
2357+
if (*prototype != map->prototype()) {
2358+
Map::SetPrototype(isolate, new_map, prototype);
2359+
}
2360+
new_map->set_new_target_is_base(false);
23432361
}
23442362
return new_map;
23452363
}

‎src/objects/map.h‎

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

862+
V8_EXPORT_PRIVATE static Handle<Map> TransitionToDerivedMap(
863+
Isolate* isolate, Handle<Map> map, Handle<HeapObject> prototype);
864+
862865
static Handle<Map> TransitionToImmutableProto(Isolate* isolate,
863866
Handle<Map> map);
864867

‎src/objects/transitions.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,10 @@ void TransitionsAccessor::PutPrototypeTransition(Isolate* isolate,
441441

442442
// static
443443
Handle<Map> TransitionsAccessor::GetPrototypeTransition(
444-
Isolate* isolate, Handle<Map> map, Handle<Object> prototype) {
444+
Isolate* isolate, Handle<Map> map, Handle<Object> prototype_handle,
445+
bool new_target_is_base) {
445446
DisallowGarbageCollection no_gc;
447+
Object prototype = *prototype_handle;
446448
WeakFixedArray cache = GetPrototypeTransitions(isolate, map);
447449
int length = TransitionArray::NumberOfPrototypeTransitions(cache);
448450
for (int i = 0; i < length; i++) {
@@ -452,7 +454,8 @@ Handle<Map> TransitionsAccessor::GetPrototypeTransition(
452454
HeapObject heap_object;
453455
if (target->GetHeapObjectIfWeak(&heap_object)) {
454456
Map target_map = Map::cast(heap_object);
455-
if (target_map->prototype() == *prototype) {
457+
if (target_map->prototype() == prototype &&
458+
target_map->new_target_is_base() == new_target_is_base) {
456459
return handle(target_map, isolate);
457460
}
458461
}

‎src/objects/transitions.h‎

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

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

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

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2023 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
8+
class A {};
9+
class B {};
10+
class C {};
11+
class D {};
12+
class E {};
13+
class V { constructor() { this.v = 1 } };
14+
class W { constructor() { this.w = 1 } };
15+
class X { constructor() { this.x = 1 } };
16+
class Y { constructor() { this.y = 1 } };
17+
class Z { constructor() { this.z = 1 } };
18+
19+
var ctrs = [
20+
function() {},
21+
A,B,C,D,E,V,W,X,Y,Z
22+
];
23+
24+
for (var it = 0; it < 20; ++it) {
25+
for (var i in ctrs) {
26+
for (var j in ctrs) {
27+
assertTrue(%HaveSameMap(Reflect.construct(ctrs[i],[],ctrs[j]),
28+
Reflect.construct(ctrs[i],[],ctrs[j])));
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)