Commit fd2548f3 authored by Toon Verwaest's avatar Toon Verwaest Committed by Commit Bot

Reland^4 "[runtime] Amortize descriptor array growing for fast-mode prototypes"

This CL:
 - stops tracking transitions for fast maps that are known to be detached
 - reuses descriptor arrays when transitioning detached maps to avoid O(n^2) performance and garbage creation

Fix2 in reland: constructor_or_backpointer can be a smi since it can also hold a user-provided function.prototype
Fix in reland: check whether the map of the back pointer is the metamap rather than reading the map of the constructor-or-backpointer slot. If the slot contains a constructor, it's possible that the object transitions while the concurrent marker is reading the map (from which it's reading the instance type); and it's possible that the transitioned map isn't set up yet fully when we read the instance type. An acquire load for the constructor-or-backpointer map would also fix it by serializing stores, but is more expensive. Checking the metamap is faster.

Original commit message:
> This avoids an O(n^2) algorithm that creates an equal amount of garbage.
> Even though the actual final descriptor array might be a little bigger,
> it reduces peak memory usage by allocating less.

Change-Id: Id99dc76a369057e5c4d76a31163605cb38a66867
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2172080Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67501}
parent 4911ab95
......@@ -439,8 +439,12 @@ void Map::MapVerify(Isolate* isolate) {
if (GetBackPointer().IsUndefined(isolate)) {
// Root maps must not have descriptors in the descriptor array that do not
// belong to the map.
CHECK_EQ(NumberOfOwnDescriptors(),
instance_descriptors().number_of_descriptors());
int nof = instance_descriptors().number_of_descriptors();
if (IsDetached(isolate)) {
CHECK(base::IsInRange(NumberOfOwnDescriptors(), 0, nof));
} else {
CHECK_EQ(NumberOfOwnDescriptors(), nof);
}
} else {
// If there is a parent map it must be non-stable.
Map parent = Map::cast(GetBackPointer());
......
......@@ -407,7 +407,14 @@ int MarkingVisitorBase<ConcreteVisitor, MarkingState>::VisitMap(Map meta_map,
DescriptorArray descriptors = map.synchronized_instance_descriptors();
size += MarkDescriptorArrayBlack(map, descriptors);
int number_of_own_descriptors = map.NumberOfOwnDescriptors();
if (number_of_own_descriptors) {
if (map.IsDetached(heap_->isolate())) {
// Mark all descriptors in detached maps. Descriptor arrays are reused as
// objects transition between maps, and its possible that a newer map dies
// before an older map. Marking all descriptors through each map
// guarantees that we don't end up with dangling references that we don't
// clear since there are no explicit transitions.
VisitDescriptors(descriptors, descriptors.number_of_descriptors());
} else if (number_of_own_descriptors) {
// It is possible that the concurrent marker observes the
// number_of_own_descriptors out of sync with the descriptors. In that
// case the marking write barrier for the descriptor array will ensure
......
......@@ -838,7 +838,7 @@ MaybeHandle<Object> JsonParser<Char>::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);
......
......@@ -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
......
......@@ -658,7 +658,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;
}
......@@ -1221,7 +1221,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);
......@@ -1354,7 +1354,7 @@ static Handle<Map> 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;
......@@ -1721,14 +1721,14 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> 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));
}
......@@ -1755,7 +1755,9 @@ Handle<Map> 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);
......@@ -1766,19 +1768,11 @@ Handle<Map> 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,
......@@ -1960,7 +1954,7 @@ Handle<Map> Map::AsLanguageMode(Isolate* isolate, Handle<Map> initial_map,
}
Handle<Map> Map::CopyForElementsTransition(Isolate* isolate, Handle<Map> map) {
DCHECK(!map->is_prototype_map());
DCHECK(!map->IsDetached(isolate));
Handle<Map> new_map = CopyDropDescriptors(isolate, map);
if (map->owns_descriptors()) {
......@@ -2161,7 +2155,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
StoreOrigin store_origin) {
RuntimeCallTimerScope stats_scope(
isolate,
map->is_prototype_map()
map->IsDetached(isolate)
? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
: RuntimeCallCounterId::kMap_TransitionToDataProperty);
......@@ -2275,7 +2269,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
PropertyAttributes attributes) {
RuntimeCallTimerScope stats_scope(
isolate,
map->is_prototype_map()
map->IsDetached(isolate)
? RuntimeCallCounterId::kPrototypeMap_TransitionToAccessorProperty
: RuntimeCallCounterId::kMap_TransitionToAccessorProperty);
......@@ -2390,19 +2384,64 @@ Handle<Map> Map::CopyAddDescriptor(Isolate* isolate, Handle<Map> map,
return ShareDescriptor(isolate, map, descriptors, descriptor);
}
int nof = map->NumberOfOwnDescriptors();
Handle<DescriptorArray> new_descriptors =
DescriptorArray::CopyUpTo(isolate, descriptors, nof, 1);
new_descriptors->Append(descriptor);
Handle<DescriptorArray> new_descriptors;
Handle<LayoutDescriptor> new_layout_descriptor;
if (map->IsDetached(isolate)) {
if (descriptors->number_of_slack_descriptors() == 0) {
int old_size = descriptors->number_of_descriptors();
if (old_size == 0) {
new_descriptors = DescriptorArray::Allocate(isolate, 0, 1);
} else {
int slack = SlackForArraySize(old_size, kMaxNumberOfDescriptors);
EnsureDescriptorSlack(isolate, map, slack);
new_descriptors = handle(map->instance_descriptors(), isolate);
}
} else {
new_descriptors = descriptors;
}
new_layout_descriptor =
FLAG_unbox_double_fields
? LayoutDescriptor::ShareAppend(isolate, map,
descriptor->GetDetails())
: handle(LayoutDescriptor::FastPointerLayout(), isolate);
} else {
int nof = map->NumberOfOwnDescriptors();
new_descriptors = DescriptorArray::CopyUpTo(isolate, descriptors, nof, 1);
new_layout_descriptor =
FLAG_unbox_double_fields
? LayoutDescriptor::New(isolate, map, new_descriptors, nof + 1)
: handle(LayoutDescriptor::FastPointerLayout(), isolate);
}
Handle<LayoutDescriptor> new_layout_descriptor =
FLAG_unbox_double_fields
? LayoutDescriptor::New(isolate, map, new_descriptors, nof + 1)
: handle(LayoutDescriptor::FastPointerLayout(), isolate);
Handle<Map> result = CopyDropDescriptors(isolate, map);
return CopyReplaceDescriptors(
isolate, map, new_descriptors, new_layout_descriptor, flag,
descriptor->GetKey(), "CopyAddDescriptor", SIMPLE_PROPERTY_TRANSITION);
if (descriptor->GetKey()->IsInterestingSymbol()) {
result->set_may_have_interesting_symbols(true);
}
{
DisallowHeapAllocation no_gc;
new_descriptors->Append(descriptor);
result->InitializeDescriptors(isolate, *new_descriptors,
*new_layout_descriptor);
}
if (flag == INSERT_TRANSITION &&
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()) {
ConnectTransition(isolate, map, result, descriptor->GetKey(),
SIMPLE_PROPERTY_TRANSITION);
}
if (FLAG_trace_maps &&
// Mirror conditions above that did not call ConnectTransition().
(map->IsDetached(isolate) ||
!(flag == INSERT_TRANSITION &&
TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
LOG(isolate, MapEvent("ReplaceDescriptors", map, result,
"CopyAddDescriptor", descriptor->GetKey()));
}
return result;
}
Handle<Map> Map::CopyInsertDescriptor(Isolate* isolate, Handle<Map> map,
......
......@@ -422,6 +422,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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment