-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Type sharing for __proto__ #1489
Conversation
@@ -344,6 +344,7 @@ PHASE(All) | |||
PHASE(StackFramesEvent) | |||
#endif | |||
PHASE(PerfHint) | |||
PHASE(TypeShare) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give this a less generic name? It doesn't control type-sharing, only a very specific instance of type-sharing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Will change it to TypeSharingForChangePrototype
.
8b4918d
to
d3c666e
Compare
56b00d0
to
2d035e5
Compare
// Use TypeOfPrototypeObjectInlined slot only if this is DynamicType and typeId is TypeIds_Object | ||
// For everything else, i.e. (DynamicType, other typeIds) and (JsrtExternalType, TypeIds_Object) use | ||
// TypeOfPrototypeObjectDictionary | ||
Js::InternalPropertyIds propertyIdHoldingCache = (!isJsrtExternalType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious have you tried to allocate inline slot for jsrtexternalobject as well?
@pleath, I have updated the code with new design we discussed. |
: DynamicTypeHandler::RoundUpInlineSlotCapacity(requestedInlineSlotCapacity); | ||
swprintf_s(reason, 1024, _u("InlineSlotCapacity mismatch. Required = %d, Cached = %d"), requiredCapacity, cachedDynamicTypeHandler->GetInlineSlotCapacity()); | ||
#endif | ||
Assert(cachedDynamicTypeHandler->GetInlineSlotCapacity() >= roundedInlineSlotCapacity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also confirm that it's safe to shrink the capacity in this way by verifying that the total number of properties <= the current inlineSlotCapacity?
LGTM. Thanks. |
9bde02d
to
8acb824
Compare
Implementation of type sharing using PathTypeHandler for __proto__ scenario. 1. InlineSlot - Used for caching type in CreateObject() scenario like before when object doesn't have a type to start with. 2. DictionarySlot - Used for __proto__ cases. This is a BaseDictionary from oldType (DynamicType) to promoted newType (again DynamicType). The idea is that we will cache the promoted type instead of promoting the base type everytime. Added unit test Added `-trace:TypeShareForChangePrototype` for testing
8acb824
to
8be8fc8
Compare
Type sharing for proto scenario
Problem :
Today everytime a prototype of an object
obj
is changed, we convert its type handler toSimpleDictionaryTypeHandler
, create a newDynamicType
and assign it to the object. In future, every single time there is a property access on this object (obj.a =
or= obj.a
), profile data would see a new type forobj
and would conclude that the call site has polymorphic types and would create optimization based on caching those types. But even after JIT, the cache won't help because it would still get new type and thus we end up doing bunch of checks in JIT after which either we bailout or take slowpath to extract the property from typehandler. It would be nice if while executingobj.__proto__ = protoObj;
we detect ifobj
's current type is X andprotoObj
's current type is Y, then always assign type Z toobj
. That way in future, whereverobj
is used for property access, it would see same type Z and profile data / JIT would be optimized to take advantage of object type specialization.Solution :
Today, in creating an object from prototype, we make use of
TypeOfPrototypeObject
internal property id. This internal property is used on prototype object to cache thedynamicType
that is assigned to an object using that prototype object. Later if another object is created with same prototype object, it would get the cached type instead of creating a new type, thus leads to sharing types between two objects.I have extended the logic to
__proto__
scenario by caching thedynamicType
inside the new prototype object that we are about to assign to anobj
. If we find a pre-existing type, we would just assign it toobj
instead of creating a new type. Only difference is that we have to assign a type toobj
such that it contains all the properties thatobj
had before execution of__proto__
. To address that, after fetching the type from cache (internal property id of new prototype object), I evolve that type based on existing properties of an object. If the evolution on cached type is happening for first time, PathTypeHandler would perform the acutal evolution of type and assign the evolved type toobj
. Next time if we try to set same prototype object to a different objectobj2
(but having same properties asobj
), PathTypeHandler would evolve cached type to same type that was assigned toobj
. Thus we would always assign same type toobj
andobj2
.For example, lets say we cache the type
T1
in prototype objectprotoObj
. When we executeobj.__proto__ = protoObj
we would evolveT1
toT2
and assign toobj
. Later, if we are executingobj2.__proto__ = protoObj
, we would start with cached typeT1
and by based on properties present onobj2
, PathTypeHandler would evolve toT2
. Thusobj2
will get typeT2
which is same as that ofobj
.We update the cache with new type for below conditions:
inlineSlotCapacity
is different than that of current object's type.offsetOfInlineSlot
is different than that of current object's type.However there might be scenarios where same prototype
protoObj
is used to set prototype of 2 different types of objects alternatively. One that hasinlineSlotCapacity
zero (e.g. objects having ExternalTypes) and other that hasinlineSlotCapacity
non-zero. In this case, we would keep updating prototype's cached type with a new type which would defeat the purpose of type sharing. Hence I have splitted the existingTypeOfPrototypeObject
inZTypeOfPrototypeObject
(cached types that has zero inlineSlots) andNZTypeOfPrototypeObject
(cached types that has non-zero inlineSlots).Test: Unit-test passes, automated test in progress
Perf: This gives _25%_ win in acme-air. On my 2 minutes run, turned out that
__proto__
was called 85292 times. That means we would have created 85292 new types. After my change, we just create 14 types and they are shared 85,278 objects. Other benchmarks had no change because they don't use__proto__
.