Diff
Modified: trunk/JSTests/ChangeLog (265641 => 265642)
--- trunk/JSTests/ChangeLog 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/JSTests/ChangeLog 2020-08-14 04:11:09 UTC (rev 265642)
@@ -1,3 +1,12 @@
+2020-08-13 Alexey Shvayka <[email protected]>
+
+ Cache Structure::attributeChangeTransition()
+ https://bugs.webkit.org/show_bug.cgi?id=214890
+
+ Reviewed by Yusuke Suzuki.
+
+ * microbenchmarks/redefine-property-previous-attributes.js: Added.
+
2020-08-12 Saam Barati <[email protected]>
Inline cache Replace and Setters on PureForwardingProxy
Added: trunk/JSTests/microbenchmarks/redefine-property-previous-attributes.js (0 => 265642)
--- trunk/JSTests/microbenchmarks/redefine-property-previous-attributes.js (rev 0)
+++ trunk/JSTests/microbenchmarks/redefine-property-previous-attributes.js 2020-08-14 04:11:09 UTC (rev 265642)
@@ -0,0 +1,13 @@
+var desc1 = {value: 1, writable: true, enumerable: false, configurable: true};
+var desc2 = {value: 1, writable: false, enumerable: true, configurable: true};
+
+for (var i = 0; i < 1e5; ++i) {
+ var obj = {foo: 1};
+
+ Object.defineProperty(obj, "foo", desc1);
+ Object.defineProperty(obj, "foo", desc2);
+ Object.defineProperty(obj, "foo", desc1);
+ Object.defineProperty(obj, "foo", desc2);
+ Object.defineProperty(obj, "foo", desc1);
+ Object.defineProperty(obj, "foo", desc2);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (265641 => 265642)
--- trunk/Source/_javascript_Core/ChangeLog 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-08-14 04:11:09 UTC (rev 265642)
@@ -1,5 +1,34 @@
2020-08-13 Alexey Shvayka <[email protected]>
+ Cache Structure::attributeChangeTransition()
+ https://bugs.webkit.org/show_bug.cgi?id=214890
+
+ Reviewed by Yusuke Suzuki.
+
+ With this change, a non-dictionary structure adds attribute-change transitions
+ to transition table, making redefinition to previous atttributes a fast path.
+
+ After too many transitions, the structure becomes a dictionary, firing the
+ transition watchpoint. Attribute-change transitions pin their property tables,
+ preventing forEachPropertyConcurrently() traversal.
+
+ This patch advances provided microbenchmark by ~90% and progresses
+ Speedometer2/EmberJS-Debug-TodoMVC by ~12% (~5% over r264573).
+
+ No behavior change.
+
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::getRegExpPrototypeProperty):
+ * runtime/JSObjectInlines.h:
+ (JSC::JSObject::putDirectInternal):
+ * runtime/Structure.cpp:
+ (JSC::Structure::materializePropertyTable):
+ (JSC::Structure::removeNewPropertyTransition):
+ (JSC::Structure::attributeChangeTransition):
+ * runtime/Structure.h:
+
+2020-08-13 Alexey Shvayka <[email protected]>
+
Rework StructureTransitionTable::Hash::Key encoding
https://bugs.webkit.org/show_bug.cgi?id=215483
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (265641 => 265642)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2020-08-14 04:11:09 UTC (rev 265642)
@@ -1726,8 +1726,7 @@
bool Graph::getRegExpPrototypeProperty(JSObject* regExpPrototype, Structure* regExpPrototypeStructure, UniquedStringImpl* uid, JSValue& returnJSValue)
{
- unsigned attributesUnused;
- PropertyOffset offset = regExpPrototypeStructure->getConcurrently(uid, attributesUnused);
+ PropertyOffset offset = regExpPrototypeStructure->getConcurrently(uid);
if (!isValidOffset(offset))
return false;
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (265641 => 265642)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2020-08-14 04:11:09 UTC (rev 265642)
@@ -364,6 +364,10 @@
return true;
}
+ // We want the structure transition watchpoint to fire after this object has switched structure.
+ // This allows adaptive watchpoints to observe if the new structure is the one we want.
+ DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure);
+
unsigned currentAttributes;
offset = structure->get(vm, propertyName, currentAttributes);
if (offset != invalidOffset) {
@@ -376,7 +380,7 @@
// FIXME: Check attributes against PropertyAttribute::CustomAccessorOrValue. Changing GetterSetter should work w/o transition.
// https://bugs.webkit.org/show_bug.cgi?id=214342
if (mode == PutModeDefineOwnProperty && (attributes != currentAttributes || (attributes & PropertyAttribute::AccessorOrCustomAccessorOrValue)))
- setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
+ setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes, &deferredWatchpointFire));
else
slot.setExistingProperty(this, offset);
@@ -385,11 +389,6 @@
if ((mode == PutModePut) && !isStructureExtensible(vm))
return false;
-
- // We want the structure transition watchpoint to fire after this object has switched
- // structure. This allows adaptive watchpoints to observe if the new structure is the one
- // we want.
- DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure);
newStructure = Structure::addNewPropertyTransition(
vm, structure, propertyName, attributes, offset, slot.context(), &deferredWatchpointFire);
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (265641 => 265642)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2020-08-14 04:11:09 UTC (rev 265642)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2008-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2020 Alexey Shvayka <[email protected]>.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -444,6 +445,7 @@
table->addDeletedOffset(structure->transitionOffset());
continue;
}
+ ASSERT(structure->isPropertyAdditionTransition());
PropertyMapEntry entry(structure->m_transitionPropertyName.get(), structure->transitionOffset(), structure->transitionPropertyAttributes());
auto nextOffset = table->nextOffset(structure->inlineCapacity());
ASSERT_UNUSED(nextOffset, nextOffset == structure->transitionOffset());
@@ -643,11 +645,7 @@
ASSERT(!Structure::removePropertyTransitionFromExistingStructure(structure, propertyName, offset));
ASSERT(structure->getConcurrently(propertyName.uid()) != invalidOffset);
- int transitionCount = 0;
- for (auto* s = structure; s && transitionCount <= s_maxTransitionLength; s = s->previousID())
- ++transitionCount;
-
- if (transitionCount > s_maxTransitionLength) {
+ if (structure->transitionCountHasOverflowed()) {
ASSERT(!isCopyOnWrite(structure->indexingMode()));
Structure* transition = toUncacheableDictionaryTransition(vm, structure, deferred);
ASSERT(structure != transition);
@@ -706,30 +704,79 @@
return transition;
}
-Structure* Structure::attributeChangeTransition(VM& vm, Structure* structure, PropertyName propertyName, unsigned attributes)
+Structure* Structure::attributeChangeTransition(VM& vm, Structure* structure, PropertyName propertyName, unsigned attributes, DeferredStructureTransitionWatchpointFire* deferred)
{
- if (!structure->isUncacheableDictionary()) {
- Structure* transition = create(vm, structure);
+ auto setEntryAttributes = [&](Structure* structure) {
+ PropertyMapEntry* entry = structure->ensurePropertyTable(vm)->get(propertyName.uid());
+ ASSERT(entry);
+ entry->attributes = attributes;
+ };
- PropertyTable* table = structure->copyPropertyTableForPinning(vm);
- transition->pin(holdLock(transition->m_lock), vm, table);
- transition->setMaxOffset(vm, structure->maxOffset());
-
- structure = transition;
+ auto setStructureFlags = [&](Structure* structure) {
+ if (attributes & PropertyAttribute::ReadOnly)
+ structure->setContainsReadOnlyProperties();
+ if (attributes & PropertyAttribute::DontEnum)
+ structure->setIsQuickPropertyAccessAllowedForEnumeration(false);
+ };
+
+ if (structure->isUncacheableDictionary()) {
+ setEntryAttributes(structure);
+ setStructureFlags(structure);
+ structure->checkOffsetConsistency();
+ return structure;
}
- PropertyMapEntry* entry = structure->ensurePropertyTable(vm)->get(propertyName.uid());
+ if (!structure->isDictionary()) {
+ if (Structure* existingTransition = structure->m_transitionTable.get(propertyName.uid(), attributes, TransitionKind::PropertyAttributeChange)) {
+ validateOffset(existingTransition->transitionOffset(), existingTransition->inlineCapacity());
+ existingTransition->checkOffsetConsistency();
+ return existingTransition;
+ }
+
+ if (structure->transitionCountHasOverflowed()) {
+ ASSERT(!isCopyOnWrite(structure->indexingMode()));
+ Structure* transition = toUncacheableDictionaryTransition(vm, structure, deferred);
+ ASSERT(structure != transition);
+ setEntryAttributes(transition);
+ setStructureFlags(transition);
+ return transition;
+ }
+ }
+
+ Structure* transition = create(vm, structure);
+
+ {
+ ConcurrentJSLocker locker(transition->m_lock);
+ transition->setProtectPropertyTableWhileTransitioning(true);
+ }
+
+ PropertyTable* table = structure->copyPropertyTableForPinning(vm);
+ if (structure->isDictionary())
+ transition->pin(holdLock(transition->m_lock), vm, table);
+ else
+ transition->pinForCaching(holdLock(transition->m_lock), vm, table);
+ transition->m_transitionPropertyName = propertyName.uid();
+ transition->setTransitionPropertyAttributes(attributes);
+ transition->setTransitionKind(TransitionKind::PropertyAttributeChange);
+ transition->setMaxOffset(vm, structure->maxOffset());
+
+ PropertyMapEntry* entry = table->get(propertyName.uid());
ASSERT(entry);
entry->attributes = attributes;
+ transition->setTransitionOffset(vm, entry->offset);
+ setStructureFlags(transition);
- if (attributes & PropertyAttribute::ReadOnly)
- structure->setContainsReadOnlyProperties();
+ WTF::storeStoreFence();
+ transition->setProtectPropertyTableWhileTransitioning(false);
- if (attributes & PropertyAttribute::DontEnum)
- structure->setIsQuickPropertyAccessAllowedForEnumeration(false);
-
+ checkOffset(transition->transitionOffset(), transition->inlineCapacity());
+ if (!structure->isDictionary()) {
+ GCSafeConcurrentJSLocker locker(structure->m_lock, vm.heap);
+ structure->m_transitionTable.add(vm, transition);
+ }
+ transition->checkOffsetConsistency();
structure->checkOffsetConsistency();
- return structure;
+ return transition;
}
Structure* Structure::toDictionaryTransition(VM& vm, Structure* structure, DictionaryKind kind, DeferredStructureTransitionWatchpointFire* deferred)
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (265641 => 265642)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2020-08-14 02:01:56 UTC (rev 265641)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2020-08-14 04:11:09 UTC (rev 265642)
@@ -197,7 +197,7 @@
static Structure* removePropertyTransitionFromExistingStructure(Structure*, PropertyName, PropertyOffset&);
static Structure* removePropertyTransitionFromExistingStructureConcurrently(Structure*, PropertyName, PropertyOffset&);
static Structure* changePrototypeTransition(VM&, Structure*, JSValue prototype, DeferredStructureTransitionWatchpointFire&);
- JS_EXPORT_PRIVATE static Structure* attributeChangeTransition(VM&, Structure*, PropertyName, unsigned attributes);
+ JS_EXPORT_PRIVATE static Structure* attributeChangeTransition(VM&, Structure*, PropertyName, unsigned attributes, DeferredStructureTransitionWatchpointFire* = nullptr);
JS_EXPORT_PRIVATE static Structure* toCacheableDictionaryTransition(VM&, Structure*, DeferredStructureTransitionWatchpointFire* = nullptr);
static Structure* toUncacheableDictionaryTransition(VM&, Structure*, DeferredStructureTransitionWatchpointFire* = nullptr);
JS_EXPORT_PRIVATE static Structure* sealTransition(VM&, Structure*);
@@ -796,6 +796,17 @@
return numberOfSlotsForMaxOffset(maxOffset(), m_inlineCapacity);
}
+ ALWAYS_INLINE bool transitionCountHasOverflowed() const
+ {
+ int transitionCount = 0;
+ for (auto* structure = this; structure; structure = structure->previousID()) {
+ if (++transitionCount > s_maxTransitionLength)
+ return true;
+ }
+
+ return false;
+ }
+
bool isValid(JSGlobalObject*, StructureChain* cachedPrototypeChain, JSObject* base) const;
// You have to hold the structure lock to do these.
