Title: [201562] trunk/Source/_javascript_Core
- Revision
- 201562
- Author
- [email protected]
- Date
- 2016-06-01 12:32:34 -0700 (Wed, 01 Jun 2016)
Log Message
Dictionary property access should be fast
https://bugs.webkit.org/show_bug.cgi?id=158250
Reviewed by Keith Miller.
We have some remnant code that unnecessarily takes a slow path for
dictionaries. This caused the Dromaeo regression in r201436. Let's fix
that.
* jit/Repatch.cpp:
(JSC::tryCacheGetByID): Attempt to flatten a dictionary if necessary, but
not too much. This is our idiom in other places.
(JSC::tryCachePutByID): See tryCacheGetByID.
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::setupGetByIdPrototypeCache): See tryCacheGetByID.
* runtime/JSObject.cpp:
(JSC::JSObject::fillGetterPropertySlot):
* runtime/JSObject.h:
(JSC::JSObject::fillCustomGetterPropertySlot): The rules for caching a
getter are the same as the rules for caching anything else: We're
allowed to cache even in dictionaries, as long as they're cacheable
dictionaries. Any transition that would change to/from getter/setter
or change other attributes requires a structure transition.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201561 => 201562)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-01 18:50:09 UTC (rev 201561)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-01 19:32:34 UTC (rev 201562)
@@ -1,3 +1,32 @@
+2016-05-31 Geoffrey Garen <[email protected]>
+
+ Dictionary property access should be fast
+ https://bugs.webkit.org/show_bug.cgi?id=158250
+
+ Reviewed by Keith Miller.
+
+ We have some remnant code that unnecessarily takes a slow path for
+ dictionaries. This caused the Dromaeo regression in r201436. Let's fix
+ that.
+
+ * jit/Repatch.cpp:
+ (JSC::tryCacheGetByID): Attempt to flatten a dictionary if necessary, but
+ not too much. This is our idiom in other places.
+
+ (JSC::tryCachePutByID): See tryCacheGetByID.
+
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::setupGetByIdPrototypeCache): See tryCacheGetByID.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::fillGetterPropertySlot):
+ * runtime/JSObject.h:
+ (JSC::JSObject::fillCustomGetterPropertySlot): The rules for caching a
+ getter are the same as the rules for caching anything else: We're
+ allowed to cache even in dictionaries, as long as they're cacheable
+ dictionaries. Any transition that would change to/from getter/setter
+ or change other attributes requires a structure transition.
+
2016-05-31 Yusuke Suzuki <[email protected]>
[JSC] Drop "replace" from JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL_NOT_IMPLEMENTED_YET
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (201561 => 201562)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2016-06-01 18:50:09 UTC (rev 201561)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2016-06-01 19:32:34 UTC (rev 201562)
@@ -297,8 +297,14 @@
PropertyOffset offset = slot.isUnset() ? invalidOffset : slot.cachedOffset();
if (slot.isUnset() || slot.slotBase() != baseValue) {
- if (structure->typeInfo().prohibitsPropertyCaching() || structure->isDictionary())
+ if (structure->typeInfo().prohibitsPropertyCaching())
return GiveUpOnCache;
+
+ if (structure->isDictionary()) {
+ if (structure->hasBeenFlattenedBefore())
+ return GiveUpOnCache;
+ structure->flattenDictionaryStructure(vm, jsCast<JSObject*>(baseCell));
+ }
if (slot.isUnset() && structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
return GiveUpOnCache;
@@ -445,9 +451,15 @@
} else {
ASSERT(slot.type() == PutPropertySlot::NewProperty);
- if (!structure->isObject() || structure->isDictionary())
+ if (!structure->isObject())
return GiveUpOnCache;
+ if (structure->isDictionary()) {
+ if (structure->hasBeenFlattenedBefore())
+ return GiveUpOnCache;
+ structure->flattenDictionaryStructure(vm, jsCast<JSObject*>(baseValue));
+ }
+
PropertyOffset offset;
Structure* newStructure =
Structure::addPropertyTransitionToExistingStructureConcurrently(
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (201561 => 201562)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-06-01 18:50:09 UTC (rev 201561)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-06-01 19:32:34 UTC (rev 201562)
@@ -2001,10 +2001,13 @@
NEVER_INLINE void JSObject::fillGetterPropertySlot(PropertySlot& slot, JSValue getterSetter, unsigned attributes, PropertyOffset offset)
{
- if (structure()->isDictionary()) {
+ if (structure()->isUncacheableDictionary()) {
slot.setGetterSlot(this, attributes, jsCast<GetterSetter*>(getterSetter));
return;
}
+
+ // This access is cacheable because Structure requires an attributeChangedTransition
+ // if this property stops being an accessor.
slot.setCacheableGetterSlot(this, attributes, jsCast<GetterSetter*>(getterSetter), offset);
}
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (201561 => 201562)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2016-06-01 18:50:09 UTC (rev 201561)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2016-06-01 19:32:34 UTC (rev 201562)
@@ -1225,10 +1225,13 @@
ALWAYS_INLINE void JSObject::fillCustomGetterPropertySlot(PropertySlot& slot, JSValue customGetterSetter, unsigned attributes, Structure& structure)
{
- if (structure.isDictionary()) {
+ if (structure.isUncacheableDictionary()) {
slot.setCustom(this, attributes, jsCast<CustomGetterSetter*>(customGetterSetter)->getter());
return;
}
+
+ // This access is cacheable because Structure requires an attributeChangedTransition
+ // if this property stops being an accessor.
slot.setCacheableCustom(this, attributes, jsCast<CustomGetterSetter*>(customGetterSetter)->getter());
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
