use ForInCache for enumerator in Object.assign #4852
Conversation
|
For reviewing convenience, I have the renaming as a separate commit |
lib/Runtime/Base/ScriptContext.cpp
Outdated
| { | ||
| if (Cache()->assignCache) | ||
| { | ||
| memset(Cache()->assignCache, 0, Js::Cache::AssignCacheSize); |
There was a problem hiding this comment.
oops, * sizeof(EnumeratorCache)
There was a problem hiding this comment.
actually... i should just use the forInCacheAllocator
In reply to: 176248209 [](ancestors = 176248209)
| this->cache.assignCache = AllocatorNewArrayZ(CacheAllocator, scriptContext->GetEnumeratorAllocator(), EnumeratorCache, Cache::AssignCacheSize); | ||
| } | ||
|
|
||
| return &this->cache.assignCache[(((size_t)type) >> PolymorphicInlineCacheShift) & (Cache::AssignCacheSize - 1)]; |
There was a problem hiding this comment.
AssignCacheSize [](start = 98, length = 15)
Maybe we could add a static assertion that AssignCacheSize is a power of 2, to avoid surprising out-of-bounds behavior if someone tries tweaking the constant.
| @@ -390,7 +390,7 @@ namespace Js | |||
| static void OP_InitComputedProperty(Var object, Var elementName, Var value, ScriptContext* scriptContext, PropertyOperationFlags flags = PropertyOperation_None); | |||
| static void OP_InitProto(Var object, PropertyId propertyId, Var value); | |||
|
|
|||
| static void OP_InitForInEnumerator(Var enumerable, ForInObjectEnumerator * enumerator, ScriptContext* scriptContext, ForInCache * forInCache = nullptr); | |||
| static void OP_InitForInEnumerator(Var enumerable, ForInObjectEnumerator * enumerator, ScriptContext* scriptContext, EnumeratorCache * forInCache = nullptr); | |||
There was a problem hiding this comment.
forInCache [](start = 143, length = 10)
Nit: could rename this param for consistency with others (and in corresponding cpp) #ByDesign
There was a problem hiding this comment.
I left this one on purpose because it is used specifically for for in
There was a problem hiding this comment.
Merge pull request #4852 from MikeHolman:assigncache Adds a per-ScriptContext polymorphic cache of size 16 for Object.assign. Preliminary perf looks like it improves React-redux by about 8%. Full perf run gave me numbers that are too good to be true (5% geomean improvement), but I'm not sure I believe it.
Adds a per-ScriptContext polymorphic cache of size 16 for Object.assign.
Preliminary perf looks like it improves React-redux by about 8%.
Full perf run gave me numbers that are too good to be true (5% geomean improvement), but I'm not sure I believe it.