From 7bba17bcdbf724993dac6ba8f6c7953fd9712df3 Mon Sep 17 00:00:00 2001 From: ulan Date: Thu, 18 Jun 2015 08:51:53 -0700 Subject: [PATCH] Revert of Replace ad-hoc weakness in transition array with WeakCell. (patchset #5 id:80001 of https://codereview.chromium.org/1157943003/) Reason for revert: Breaks descriptor array clearing. Original issue's description: > Replace ad-hoc weakness in transition array with WeakCell. > > BUG= > > Committed: https://crrev.com/885455e99de817f86a0b5df2dc0d932cfc179749 > Cr-Commit-Position: refs/heads/master@{#29083} TBR=jkummerow@chromium.org,hpayer@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/1194673002 Cr-Commit-Position: refs/heads/master@{#29121} --- src/heap/mark-compact.cc | 46 ++++++++++++--------------------- src/heap/objects-visiting-inl.h | 22 ++++++++++++++++ src/heap/objects-visiting.h | 1 + src/transitions-inl.h | 27 +++++++------------ src/transitions.cc | 35 +++++++++++++------------ src/transitions.h | 19 +++++--------- 6 files changed, 74 insertions(+), 76 deletions(-) diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 7ee78083c..90fc13616 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -2468,39 +2468,25 @@ void MarkCompactCollector::ClearMapTransitions(Map* map, Map* dead_transition) { int transition_index = 0; - // This flag will be cleared if we find the live owner in the loop below. - bool descriptors_owner_died = true; + bool descriptors_owner_died = false; // Compact all live descriptors to the left. - if (TransitionArray::IsFullTransitionArray(transitions)) { - for (int i = 0; i < num_transitions; ++i) { - WeakCell* target_cell = - TransitionArray::cast(transitions)->GetTargetCell(i); - if (!target_cell->cleared()) { - if (Map::cast(target_cell->value())->instance_descriptors() == - descriptors) { - descriptors_owner_died = false; - } - if (i != transition_index) { - TransitionArray* t = TransitionArray::cast(transitions); - Name* key = t->GetKey(i); - t->SetKey(transition_index, key); - Object** key_slot = t->GetKeySlot(transition_index); - RecordSlot(key_slot, key_slot, key); - WeakCell* target_cell = t->GetTargetCell(i); - t->SetTargetCell(transition_index, target_cell); - Object** target_slot = t->GetTargetSlot(transition_index); - RecordSlot(target_slot, target_slot, target_cell); - } - transition_index++; + for (int i = 0; i < num_transitions; ++i) { + Map* target = TransitionArray::GetTarget(transitions, i); + if (ClearMapBackPointer(target)) { + if (target->instance_descriptors() == descriptors) { + descriptors_owner_died = true; } - } - } else if (transitions->IsWeakCell()) { - WeakCell* target_cell = WeakCell::cast(transitions); - if (!target_cell->cleared()) { - if (Map::cast(target_cell->value())->instance_descriptors() == - descriptors) { - descriptors_owner_died = false; + } else { + if (i != transition_index) { + DCHECK(TransitionArray::IsFullTransitionArray(transitions)); + TransitionArray* t = TransitionArray::cast(transitions); + Name* key = t->GetKey(i); + t->SetKey(transition_index, key); + Object** key_slot = t->GetKeySlot(transition_index); + RecordSlot(key_slot, key_slot, key); + // Target slots do not need to be recorded since maps are not compacted. + t->SetTarget(transition_index, t->GetTarget(i)); } transition_index++; } diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h index d22f6d268..433fc6dea 100644 --- a/src/heap/objects-visiting-inl.h +++ b/src/heap/objects-visiting-inl.h @@ -537,6 +537,11 @@ void StaticMarkingVisitor::VisitJSDataView(Map* map, template void StaticMarkingVisitor::MarkMapContents(Heap* heap, Map* map) { + Object* raw_transitions = map->raw_transitions(); + if (TransitionArray::IsFullTransitionArray(raw_transitions)) { + MarkTransitionArray(heap, TransitionArray::cast(raw_transitions)); + } + // Since descriptor arrays are potentially shared, ensure that only the // descriptors that belong to this map are marked. The first time a // non-empty descriptor array is marked, its header is also visited. The slot @@ -565,6 +570,23 @@ void StaticMarkingVisitor::MarkMapContents(Heap* heap, } +template +void StaticMarkingVisitor::MarkTransitionArray( + Heap* heap, TransitionArray* transitions) { + if (!StaticVisitor::MarkObjectWithoutPush(heap, transitions)) return; + + if (transitions->HasPrototypeTransitions()) { + StaticVisitor::VisitPointer(heap, + transitions->GetPrototypeTransitionsSlot()); + } + + int num_transitions = TransitionArray::NumberOfTransitions(transitions); + for (int i = 0; i < num_transitions; ++i) { + StaticVisitor::VisitPointer(heap, transitions->GetKeySlot(i)); + } +} + + template void StaticMarkingVisitor::MarkInlinedFunctionsCode(Heap* heap, Code* code) { diff --git a/src/heap/objects-visiting.h b/src/heap/objects-visiting.h index 9d1634516..1b788e893 100644 --- a/src/heap/objects-visiting.h +++ b/src/heap/objects-visiting.h @@ -438,6 +438,7 @@ class StaticMarkingVisitor : public StaticVisitorBase { // Mark pointers in a Map and its TransitionArray together, possibly // treating transitions or back pointers weak. static void MarkMapContents(Heap* heap, Map* map); + static void MarkTransitionArray(Heap* heap, TransitionArray* transitions); // Code flushing support. INLINE(static bool IsFlushable(Heap* heap, JSFunction* function)); diff --git a/src/transitions-inl.h b/src/transitions-inl.h index 870c8e7c0..f31eff96b 100644 --- a/src/transitions-inl.h +++ b/src/transitions-inl.h @@ -47,12 +47,6 @@ Object** TransitionArray::GetKeySlot(int transition_number) { } -Object** TransitionArray::GetTargetSlot(int transition_number) { - DCHECK(transition_number < number_of_transitions()); - return RawFieldOfElementAt(ToTargetIndex(transition_number)); -} - - Name* TransitionArray::GetKey(int transition_number) { DCHECK(transition_number < number_of_transitions()); return Name::cast(get(ToKeyIndex(transition_number))); @@ -77,8 +71,7 @@ void TransitionArray::SetKey(int transition_number, Name* key) { Map* TransitionArray::GetTarget(int transition_number) { DCHECK(transition_number < number_of_transitions()); - WeakCell* cell = GetTargetCell(transition_number); - return Map::cast(cell->value()); + return Map::cast(get(ToTargetIndex(transition_number))); } @@ -93,13 +86,7 @@ Map* TransitionArray::GetTarget(Object* raw_transitions, } -WeakCell* TransitionArray::GetTargetCell(int transition_number) { - DCHECK(transition_number < number_of_transitions()); - return WeakCell::cast(get(ToTargetIndex(transition_number))); -} - - -void TransitionArray::SetTargetCell(int transition_number, WeakCell* value) { +void TransitionArray::SetTarget(int transition_number, Map* value) { DCHECK(transition_number < number_of_transitions()); set(ToTargetIndex(transition_number), value); } @@ -171,9 +158,13 @@ PropertyDetails TransitionArray::GetTargetDetails(Name* name, Map* target) { } -void TransitionArray::Set(int transition_number, Name* key, WeakCell* target) { - set(ToKeyIndex(transition_number), key); - set(ToTargetIndex(transition_number), target); +void TransitionArray::NoIncrementalWriteBarrierSet(int transition_number, + Name* key, + Map* target) { + FixedArray::NoIncrementalWriteBarrierSet( + this, ToKeyIndex(transition_number), key); + FixedArray::NoIncrementalWriteBarrierSet( + this, ToTargetIndex(transition_number), target); } diff --git a/src/transitions.cc b/src/transitions.cc index d646eb80c..c39534bbb 100644 --- a/src/transitions.cc +++ b/src/transitions.cc @@ -17,11 +17,11 @@ void TransitionArray::Insert(Handle map, Handle name, Handle target, SimpleTransitionFlag flag) { Isolate* isolate = map->GetIsolate(); target->SetBackPointer(*map); - Handle cell = Map::WeakCellForMap(target); // If the map doesn't have any transitions at all yet, install the new one. if (CanStoreSimpleTransition(map->raw_transitions())) { if (flag == SIMPLE_PROPERTY_TRANSITION) { + Handle cell = Map::WeakCellForMap(target); ReplaceTransitions(map, *cell); return; } @@ -42,6 +42,7 @@ void TransitionArray::Insert(Handle map, Handle name, if (flag == SIMPLE_PROPERTY_TRANSITION && key->Equals(*name) && old_details.kind() == new_details.kind() && old_details.attributes() == new_details.attributes()) { + Handle cell = Map::WeakCellForMap(target); ReplaceTransitions(map, *cell); return; } @@ -50,8 +51,8 @@ void TransitionArray::Insert(Handle map, Handle name, // Re-read existing data; the allocation might have caused it to be cleared. if (IsSimpleTransition(map->raw_transitions())) { old_target = GetSimpleTransition(map->raw_transitions()); - result->Set(0, GetSimpleTransitionKey(old_target), - GetSimpleTransitionCell(map->raw_transitions())); + result->NoIncrementalWriteBarrierSet( + 0, GetSimpleTransitionKey(old_target), old_target); } else { result->SetNumberOfTransitions(0); } @@ -82,7 +83,7 @@ void TransitionArray::Insert(Handle map, Handle name, &insertion_index); // If an existing entry was found, overwrite it and return. if (index != kNotFound) { - array->SetTargetCell(index, *cell); + array->SetTarget(index, *target); return; } @@ -95,10 +96,10 @@ void TransitionArray::Insert(Handle map, Handle name, array->SetNumberOfTransitions(new_nof); for (index = number_of_transitions; index > insertion_index; --index) { array->SetKey(index, array->GetKey(index - 1)); - array->SetTargetCell(index, array->GetTargetCell(index - 1)); + array->SetTarget(index, array->GetTarget(index - 1)); } array->SetKey(index, *name); - array->SetTargetCell(index, *cell); + array->SetTarget(index, *target); SLOW_DCHECK(array->IsSortedNoDuplicates()); return; } @@ -144,11 +145,11 @@ void TransitionArray::Insert(Handle map, Handle name, DCHECK_NE(kNotFound, insertion_index); for (int i = 0; i < insertion_index; ++i) { - result->CopyFrom(array, i, i); + result->NoIncrementalWriteBarrierCopyFrom(array, i, i); } - result->Set(insertion_index, *name, *cell); + result->NoIncrementalWriteBarrierSet(insertion_index, *name, *target); for (int i = insertion_index; i < number_of_transitions; ++i) { - result->CopyFrom(array, i, i + 1); + result->NoIncrementalWriteBarrierCopyFrom(array, i, i + 1); } SLOW_DCHECK(result->IsSortedNoDuplicates()); @@ -348,10 +349,12 @@ Handle TransitionArray::Allocate(Isolate* isolate, } -void TransitionArray::CopyFrom(TransitionArray* origin, int origin_transition, - int target_transition) { - Set(target_transition, origin->GetKey(origin_transition), - origin->GetTargetCell(origin_transition)); +void TransitionArray::NoIncrementalWriteBarrierCopyFrom(TransitionArray* origin, + int origin_transition, + int target_transition) { + NoIncrementalWriteBarrierSet(target_transition, + origin->GetKey(origin_transition), + origin->GetTarget(origin_transition)); } @@ -419,9 +422,9 @@ void TransitionArray::EnsureHasFullTransitionArray(Handle map) { result->Shrink(ToKeyIndex(0)); result->SetNumberOfTransitions(0); } else if (nof == 1) { - WeakCell* target_cell = GetSimpleTransitionCell(raw_transitions); - Name* key = GetSimpleTransitionKey(Map::cast(target_cell->value())); - result->Set(0, key, target_cell); + Map* target = GetSimpleTransition(raw_transitions); + Name* key = GetSimpleTransitionKey(target); + result->NoIncrementalWriteBarrierSet(0, key, target); } ReplaceTransitions(map, *result); } diff --git a/src/transitions.h b/src/transitions.h index 9e1427975..b0aab9502 100644 --- a/src/transitions.h +++ b/src/transitions.h @@ -71,11 +71,6 @@ class TransitionArray: public FixedArray { DCHECK(raw_transition->IsWeakCell()); return Map::cast(WeakCell::cast(raw_transition)->value()); } - static inline WeakCell* GetSimpleTransitionCell(Object* raw_transition) { - DCHECK(IsSimpleTransition(raw_transition)); - DCHECK(raw_transition->IsWeakCell()); - return WeakCell::cast(raw_transition); - } static inline bool IsFullTransitionArray(Object* raw_transitions) { return raw_transitions->IsTransitionArray(); } @@ -140,7 +135,6 @@ class TransitionArray: public FixedArray { inline Name* GetKey(int transition_number); inline void SetKey(int transition_number, Name* value); inline Object** GetKeySlot(int transition_number); - inline Object** GetTargetSlot(int transition_number); int GetSortedKeyIndex(int transition_number) { return transition_number; } Name* GetSortedKey(int transition_number) { @@ -149,9 +143,7 @@ class TransitionArray: public FixedArray { static inline Map* GetTarget(Object* raw_transitions, int transition_number); inline Map* GetTarget(int transition_number); - - inline WeakCell* GetTargetCell(int transition_number); - inline void SetTargetCell(int transition_number, WeakCell* target); + inline void SetTarget(int transition_number, Map* target); static inline PropertyDetails GetTargetDetails(Name* name, Map* target); @@ -291,11 +283,14 @@ class TransitionArray: public FixedArray { PropertyKind kind2, PropertyAttributes attributes2); - inline void Set(int transition_number, Name* key, WeakCell* target_cell); + inline void NoIncrementalWriteBarrierSet(int transition_number, + Name* key, + Map* target); // Copy a single transition from the origin array. - inline void CopyFrom(TransitionArray* origin, int origin_transition, - int target_transition); + inline void NoIncrementalWriteBarrierCopyFrom(TransitionArray* origin, + int origin_transition, + int target_transition); #ifdef DEBUG static void CheckNewTransitionsAreConsistent(Handle map, -- 2.34.1