Skip to content

Commit

Permalink
deps: V8: cherry-pick 548f6c81d424
Browse files Browse the repository at this point in the history
Original commit message:

    [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}

Refs: v8/v8@548f6c8

PR-URL: nodejs#33484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
dominykas committed May 25, 2020
1 parent 5c0232a commit 2d9540e
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 22 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.37',
'v8_embedder_string': '-node.38',

##### V8 defaults for Node.js #####

Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/json/json-parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,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_);
}
}
Expand Down
11 changes: 10 additions & 1 deletion deps/v8/src/objects/map-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,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,
Expand Down Expand Up @@ -697,7 +703,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
Expand Down
32 changes: 13 additions & 19 deletions deps/v8/src/objects/map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ Map Map::FindRootMap(Isolate* isolate) const {
// Initial map always owns descriptors and doesn't have unused entries
// in the descriptor array.
DCHECK(result.owns_descriptors());
DCHECK_EQ(result.NumberOfOwnDescriptors(),
DCHECK_LE(result.NumberOfOwnDescriptors(),
result.instance_descriptors().number_of_descriptors());
return result;
}
Expand Down Expand Up @@ -1192,7 +1192,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);
Expand Down Expand Up @@ -1325,7 +1325,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;
Expand Down Expand Up @@ -1687,15 +1687,15 @@ 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 keep the ownership, there must be no
// descriptors in the descriptors array that do not belong to the map.
DCHECK(parent->owns_descriptors());
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));
}
Expand All @@ -1722,7 +1722,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);
Expand All @@ -1733,19 +1735,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,
Expand Down Expand Up @@ -1926,7 +1920,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()) {
Expand Down Expand Up @@ -2126,7 +2120,7 @@ Handle<Map> Map::TransitionToDataProperty(Isolate* isolate, Handle<Map> map,
StoreOrigin store_origin) {
RuntimeCallTimerScope stats_scope(
isolate, *map,
map->is_prototype_map()
map->IsDetached(isolate)
? RuntimeCallCounterId::kPrototypeMap_TransitionToDataProperty
: RuntimeCallCounterId::kMap_TransitionToDataProperty);

Expand Down Expand Up @@ -2238,7 +2232,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);

Expand Down
5 changes: 5 additions & 0 deletions deps/v8/src/objects/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,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);
Expand Down

0 comments on commit 2d9540e

Please sign in to comment.