Skip to content

Commit 5c9b7ed

Browse files
committed
[MERGE #4818 @pleath] Use PathTypeHandler's for function objects with non-default properties/attributes
Merge pull request #4818 from pleath:funcpathtypes Move from creating new non-shared SimpleTypeHandler's when a new property is added to a SimpleTypeHandler, or an attribute is changed, to creating PathTypeHandler's. Build a new root and evolve from the root to the current state of the SimpleTypeHandler, then replace the type's existing SimpleTypeHandler with the new PathTypeHandler. From there, we can evolve to the state required by the new property/attribute, and future instances of the same function that make the same type transition will get the same type. Also fix up some shared SimpleTypeHandler's in the library that didn't represent the desired state of their library objects and were forcing type transitions during initialization.
2 parents 5a11b38 + 978c171 commit 5c9b7ed

File tree

10 files changed

+186
-73
lines changed

10 files changed

+186
-73
lines changed

lib/Runtime/Language/CacheOperators.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,16 @@ namespace Js
234234
DynamicTypeHandler* oldTypeHandler = oldType->GetTypeHandler();
235235
DynamicTypeHandler* newTypeHandler = newType->GetTypeHandler();
236236

237+
#if ENABLE_FIXED_FIELDS
237238
// the newType is a path-type so the old one should be too:
238239
Assert(oldTypeHandler->IsPathTypeHandler());
240+
#else
241+
// This may be the transition from deferred type handler to path type handler. Don't try to cache now.
242+
if (!oldTypeHandler->IsPathTypeHandler())
243+
{
244+
return;
245+
}
246+
#endif
239247

240248
int oldCapacity = oldTypeHandler->GetSlotCapacity();
241249
int newCapacity = newTypeHandler->GetSlotCapacity();

lib/Runtime/Library/JavascriptLibrary.cpp

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ namespace Js
3232
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable)
3333
};
3434

35+
SimplePropertyDescriptor const JavascriptLibrary::SharedIdMappedFunctionPropertyDescriptors[2] =
36+
{
37+
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype), PropertyNone),
38+
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable)
39+
};
40+
3541
SimplePropertyDescriptor const JavascriptLibrary::FunctionWithLengthAndNameTypeDescriptors[2] =
3642
{
3743
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length), PropertyConfigurable),
@@ -40,14 +46,15 @@ namespace Js
4046

4147
SimplePropertyDescriptor const JavascriptLibrary::ModuleNamespaceTypeDescriptors[1] =
4248
{
43-
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::_symbolToStringTag), PropertyConfigurable)
49+
SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::_symbolToStringTag), PropertyNone)
4450
};
4551

4652
SimpleTypeHandler<1> JavascriptLibrary::SharedPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::constructor), PropertyWritable | PropertyConfigurable, PropertyTypesWritableDataOnly, 4, sizeof(DynamicObject));
4753
SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithoutPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable);
4854
SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithPrototypeTypeHandlerV11(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype), PropertyWritable);
4955
SimpleTypeHandler<2> JavascriptLibrary::SharedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(SharedFunctionPropertyDescriptors));
50-
SimpleTypeHandler<1> JavascriptLibrary::SharedIdMappedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype));
56+
SimpleTypeHandler<2> JavascriptLibrary::SharedIdMappedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(SharedIdMappedFunctionPropertyDescriptors));
57+
SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithConfigurableLengthTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length), PropertyConfigurable);
5158
SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithLengthTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length));
5259
SimpleTypeHandler<2> JavascriptLibrary::SharedFunctionWithLengthAndNameTypeHandler(NO_WRITE_BARRIER_TAG(FunctionWithLengthAndNameTypeDescriptors));
5360
SimpleTypeHandler<1> JavascriptLibrary::SharedNamespaceSymbolTypeHandler(NO_WRITE_BARRIER_TAG(ModuleNamespaceTypeDescriptors));
@@ -898,6 +905,11 @@ namespace Js
898905
GetDeferredFunctionTypeHandler(), false, false);
899906
}
900907

908+
DynamicType * JavascriptLibrary::CreateFunctionWithConfigurableLengthType(FunctionInfo * functionInfo)
909+
{
910+
return CreateFunctionWithConfigurableLengthType(this->GetFunctionPrototype(), functionInfo);
911+
}
912+
901913
DynamicType * JavascriptLibrary::CreateFunctionWithLengthType(FunctionInfo * functionInfo)
902914
{
903915
return CreateFunctionWithLengthType(this->GetFunctionPrototype(), functionInfo);
@@ -913,6 +925,14 @@ namespace Js
913925
return CreateFunctionWithLengthAndPrototypeType(this->GetFunctionPrototype(), functionInfo);
914926
}
915927

928+
DynamicType * JavascriptLibrary::CreateFunctionWithConfigurableLengthType(DynamicObject * prototype, FunctionInfo * functionInfo)
929+
{
930+
Assert(!functionInfo->HasBody());
931+
return DynamicType::New(scriptContext, TypeIds_Function, prototype,
932+
this->inProfileMode? ProfileEntryThunk : functionInfo->GetOriginalEntryPoint(),
933+
&SharedFunctionWithConfigurableLengthTypeHandler);
934+
}
935+
916936
DynamicType * JavascriptLibrary::CreateFunctionWithLengthType(DynamicObject * prototype, FunctionInfo * functionInfo)
917937
{
918938
Assert(!functionInfo->HasBody());
@@ -5034,11 +5054,15 @@ namespace Js
50345054
{
50355055
function->ReplaceType(crossSiteExternalConstructFunctionWithPrototypeType);
50365056
}
5037-
else
5057+
else if (typeHandler == &SharedIdMappedFunctionWithPrototypeTypeHandler)
50385058
{
5039-
Assert(typeHandler == &SharedIdMappedFunctionWithPrototypeTypeHandler);
50405059
function->ReplaceType(crossSiteIdMappedFunctionWithPrototypeType);
50415060
}
5061+
else
5062+
{
5063+
function->ChangeType();
5064+
function->SetEntryPoint(scriptContext->CurrentCrossSiteThunk);
5065+
}
50425066
}
50435067
}
50445068

lib/Runtime/Library/JavascriptLibrary.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,13 +519,15 @@ namespace Js
519519
static SimpleTypeHandler<1> SharedFunctionWithoutPrototypeTypeHandler;
520520
static SimpleTypeHandler<1> SharedFunctionWithPrototypeTypeHandlerV11;
521521
static SimpleTypeHandler<2> SharedFunctionWithPrototypeTypeHandler;
522+
static SimpleTypeHandler<1> SharedFunctionWithConfigurableLengthTypeHandler;
522523
static SimpleTypeHandler<1> SharedFunctionWithLengthTypeHandler;
523524
static SimpleTypeHandler<2> SharedFunctionWithLengthAndNameTypeHandler;
524-
static SimpleTypeHandler<1> SharedIdMappedFunctionWithPrototypeTypeHandler;
525+
static SimpleTypeHandler<2> SharedIdMappedFunctionWithPrototypeTypeHandler;
525526
static SimpleTypeHandler<1> SharedNamespaceSymbolTypeHandler;
526527
static MissingPropertyTypeHandler MissingPropertyHolderTypeHandler;
527528

528529
static SimplePropertyDescriptor const SharedFunctionPropertyDescriptors[2];
530+
static SimplePropertyDescriptor const SharedIdMappedFunctionPropertyDescriptors[2];
529531
static SimplePropertyDescriptor const HeapArgumentsPropertyDescriptors[3];
530532
static SimplePropertyDescriptor const FunctionWithLengthAndPrototypeTypeDescriptors[2];
531533
static SimplePropertyDescriptor const FunctionWithLengthAndNameTypeDescriptors[2];
@@ -984,9 +986,11 @@ namespace Js
984986
DynamicType * CreateDeferredPrototypeFunctionType(JavascriptMethod entrypoint);
985987
DynamicType * CreateDeferredPrototypeFunctionTypeNoProfileThunk(JavascriptMethod entrypoint, bool isShared = false, bool isLengthAvailable = false);
986988
DynamicType * CreateFunctionType(JavascriptMethod entrypoint, RecyclableObject* prototype = nullptr);
989+
DynamicType * CreateFunctionWithConfigurableLengthType(FunctionInfo * functionInfo);
987990
DynamicType * CreateFunctionWithLengthType(FunctionInfo * functionInfo);
988991
DynamicType * CreateFunctionWithLengthAndNameType(FunctionInfo * functionInfo);
989992
DynamicType * CreateFunctionWithLengthAndPrototypeType(FunctionInfo * functionInfo);
993+
DynamicType * CreateFunctionWithConfigurableLengthType(DynamicObject * prototype, FunctionInfo * functionInfo);
990994
DynamicType * CreateFunctionWithLengthType(DynamicObject * prototype, FunctionInfo * functionInfo);
991995
DynamicType * CreateFunctionWithLengthAndNameType(DynamicObject * prototype, FunctionInfo * functionInfo);
992996
DynamicType * CreateFunctionWithLengthAndPrototypeType(DynamicObject * prototype, FunctionInfo * functionInfo);

lib/Runtime/Library/JavascriptProxy.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ namespace Js
130130

131131
JavascriptProxy* proxy = JavascriptProxy::Create(scriptContext, args);
132132
JavascriptLibrary* library = scriptContext->GetLibrary();
133-
DynamicType* type = library->CreateFunctionWithLengthType(&EntryInfo::Revoke);
133+
DynamicType* type = library->CreateFunctionWithConfigurableLengthType(&EntryInfo::Revoke);
134134
RuntimeFunction* revoker = RecyclerNewEnumClass(scriptContext->GetRecycler(),
135135
JavascriptLibrary::EnumFunctionClass, RuntimeFunction,
136136
type, &EntryInfo::Revoke);

lib/Runtime/Library/ScriptFunction.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ namespace Js
106106

107107
pfuncScriptWithInlineCache->SetHasSuperReference(hasSuperReference);
108108

109+
ScriptFunctionType *scFuncType = functionProxy->GetUndeferredFunctionType();
110+
if (scFuncType)
111+
{
112+
Assert(pfuncScriptWithInlineCache->GetType() == functionProxy->GetDeferredPrototypeType());
113+
pfuncScriptWithInlineCache->GetTypeHandler()->EnsureObjectReady(pfuncScriptWithInlineCache);
114+
}
115+
109116
if (PHASE_TRACE1(Js::ScriptFunctionWithInlineCachePhase))
110117
{
111118
char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
@@ -123,6 +130,13 @@ namespace Js
123130

124131
pfuncScript->SetHasSuperReference(hasSuperReference);
125132

133+
ScriptFunctionType *scFuncType = functionProxy->GetUndeferredFunctionType();
134+
if (scFuncType)
135+
{
136+
Assert(pfuncScript->GetType() == functionProxy->GetDeferredPrototypeType());
137+
pfuncScript->GetTypeHandler()->EnsureObjectReady(pfuncScript);
138+
}
139+
126140
JS_ETW(EventWriteJSCRIPT_RECYCLER_ALLOCATE_FUNCTION(pfuncScript, EtwTrace::GetFunctionId(functionProxy)));
127141

128142
return pfuncScript;

lib/Runtime/Runtime.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ namespace Js
134134
class DeferredTypeHandlerBase;
135135
template <bool IsPrototype> class NullTypeHandler;
136136
template<size_t size> class SimpleTypeHandler;
137-
class PathTypeHandler;
137+
class PathTypeHandlerBase;
138138
class IndexPropertyDescriptor;
139139
class DynamicObject;
140140
class ArrayObject;

lib/Runtime/Types/PathTypeHandler.cpp

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ namespace Js
130130
#endif
131131
}
132132

133+
void PathTypeSingleSuccessorInfo::ReplaceSuccessor(DynamicType * type, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef)
134+
{
135+
Assert(successorKey == key);
136+
successorTypeWeakRef = typeWeakRef;
137+
}
138+
133139
template<class Fn>
134140
void PathTypeSingleSuccessorInfo::MapSingleSuccessor(Fn fn)
135141
{
@@ -168,6 +174,13 @@ namespace Js
168174
propertySuccessors->Item(key, typeWeakRef);
169175
}
170176

177+
void PathTypeMultiSuccessorInfo::ReplaceSuccessor(DynamicType * type, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef)
178+
{
179+
Assert(this->propertySuccessors);
180+
Assert(propertySuccessors->Item(key));
181+
propertySuccessors->Item(key, typeWeakRef);
182+
}
183+
171184
template<class Fn>
172185
void PathTypeMultiSuccessorInfo::MapMultiSuccessors(Fn fn)
173186
{
@@ -343,9 +356,21 @@ namespace Js
343356
return found;
344357
}
345358

346-
BOOL PathTypeHandlerBase::SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes)
359+
BOOL PathTypeHandlerBase::SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes, bool isInit)
347360
{
348-
if (instanceAttributes == nullptr ? propertyAttributes == ObjectSlotAttr_Default : propertyAttributes == instanceAttributes[propertyIndex])
361+
if (instanceAttributes)
362+
{
363+
if (!isInit)
364+
{
365+
// Preserve non-default bits like accessors
366+
propertyAttributes = (ObjectSlotAttributes)(propertyAttributes | (instanceAttributes[propertyIndex] & ~ObjectSlotAttr_PropertyAttributesMask));
367+
}
368+
if (propertyAttributes == instanceAttributes[propertyIndex])
369+
{
370+
return true;
371+
}
372+
}
373+
else if (propertyAttributes == ObjectSlotAttr_Default)
349374
{
350375
return true;
351376
}
@@ -714,18 +739,16 @@ namespace Js
714739
// In CacheOperators::CachePropertyWrite we ensure that we never cache property adds for types that aren't shared.
715740
Assert(!instance->GetDynamicType()->GetIsShared() || GetIsShared());
716741

717-
Assert(instance->GetDynamicType()->GetIsShared() == GetIsShared());
718-
719742
if (setAttributes)
720743
{
721-
this->SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), attr);
744+
this->SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), attr, isInit);
722745
}
723746
else if (isInit)
724747
{
725748
ObjectSlotAttributes * attributes = this->GetAttributeArray();
726749
if (attributes && (attributes[index] & ObjectSlotAttr_Accessor))
727750
{
728-
this->SetAttributesHelper(instance, propertyId, index, attributes, (ObjectSlotAttributes)(attributes[index] & ~ObjectSlotAttr_Accessor));
751+
this->SetAttributesHelper(instance, propertyId, index, attributes, (ObjectSlotAttributes)(attributes[index] & ~ObjectSlotAttr_Accessor), true);
729752
}
730753
}
731754
PathTypeHandlerBase *newTypeHandler = PathTypeHandlerBase::FromTypeHandler(instance->GetDynamicType()->GetTypeHandler());
@@ -1722,7 +1745,12 @@ namespace Js
17221745
return true;
17231746
}
17241747

1725-
return SetAttributesHelper(instance, propertyId, propertyIndex, GetAttributeArray(), PropertyAttributesToObjectSlotAttributes(attributes));
1748+
return SetAttributesAtIndex(instance, propertyId, propertyIndex, attributes);
1749+
}
1750+
1751+
BOOL PathTypeHandlerBase::SetAttributesAtIndex(DynamicObject* instance, PropertyId propertyId, PropertyIndex index, PropertyAttributes attributes)
1752+
{
1753+
return SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), PropertyAttributesToObjectSlotAttributes(attributes));
17261754
}
17271755

17281756
BOOL PathTypeHandlerBase::GetAttributesWithPropertyIndex(DynamicObject * instance, PropertyId propertyId, BigPropertyIndex index, PropertyAttributes * attributes)

lib/Runtime/Types/PathTypeHandler.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ namespace Js
3939
bool IsMultiSuccessor() const { return !IsSingleSuccessor(); }
4040
virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const = 0;
4141
virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) = 0;
42+
virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) = 0;
4243

4344
template<class Fn> void MapSuccessors(Fn fn);
4445
template<class Fn> void MapSuccessorsUntil(Fn fn);
@@ -61,6 +62,7 @@ namespace Js
6162

6263
virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const override;
6364
virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) override;
65+
virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) override;
6466

6567
template<class Fn> void MapSingleSuccessor(Fn fn);
6668

@@ -78,6 +80,7 @@ namespace Js
7880

7981
virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const override;
8082
virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) override;
83+
virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) override;
8184

8285
template<class Fn> void MapMultiSuccessors(Fn fn);
8386
template<class Fn> void MapMultiSuccessorsUntil(Fn fn);
@@ -89,6 +92,8 @@ namespace Js
8992

9093
class PathTypeHandlerBase : public DynamicTypeHandler
9194
{
95+
template<size_t size>
96+
friend class SimpleTypeHandler;
9297
friend class PathTypeHandlerNoAttr;
9398
friend class PathTypeHandlerWithAttr;
9499
friend class DynamicObject;
@@ -115,6 +120,7 @@ namespace Js
115120
template<class Fn> void MapSuccessorsUntil(Fn fn);
116121
PathTypeSuccessorInfo * GetSuccessorInfo() const { return successorInfo; }
117122
void SetSuccessorInfo(PathTypeSuccessorInfo * info) { successorInfo = info; }
123+
void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef) { return successorInfo->ReplaceSuccessor(type, key, typeWeakRef); }
118124

119125
static PropertyAttributes ObjectSlotAttributesToPropertyAttributes(const ObjectSlotAttributes attr) { return attr & ObjectSlotAttr_PropertyAttributesMask; }
120126
static ObjectSlotAttributes PropertyAttributesToObjectSlotAttributes(const PropertyAttributes attr) { return (ObjectSlotAttributes)(attr & ObjectSlotAttr_PropertyAttributesMask); }
@@ -182,7 +188,8 @@ namespace Js
182188

183189
BOOL FindNextPropertyHelper(ScriptContext* scriptContext, ObjectSlotAttributes * objectAttributes, PropertyIndex& index, JavascriptString** propertyString,
184190
PropertyId* propertyId, PropertyAttributes* attributes, Type* type, DynamicType *typeToEnumerate, EnumeratorFlags flags, DynamicObject* instance, PropertyValueInfo* info);
185-
BOOL SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes);
191+
BOOL SetAttributesAtIndex(DynamicObject* instance, PropertyId propertyId, PropertyIndex index, PropertyAttributes attributes);
192+
BOOL SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes, bool isInit = false);
186193
BOOL SetAccessorsHelper(DynamicObject* instance, PropertyId propertyId, ObjectSlotAttributes * attributes, PathTypeSetterSlotIndex * setters, Var getter, Var setter, PropertyOperationFlags flags);
187194

188195
#if ENABLE_NATIVE_CODEGEN

0 commit comments

Comments
 (0)