-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Type sharing for __proto__ #1489
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
Conversation
lib/Common/ConfigFlagsList.h
Outdated
| 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
objis changed, we convert its type handler toSimpleDictionaryTypeHandler, create a newDynamicTypeand 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 forobjand 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, whereverobjis 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
TypeOfPrototypeObjectinternal property id. This internal property is used on prototype object to cache thedynamicTypethat 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 thedynamicTypeinside the new prototype object that we are about to assign to anobj. If we find a pre-existing type, we would just assign it toobjinstead of creating a new type. Only difference is that we have to assign a type toobjsuch that it contains all the properties thatobjhad 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 toobjandobj2.For example, lets say we cache the type
T1in prototype objectprotoObj. When we executeobj.__proto__ = protoObjwe would evolveT1toT2and assign toobj. Later, if we are executingobj2.__proto__ = protoObj, we would start with cached typeT1and by based on properties present onobj2, PathTypeHandler would evolve toT2. Thusobj2will get typeT2which is same as that ofobj.We update the cache with new type for below conditions:
inlineSlotCapacityis different than that of current object's type.offsetOfInlineSlotis different than that of current object's type.However there might be scenarios where same prototype
protoObjis used to set prototype of 2 different types of objects alternatively. One that hasinlineSlotCapacityzero (e.g. objects having ExternalTypes) and other that hasinlineSlotCapacitynon-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 existingTypeOfPrototypeObjectinZTypeOfPrototypeObject(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__.