Skip to content

Commit 4bf71d0

Browse files
committed
[MERGE #3587 @rajatd] Setting internal properties with "enumerable" attribute off.
Merge pull request #3587 from rajatd:hasNoEnumerableProps-2 While we don't enumerate internal properties, setting an internal property as enumerable unnecessarily sets the hasNoEnumerableProperties flag to false on the type handler. This may have perf impact as this flag is used to skip prototypes when enumerating properties. Changing this to set internal properties as non-enumerable from the beginning. Doing this for non-path type handlers only because, 1. An object with a path type handler should theoretically have hasNoEnumerableProperties set to false anyway. 2. Setting a non-enumerable property on an object with a path type handler will convert its type handler to a (simple) dictionary type handler. Fixes #1622
2 parents e5c80f3 + b25f86d commit 4bf71d0

9 files changed

+39
-1
lines changed

lib/Runtime/Types/DeferredTypeHandler.h

+12
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ namespace Js
9797
virtual BOOL GetProperty(DynamicObject* instance, Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
9898
virtual BOOL SetProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
9999
virtual BOOL SetProperty(DynamicObject* instance, JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
100+
virtual BOOL SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags) override;
100101
virtual DescriptorFlags GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
101102
virtual DescriptorFlags GetSetter(DynamicObject* instance, JavascriptString* propertyNameString, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
102103
virtual BOOL DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags flags) override;
@@ -320,6 +321,17 @@ namespace Js
320321
return GetCurrentTypeHandler(instance)->SetProperty(instance, propertyNameString, value, flags, info);
321322
}
322323

324+
template <DeferredTypeInitializer initializer, typename DeferredTypeFilter, bool isPrototypeTemplate, uint16 _inlineSlotCapacity, uint16 _offsetOfInlineSlots>
325+
BOOL DeferredTypeHandler<initializer, DeferredTypeFilter, isPrototypeTemplate, _inlineSlotCapacity, _offsetOfInlineSlots>::SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags)
326+
{
327+
if (!EnsureObjectReady(instance, DeferredInitializeMode_Set))
328+
{
329+
return TRUE;
330+
}
331+
332+
return GetCurrentTypeHandler(instance)->SetInternalProperty(instance, propertyId, value, flags);
333+
}
334+
323335
template <DeferredTypeInitializer initializer, typename DeferredTypeFilter, bool isPrototypeTemplate, uint16 _inlineSlotCapacity, uint16 _offsetOfInlineSlots>
324336
DescriptorFlags DeferredTypeHandler<initializer, DeferredTypeFilter, isPrototypeTemplate, _inlineSlotCapacity, _offsetOfInlineSlots>::GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext)
325337
{

lib/Runtime/Types/DictionaryTypeHandler.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,12 @@ namespace Js
864864
return DictionaryTypeHandlerBase<T>::SetProperty(instance, propertyRecord->GetPropertyId(), value, flags, info);
865865
}
866866

867+
template <typename T>
868+
BOOL DictionaryTypeHandlerBase<T>::SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags)
869+
{
870+
return SetPropertyWithAttributes(instance, propertyId, value, PropertyWritable & PropertyConfigurable, nullptr, flags);
871+
}
872+
867873
template <typename T>
868874
BOOL DictionaryTypeHandlerBase<T>::DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags propertyOperationFlags)
869875
{

lib/Runtime/Types/DictionaryTypeHandler.h

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ namespace Js
8989
virtual BOOL InitProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
9090
virtual BOOL SetProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
9191
virtual BOOL SetProperty(DynamicObject* instance, JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
92+
virtual BOOL SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags) override;
9293
virtual DescriptorFlags GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
9394
virtual DescriptorFlags GetSetter(DynamicObject* instance, JavascriptString* propertyNameString, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
9495
virtual BOOL DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags flags) override;

lib/Runtime/Types/NullTypeHandler.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,11 @@ namespace Js
132132
return ConvertToSimpleType(instance)->SetProperty(instance, propertyNameString, value, flags, info);
133133
}
134134

135+
BOOL NullTypeHandlerBase::SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags)
136+
{
137+
return SetPropertyWithAttributes(instance, propertyId, value, PropertyWritable & PropertyConfigurable, nullptr, flags);
138+
}
139+
135140
BOOL NullTypeHandlerBase::AddProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyAttributes attributes, PropertyValueInfo* info, PropertyOperationFlags flags, SideEffects possibleSideEffects)
136141
{
137142
if (this->isPrototype && (ChangeTypeOnProto() || (GetIsShared() && IsolatePrototypes())))

lib/Runtime/Types/NullTypeHandler.h

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ namespace Js
3939
virtual BOOL GetProperty(DynamicObject* instance, Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
4040
virtual BOOL SetProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
4141
virtual BOOL SetProperty(DynamicObject* instance, JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
42+
virtual BOOL SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags) override;
4243
virtual BOOL DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags flags) override;
4344
virtual BOOL IsEnumerable(DynamicObject* instance, PropertyId propertyId) override;
4445
virtual BOOL IsWritable(DynamicObject* instance, PropertyId propertyId) override;

lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,12 @@ namespace Js
14521452
return true;
14531453
}
14541454

1455+
template <typename TPropertyIndex, typename TMapKey, bool IsNotExtensibleSupported>
1456+
BOOL SimpleDictionaryTypeHandlerBase<TPropertyIndex, TMapKey, IsNotExtensibleSupported>::SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags)
1457+
{
1458+
return SetPropertyWithAttributes(instance, propertyId, value, PropertyWritable & PropertyConfigurable, nullptr, flags);
1459+
}
1460+
14551461
template <typename TPropertyIndex, typename TMapKey, bool IsNotExtensibleSupported>
14561462
DescriptorFlags SimpleDictionaryTypeHandlerBase<TPropertyIndex, TMapKey, IsNotExtensibleSupported>::GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext)
14571463
{

lib/Runtime/Types/SimpleDictionaryTypeHandler.h

+1
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ namespace Js
130130
virtual BOOL GetProperty(DynamicObject* instance, Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
131131
virtual BOOL SetProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
132132
virtual BOOL SetProperty(DynamicObject* instance, JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
133+
virtual BOOL SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags) override;
133134
virtual DescriptorFlags GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
134135
virtual DescriptorFlags GetSetter(DynamicObject* instance, JavascriptString* propertyNameString, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
135136
virtual BOOL DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags flags) override sealed;

lib/Runtime/Types/SimpleTypeHandler.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,12 @@ namespace Js
10591059
ConvertToSimpleDictionaryType(instance)->SetIsPrototype(instance);
10601060
}
10611061

1062+
template<size_t size>
1063+
BOOL SimpleTypeHandler<size>::SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags)
1064+
{
1065+
return SetPropertyWithAttributes(instance, propertyId, value, PropertyWritable & PropertyConfigurable, nullptr, flags);
1066+
}
1067+
10621068
#if DBG
10631069
template<size_t size>
10641070
bool SimpleTypeHandler<size>::CanStorePropertyValueDirectly(const DynamicObject* instance, PropertyId propertyId, bool allowLetConst)

lib/Runtime/Types/SimpleTypeHandler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ namespace Js
4545
virtual BOOL GetProperty(DynamicObject* instance, Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
4646
virtual BOOL SetProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
4747
virtual BOOL SetProperty(DynamicObject* instance, JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
48+
virtual BOOL SetInternalProperty(DynamicObject* instance, PropertyId propertyId, Var value, PropertyOperationFlags flags) override;
4849
virtual DescriptorFlags GetSetter(DynamicObject* instance, PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
4950
virtual DescriptorFlags GetSetter(DynamicObject* instance, JavascriptString* propertyNameString, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
5051
virtual BOOL DeleteProperty(DynamicObject* instance, PropertyId propertyId, PropertyOperationFlags flags) override;
@@ -60,7 +61,6 @@ namespace Js
6061
virtual BOOL SetPropertyWithAttributes(DynamicObject* instance, PropertyId propertyId, Var value, PropertyAttributes attributes, PropertyValueInfo* info, PropertyOperationFlags flags = PropertyOperation_None, SideEffects possibleSideEffects = SideEffects_Any) override;
6162
virtual BOOL SetAttributes(DynamicObject* instance, PropertyId propertyId, PropertyAttributes attributes) override;
6263
virtual BOOL GetAttributesWithPropertyIndex(DynamicObject * instance, PropertyId propertyId, BigPropertyIndex index, PropertyAttributes * attributes) override;
63-
6464
virtual void SetAllPropertiesToUndefined(DynamicObject* instance, bool invalidateFixedFields) override;
6565
virtual void MarshalAllPropertiesToScriptContext(DynamicObject* instance, ScriptContext* targetScriptContext, bool invalidateFixedFields) override;
6666
virtual DynamicTypeHandler* ConvertToTypeWithItemAttributes(DynamicObject* instance) override;

0 commit comments

Comments
 (0)