Skip to content

Commit 5d30c52

Browse files
pleathpleath
committed
[MERGE #5457 @pleath] OS#18076647: Invalidate prototype inline caches for properties of objects added to the prototype chain
Merge pull request #5457 from pleath:18076647 Currently we invalidate proto inline caches for properties in the old prototype chain but not in the new one. We need to invalidate properties in the new prototype chain as well to make sure that missing property caches are cleared as necessary. Since the old and new prototype chains probably overlap, do some tracking to avoid duplicated effort.
2 parents 2f088a6 + 1dcea24 commit 5d30c52

File tree

12 files changed

+104
-44
lines changed

12 files changed

+104
-44
lines changed

‎lib/Runtime/Base/CrossSiteObject.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ namespace Js
3333
virtual DescriptorFlags GetSetter(PropertyId propertyId, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
3434
virtual DescriptorFlags GetSetter(JavascriptString* propertyNameString, Var* setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
3535
virtual BOOL SetAccessors(PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags) override;
36-
virtual void RemoveFromPrototype(ScriptContext * requestContext) override;
37-
virtual void AddToPrototype(ScriptContext * requestContext) override;
36+
virtual void RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
37+
virtual void AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
3838
virtual void SetPrototype(RecyclableObject* newPrototype) override;
3939

4040
virtual BOOL IsCrossSiteObject() const override { return TRUE; }
@@ -227,15 +227,15 @@ namespace Js
227227
}
228228

229229
template <typename T>
230-
void CrossSiteObject<T>::RemoveFromPrototype(ScriptContext * requestContext)
230+
void CrossSiteObject<T>::RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
231231
{
232-
__super::RemoveFromPrototype(this->GetScriptContext());
232+
__super::RemoveFromPrototype(this->GetScriptContext(), allProtoCachesInvalidated);
233233
}
234234

235235
template <typename T>
236-
void CrossSiteObject<T>::AddToPrototype(ScriptContext * requestContext)
236+
void CrossSiteObject<T>::AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
237237
{
238-
__super::AddToPrototype(this->GetScriptContext());
238+
__super::AddToPrototype(this->GetScriptContext(), allProtoCachesInvalidated);
239239
}
240240

241241
template <typename T>

‎lib/Runtime/Language/ModuleNamespace.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ namespace Js
7979
virtual BOOL GetDiagValueString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext) override;
8080
virtual BOOL GetDiagTypeString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext) override;
8181

82-
virtual void RemoveFromPrototype(ScriptContext * requestContext) override { Assert(false); }
83-
virtual void AddToPrototype(ScriptContext * requestContext) override { Assert(false); }
82+
virtual void RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override { Assert(false); }
83+
virtual void AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override { Assert(false); }
8484
virtual void SetPrototype(RecyclableObject* newPrototype) override { Assert(false); return; }
8585

8686
private:

‎lib/Runtime/Library/JavascriptObject.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,13 @@ using namespace Js;
227227

228228
if (isInvalidationOfInlineCacheNeeded)
229229
{
230+
bool allProtoCachesInvalidated = false;
231+
230232
// Notify old prototypes that they are being removed from a prototype chain. This triggers invalidating protocache, etc.
231-
JavascriptOperators::MapObjectAndPrototypes<true>(object->GetPrototype(), [=](RecyclableObject* obj)
233+
JavascriptOperators::MapObjectAndPrototypesUntil<true>(object->GetPrototype(), [&](RecyclableObject* obj)->bool
232234
{
233-
obj->RemoveFromPrototype(scriptContext);
235+
obj->RemoveFromPrototype(scriptContext, &allProtoCachesInvalidated);
236+
return allProtoCachesInvalidated;
234237
});
235238

236239
// Examine new prototype chain. If it brings in any special property, we need to invalidate related caches.
@@ -271,17 +274,20 @@ using namespace Js;
271274
object->GetLibrary()->GetTypesWithOnlyWritablePropertyProtoChainCache()->Clear();
272275
}
273276

274-
if (!objectAndPrototypeChainHasOnlyWritableDataProperties)
277+
if (!allProtoCachesInvalidated)
275278
{
276279
// Invalidate StoreField/PropertyGuards for any non-WritableData property in the new chain
277-
JavascriptOperators::MapObjectAndPrototypes<true>(newPrototype, [=](RecyclableObject* obj)
280+
JavascriptOperators::MapObjectAndPrototypesUntil<true>(newPrototype, [&](RecyclableObject* obj)->bool
278281
{
279-
if (!obj->HasOnlyWritableDataProperties())
280-
{
281-
obj->AddToPrototype(scriptContext);
282-
}
282+
obj->AddToPrototype(scriptContext, &allProtoCachesInvalidated);
283+
return allProtoCachesInvalidated;
283284
});
284285
}
286+
287+
JavascriptOperators::MapObjectAndPrototypesUntil<true>(object->GetPrototype(), [](RecyclableObject* obj)->bool
288+
{
289+
return obj->ClearProtoCachesWereInvalidated();
290+
});
285291
}
286292

287293
// Set to new prototype

‎lib/Runtime/Library/JavascriptProxy.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,12 +1507,12 @@ namespace Js
15071507
return nullptr;
15081508
}
15091509

1510-
void JavascriptProxy::RemoveFromPrototype(ScriptContext * requestContext)
1510+
void JavascriptProxy::RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
15111511
{
15121512
Assert(FALSE);
15131513
}
15141514

1515-
void JavascriptProxy::AddToPrototype(ScriptContext * requestContext)
1515+
void JavascriptProxy::AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
15161516
{
15171517
Assert(FALSE);
15181518
}

‎lib/Runtime/Library/JavascriptProxy.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ namespace Js
138138
virtual bool CanStorePropertyValueDirectly(PropertyId propertyId, bool allowLetConst) { Assert(false); return false; };
139139
#endif
140140

141-
virtual void RemoveFromPrototype(ScriptContext * requestContext) override;
142-
virtual void AddToPrototype(ScriptContext * requestContext) override;
141+
virtual void RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
142+
virtual void AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
143143
virtual void SetPrototype(RecyclableObject* newPrototype) override;
144144

145145
BOOL SetPrototypeTrap(RecyclableObject* newPrototype, bool showThrow, ScriptContext * requestContext);

‎lib/Runtime/Types/DynamicObject.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,9 @@ namespace Js
296296
virtual bool CanStorePropertyValueDirectly(PropertyId propertyId, bool allowLetConst) override;
297297
#endif
298298

299-
virtual void RemoveFromPrototype(ScriptContext * requestContext) override;
300-
virtual void AddToPrototype(ScriptContext * requestContext) override;
299+
virtual void RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
300+
virtual void AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) override;
301+
virtual bool ClearProtoCachesWereInvalidated() override;
301302
virtual void SetPrototype(RecyclableObject* newPrototype) override;
302303

303304
virtual BOOL IsCrossSiteObject() const { return FALSE; }

‎lib/Runtime/Types/DynamicType.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,14 +609,19 @@ namespace Js
609609
}
610610
#endif
611611

612-
void DynamicObject::RemoveFromPrototype(ScriptContext * requestContext)
612+
bool DynamicObject::ClearProtoCachesWereInvalidated()
613613
{
614-
GetTypeHandler()->RemoveFromPrototype(this, requestContext);
614+
return GetTypeHandler()->ClearProtoCachesWereInvalidated();
615615
}
616616

617-
void DynamicObject::AddToPrototype(ScriptContext * requestContext)
617+
void DynamicObject::RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
618618
{
619-
GetTypeHandler()->AddToPrototype(this, requestContext);
619+
GetTypeHandler()->RemoveFromPrototype(this, requestContext, allProtoCachesInvalidated);
620+
}
621+
622+
void DynamicObject::AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated)
623+
{
624+
GetTypeHandler()->AddToPrototype(this, requestContext, allProtoCachesInvalidated);
620625
}
621626

622627
void DynamicObject::SetPrototype(RecyclableObject* newPrototype)

‎lib/Runtime/Types/RecyclableObject.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,10 @@ namespace Js {
369369
virtual bool CanStorePropertyValueDirectly(PropertyId propertyId, bool allowLetConst) { Assert(false); return false; };
370370
#endif
371371

372-
virtual void RemoveFromPrototype(ScriptContext * requestContext) { AssertMsg(false, "Shouldn't call this implementation."); }
373-
virtual void AddToPrototype(ScriptContext * requestContext) { AssertMsg(false, "Shouldn't call this implementation."); }
372+
virtual void RemoveFromPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) { AssertMsg(false, "Shouldn't call this implementation."); }
373+
virtual void AddToPrototype(ScriptContext * requestContext, bool * allProtoCachesInvalidated) { AssertMsg(false, "Shouldn't call this implementation."); }
374374
virtual void SetPrototype(RecyclableObject* newPrototype) { AssertMsg(false, "Shouldn't call this implementation."); }
375+
virtual bool ClearProtoCachesWereInvalidated() { AssertMsg(false, "Shouldn't call this implementation."); return false; }
375376

376377
virtual BOOL ToString(Js::Var* value, Js::ScriptContext* scriptContext) { AssertMsg(FALSE, "Do not use this function."); return false; }
377378

‎lib/Runtime/Types/TypeHandler.cpp

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ using namespace Js;
6464
flags(flags),
6565
propertyTypes(PropertyTypesWritableDataOnly | PropertyTypesReserved),
6666
offsetOfInlineSlots(offsetOfInlineSlots),
67-
unusedBytes(Js::AtomTag)
67+
unusedBytes(Js::AtomTag),
68+
protoCachesWereInvalidated(false)
6869
{
6970
Assert(!GetIsOrMayBecomeShared() || GetIsLocked());
7071
Assert(offsetOfInlineSlots != 0 || inlineSlotCapacity == 0);
@@ -263,7 +264,7 @@ using namespace Js;
263264
}
264265

265266
template<bool isStoreField>
266-
void DynamicTypeHandler::InvalidateInlineCachesForAllProperties(ScriptContext* requestContext)
267+
bool DynamicTypeHandler::InvalidateInlineCachesForAllProperties(ScriptContext* requestContext)
267268
{
268269
int count = GetPropertyCount();
269270
if (count < 128) // Invalidate a propertyId involves dictionary lookups. Only do this when the number is relatively small.
@@ -276,31 +277,51 @@ using namespace Js;
276277
isStoreField ? requestContext->InvalidateStoreFieldCaches(propertyId) : requestContext->InvalidateProtoCaches(propertyId);
277278
}
278279
}
280+
return false;
279281
}
280282
else
281283
{
282284
isStoreField ? requestContext->InvalidateAllStoreFieldCaches() : requestContext->InvalidateAllProtoCaches();
285+
return true;
283286
}
284287
}
285288

286-
void DynamicTypeHandler::InvalidateProtoCachesForAllProperties(ScriptContext* requestContext)
289+
bool DynamicTypeHandler::InvalidateProtoCachesForAllProperties(ScriptContext* requestContext)
290+
{
291+
bool result = InvalidateInlineCachesForAllProperties<false>(requestContext);
292+
this->SetProtoCachesWereInvalidated();
293+
return result;
294+
}
295+
296+
bool DynamicTypeHandler::InvalidateStoreFieldCachesForAllProperties(ScriptContext* requestContext)
287297
{
288-
InvalidateInlineCachesForAllProperties<false>(requestContext);
298+
return InvalidateInlineCachesForAllProperties<true>(requestContext);
289299
}
290300

291-
void DynamicTypeHandler::InvalidateStoreFieldCachesForAllProperties(ScriptContext* requestContext)
301+
bool DynamicTypeHandler::ClearProtoCachesWereInvalidated()
292302
{
293-
InvalidateInlineCachesForAllProperties<true>(requestContext);
303+
bool done = !this->ProtoCachesWereInvalidated();
304+
this->protoCachesWereInvalidated = false;
305+
return done;
294306
}
295307

296-
void DynamicTypeHandler::RemoveFromPrototype(DynamicObject* instance, ScriptContext * requestContext)
308+
void DynamicTypeHandler::RemoveFromPrototype(DynamicObject* instance, ScriptContext * requestContext, bool * allProtoCachesInvalidated)
297309
{
298-
InvalidateProtoCachesForAllProperties(requestContext);
310+
Assert(!*allProtoCachesInvalidated);
311+
*allProtoCachesInvalidated = InvalidateProtoCachesForAllProperties(requestContext);
299312
}
300313

301-
void DynamicTypeHandler::AddToPrototype(DynamicObject* instance, ScriptContext * requestContext)
314+
void DynamicTypeHandler::AddToPrototype(DynamicObject* instance, ScriptContext * requestContext, bool * allProtoCachesInvalidated)
302315
{
303-
InvalidateStoreFieldCachesForAllProperties(requestContext);
316+
Assert(!*allProtoCachesInvalidated);
317+
if (this->ProtoCachesWereInvalidated())
318+
{
319+
*allProtoCachesInvalidated = true;
320+
}
321+
else
322+
{
323+
*allProtoCachesInvalidated = InvalidateProtoCachesForAllProperties(requestContext);
324+
}
304325
}
305326

306327
void DynamicTypeHandler::SetPrototype(DynamicObject* instance, RecyclableObject* newPrototype)

‎lib/Runtime/Types/TypeHandler.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ namespace Js
4949
Field(uint16) unusedBytes; // This always has it's lowest bit set to avoid false references
5050
Field(uint16) inlineSlotCapacity;
5151
Field(bool) isNotPathTypeHandlerOrHasUserDefinedCtor;
52+
Field(bool) protoCachesWereInvalidated;
5253

5354
public:
5455
DEFINE_GETCPPNAME_ABSTRACT();
@@ -58,7 +59,8 @@ namespace Js
5859
slotCapacity(typeHandler->slotCapacity),
5960
offsetOfInlineSlots(typeHandler->offsetOfInlineSlots),
6061
isNotPathTypeHandlerOrHasUserDefinedCtor(typeHandler->isNotPathTypeHandlerOrHasUserDefinedCtor),
61-
unusedBytes(typeHandler->unusedBytes)
62+
unusedBytes(typeHandler->unusedBytes),
63+
protoCachesWereInvalidated(false)
6264
{
6365
}
6466

@@ -528,15 +530,19 @@ namespace Js
528530

529531
private:
530532
template<bool isStoreField>
531-
void InvalidateInlineCachesForAllProperties(ScriptContext* requestContext);
533+
bool InvalidateInlineCachesForAllProperties(ScriptContext* requestContext);
532534

533535
public:
534-
void InvalidateProtoCachesForAllProperties(ScriptContext* requestContext);
535-
void InvalidateStoreFieldCachesForAllProperties(ScriptContext* requestContext);
536+
bool InvalidateProtoCachesForAllProperties(ScriptContext* requestContext);
537+
bool InvalidateStoreFieldCachesForAllProperties(ScriptContext* requestContext);
538+
539+
bool ProtoCachesWereInvalidated() const { return protoCachesWereInvalidated; }
540+
void SetProtoCachesWereInvalidated() { protoCachesWereInvalidated = true; }
541+
bool ClearProtoCachesWereInvalidated();
536542

537543
// For changing __proto__
538-
void RemoveFromPrototype(DynamicObject* instance, ScriptContext * requestContext);
539-
void AddToPrototype(DynamicObject* instance, ScriptContext * requestContext);
544+
void RemoveFromPrototype(DynamicObject* instance, ScriptContext * requestContext, bool * allProtoCachesInvalidated);
545+
void AddToPrototype(DynamicObject* instance, ScriptContext * requestContext, bool * allProtoCachesInvalidated);
540546
virtual void SetPrototype(DynamicObject* instance, RecyclableObject* newPrototype);
541547

542548
virtual void ResetTypeHandler(DynamicObject * instance);

0 commit comments

Comments
 (0)