[MERGE #5032 @sethbrenith] Share type handler for strict-mode arguments object
Merge pull request #5032 from sethbrenith:user/sethb/strict-args
The Preact test in Speedometer is spending around 3% of its script time in LoadHeapArguments, which is currently slower for strict mode than sloppy mode because we don't share types. This change updates CreateHeapArguments to use a single shared type for all strict-mode `arguments` objects, which improves the following microbenchmark by about 34%. Overall Preact improvement is much more modest; I'd guess 0.3% based on trace data comparison.
```javascript
function arg() {
"use strict";
return Array.prototype.slice.call(arguments)[0];
}
const start = Date.now();
for (let i = 0; i < 1e6; ++i) {
arg(i);
}
print(Date.now() - start);
```
diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
index 2207fbd..c278d5b 100644
--- a/lib/Runtime/Language/JavascriptOperators.cpp
+++ b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -6910,6 +6910,10 @@
JavascriptLibrary *library = scriptContext->GetLibrary();
HeapArgumentsObject *argsObj = library->CreateHeapArguments(frameObj, formalsCount, !!funcCallee->IsStrictMode());
+#if DBG
+ DynamicTypeHandler* typeHandler = argsObj->GetTypeHandler();
+#endif
+
//
// Set the number of arguments of Arguments Object
//
@@ -6929,6 +6933,8 @@
StackScriptFunction::EnsureBoxed(BOX_PARAM(funcCallee, nullptr, _u("callee"))), scriptContext);
}
+ AssertMsg(argsObj->GetTypeHandler() == typeHandler || scriptContext->IsScriptContextInDebugMode(), "type handler should not transition because we initialized it correctly");
+
return argsObj;
}
diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp
index 333ed21..5c630b8 100644
--- a/lib/Runtime/Library/JavascriptLibrary.cpp
+++ b/lib/Runtime/Library/JavascriptLibrary.cpp
@@ -398,6 +398,45 @@
heapArgumentsType = DynamicType::New(scriptContext, TypeIds_Arguments, objectPrototype, nullptr,
SimpleDictionaryTypeHandler::New(scriptContext, HeapArgumentsPropertyDescriptors, _countof(HeapArgumentsPropertyDescriptors), 0, 0, true, true), true, true);
+ TypePath *const strictHeapArgumentsTypePath = TypePath::New(recycler);
+ strictHeapArgumentsTypePath->Add(BuiltInPropertyRecords::callee);
+ strictHeapArgumentsTypePath->Add<true /*isSetter*/>(BuiltInPropertyRecords::callee);
+ strictHeapArgumentsTypePath->Add(BuiltInPropertyRecords::length);
+ strictHeapArgumentsTypePath->Add(BuiltInPropertyRecords::_symbolIterator);
+ ObjectSlotAttributes *strictHeapArgumentsAttributes = RecyclerNewArrayLeaf(recycler, ObjectSlotAttributes, strictHeapArgumentsTypePath->GetPathSize());
+ strictHeapArgumentsAttributes[0] = (ObjectSlotAttributes)(ObjectSlotAttr_Writable | ObjectSlotAttr_Accessor);
+ strictHeapArgumentsAttributes[1] = ObjectSlotAttr_Setter;
+ strictHeapArgumentsAttributes[2] = (ObjectSlotAttributes)PropertyBuiltInMethodDefaults;
+ strictHeapArgumentsAttributes[3] = (ObjectSlotAttributes)PropertyBuiltInMethodDefaults;
+ for (int i = 4; i < strictHeapArgumentsTypePath->GetPathSize(); ++i)
+ {
+ strictHeapArgumentsAttributes[i] = ObjectSlotAttr_Default;
+ }
+ PathTypeSetterSlotIndex * strictHeapArgumentsSetters = RecyclerNewArrayLeaf(recycler, PathTypeSetterSlotIndex, strictHeapArgumentsTypePath->GetPathSize());
+ strictHeapArgumentsSetters[0] = 1;
+ for (int i = 1; i < strictHeapArgumentsTypePath->GetPathSize(); ++i)
+ {
+ strictHeapArgumentsSetters[i] = NoSetterSlot;
+ }
+ strictHeapArgumentsType = DynamicType::New(
+ scriptContext,
+ TypeIds_Arguments,
+ objectPrototype,
+ nullptr,
+ PathTypeHandlerWithAttr::New(
+ scriptContext,
+ strictHeapArgumentsTypePath,
+ strictHeapArgumentsAttributes,
+ strictHeapArgumentsSetters,
+ 1 /*setterCount*/,
+ strictHeapArgumentsTypePath->GetPathLength(),
+ strictHeapArgumentsTypePath->GetPathLength() /*inlineSlotCapacity*/,
+ sizeof(HeapArgumentsObject) /*offsetOfInlineSlots*/,
+ true /*isLocked*/,
+ true /*isShared*/),
+ true /*isLocked*/,
+ true /*isShared*/);
+
#define INIT_SIMPLE_TYPE(field, typeId, prototype) \
field = DynamicType::New(scriptContext, typeId, prototype, nullptr, \
PathTypeHandlerNoAttr::New(scriptContext, this->GetRootPath(), 0, 0, 0, true, true), true, true)
@@ -5517,26 +5556,15 @@
HeapArgumentsObject* JavascriptLibrary::CreateHeapArguments(Var frameObj, uint32 formalCount, bool isStrictMode)
{
AssertMsg(heapArgumentsType, "Where's heapArgumentsType?");
+ Assert(strictHeapArgumentsType);
Recycler *recycler = this->GetRecycler();
EnsureArrayPrototypeValuesFunction(); //InitializeArrayPrototype can be delay loaded, which could prevent us from access to array.prototype.values
- DynamicType * argumentsType = nullptr;
+ DynamicType * argumentsType = isStrictMode ? strictHeapArgumentsType : heapArgumentsType;
- if (isStrictMode)
- {
- //TODO: Make DictionaryTypeHandler shareable - So that Arguments' type can be cached on the javascriptLibrary.
- DictionaryTypeHandler * dictTypeHandlerForArgumentsInStrictMode = DictionaryTypeHandler::CreateTypeHandlerForArgumentsInStrictMode(recycler, scriptContext);
- argumentsType = DynamicType::New(scriptContext, TypeIds_Arguments, objectPrototype, nullptr,
- dictTypeHandlerForArgumentsInStrictMode, false, false);
- }
- else
- {
- argumentsType = heapArgumentsType;
- }
-
- return RecyclerNew(recycler, HeapArgumentsObject, recycler,
+ return RecyclerNewPlusZ(recycler, argumentsType->GetTypeHandler()->GetInlineSlotCapacity() * sizeof(Var), HeapArgumentsObject, recycler,
frameObj != GetNull() ? static_cast<ActivationObject*>(frameObj) : nullptr,
formalCount, argumentsType);
}
diff --git a/lib/Runtime/Library/JavascriptLibrary.h b/lib/Runtime/Library/JavascriptLibrary.h
index dba6d89..7078d61 100644
--- a/lib/Runtime/Library/JavascriptLibrary.h
+++ b/lib/Runtime/Library/JavascriptLibrary.h
@@ -262,6 +262,7 @@
Field(DynamicType *) generatorConstructorPrototypeObjectType;
Field(DynamicType *) constructorPrototypeObjectType;
Field(DynamicType *) heapArgumentsType;
+ Field(DynamicType *) strictHeapArgumentsType;
Field(DynamicType *) activationObjectType;
Field(DynamicType *) arrayType;
Field(DynamicType *) nativeIntArrayType;
diff --git a/lib/Runtime/Types/DictionaryTypeHandler.cpp b/lib/Runtime/Types/DictionaryTypeHandler.cpp
index e91c102..38a3784 100644
--- a/lib/Runtime/Types/DictionaryTypeHandler.cpp
+++ b/lib/Runtime/Types/DictionaryTypeHandler.cpp
@@ -13,18 +13,6 @@
}
template <typename T>
- DictionaryTypeHandlerBase<T>* DictionaryTypeHandlerBase<T>::CreateTypeHandlerForArgumentsInStrictMode(Recycler * recycler, ScriptContext * scriptContext)
- {
- DictionaryTypeHandlerBase<T> * dictTypeHandler = New(recycler, 8, 0, 0);
-
- dictTypeHandler->Add(scriptContext->GetPropertyName(Js::PropertyIds::callee), PropertyWritable, scriptContext);
- dictTypeHandler->Add(scriptContext->GetPropertyName(Js::PropertyIds::length), PropertyBuiltInMethodDefaults, scriptContext);
- dictTypeHandler->Add(scriptContext->GetPropertyName(Js::PropertyIds::_symbolIterator), PropertyBuiltInMethodDefaults, scriptContext);
-
- return dictTypeHandler;
- }
-
- template <typename T>
DictionaryTypeHandlerBase<T>::DictionaryTypeHandlerBase(Recycler* recycler) :
DynamicTypeHandler(1),
nextPropertyIndex(0)
diff --git a/lib/Runtime/Types/DictionaryTypeHandler.h b/lib/Runtime/Types/DictionaryTypeHandler.h
index 92a08ba..94cf3d5 100644
--- a/lib/Runtime/Types/DictionaryTypeHandler.h
+++ b/lib/Runtime/Types/DictionaryTypeHandler.h
@@ -62,8 +62,6 @@
// Create a new type handler for a future DynamicObject. This is for public usage. "initialCapacity" indicates desired slotCapacity, subject to alignment round up.
static DictionaryTypeHandlerBase* New(Recycler * recycler, int initialCapacity, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots);
- static DictionaryTypeHandlerBase* CreateTypeHandlerForArgumentsInStrictMode(Recycler * recycler, ScriptContext * scriptContext);
-
BOOL IsBigDictionaryTypeHandler();
virtual BOOL IsLockable() const override { return false; }
diff --git a/lib/Runtime/Types/TypePath.h b/lib/Runtime/Types/TypePath.h
index 004da92..12283bc 100644
--- a/lib/Runtime/Types/TypePath.h
+++ b/lib/Runtime/Types/TypePath.h
@@ -245,13 +245,14 @@
return nullptr;
}
+ template<bool isSetter = false>
int Add(const PropertyRecord * propertyRecord)
{
#ifdef SUPPORT_FIXED_FIELDS_ON_PATH_TYPES
Assert(this->GetPathLength() == this->GetMaxInitializedLength());
this->GetData()->maxInitializedLength++;
#endif
- return AddInternal<true>(propertyRecord);
+ return AddInternal<!isSetter>(propertyRecord);
}
uint8 GetPathLength() { return this->GetData()->pathLength; }
