address cr comments

MikeHolman · MikeHolman · commit 33c293188975 · 2017-05-08T13:41:47.000-07:00
diff --git a/lib/Runtime/Base/ScriptContext.cpp b/lib/Runtime/Base/ScriptContext.cpp
@@ -256,8 +256,6 @@ namespace Js
         memset(byteCodeHistogram, 0, sizeof(byteCodeHistogram));
 #endif
 
-        ClearArray(this->Cache()->propertyStrings, 80);
-
 #if DBG || defined(RUNTIME_DATA_COLLECTION)
         this->allocId = threadContext->GetScriptContextCount();
 #endif
@@ -415,10 +413,6 @@ namespace Js
             ClearInlineCaches();
             Assert(!this->hasProtoOrStoreFieldInlineCache);
         }
-        while (this->GetGlobalPICHead())
-        {
-            this->GetGlobalPICHead()->Finalize(true);
-        }
 
         if (this->hasIsInstInlineCache)
         {
@@ -1209,6 +1203,7 @@ namespace Js
 #endif
 #endif
 
+        ClearArray(this->Cache()->propertyStrings, 80);
         this->Cache()->dynamicRegexMap =
             RegexPatternMruMap::New(
                 recycler,
@@ -1597,18 +1592,17 @@ namespace Js
     {
         const char16* buf = propString->GetBuffer();
         const uint i = PropertyStringMap::PStrMapIndex(buf[0]);
-        PropertyStringMap* strMap = this->Cache()->propertyStrings[i];
-        if (strMap == NULL)
+        if (this->Cache()->propertyStrings[i] == NULL)
         {
             InitPropertyStringMap(i);
         }
         const uint j = PropertyStringMap::PStrMapIndex(buf[1]);
-        if (strMap->strLen2[j] == NULL && !isClosed)
+        if (this->Cache()->propertyStrings[i]->strLen2[j] == NULL && !isClosed)
         {
-            strMap->strLen2[j] = GetLibrary()->CreatePropertyString(propString);
+            this->Cache()->propertyStrings[i]->strLen2[j] = GetLibrary()->CreatePropertyString(propString);
             this->TrackPid(propString);
         }
-        return strMap->strLen2[j];
+        return this->Cache()->propertyStrings[i]->strLen2[j];
     }
 
     PropertyString* ScriptContext::CachePropertyString2(const PropertyRecord* propString)
@@ -1683,7 +1677,7 @@ namespace Js
                     cache->GetInlineCaches()[cache->GetInlineCacheIndexForType(type)].Clear();
                 }
                 cache = string->GetStElemInlineCache();
-                if (cache->PretendTryGetProperty(type, &info))
+                if (cache->PretendTrySetProperty(type, type, &info))
                 {
 #ifdef ENABLE_DEBUG_CONFIG_OPTIONS
                     if (PHASE_TRACE1(PropertyStringCachePhase))
diff --git a/lib/Runtime/Base/ScriptContext.h b/lib/Runtime/Base/ScriptContext.h
@@ -533,9 +533,6 @@ namespace Js
         InlineCache * GetValueOfInlineCache() const { return valueOfInlineCache;}
         InlineCache * GetToStringInlineCache() const { return toStringInlineCache; }
 
-        ScriptContextPolymorphicInlineCache * GetGlobalPICHead() const { return this->Cache()->globalPICHead; }
-        void SetGlobalPICHead(ScriptContextPolymorphicInlineCache * cache) { this->Cache()->globalPICHead = cache; }
-
     private:
 
         JavascriptFunction* GenerateRootFunction(ParseNodePtr parseTree, uint sourceIndex, Parser* parser, uint32 grfscr, CompileScriptException * pse, const char16 *rootDisplayName);
diff --git a/lib/Runtime/Language/InlineCache.cpp b/lib/Runtime/Language/InlineCache.cpp
@@ -1005,21 +1005,14 @@ namespace Js
         }
     }
 
-    ScriptContextPolymorphicInlineCache * ScriptContextPolymorphicInlineCache::New(uint16 size, ScriptContext* scriptContext)
+    ScriptContextPolymorphicInlineCache * ScriptContextPolymorphicInlineCache::New(uint16 size, JavascriptLibrary* javascriptLibrary)
     {
+        ScriptContext * scriptContext = javascriptLibrary->GetScriptContext();
         InlineCache * inlineCaches = AllocatorNewArrayZ(InlineCacheAllocator, scriptContext->GetInlineCacheAllocator(), InlineCache, size);
 #ifdef POLY_INLINE_CACHE_SIZE_STATS
         scriptContext->GetInlineCacheAllocator()->LogPolyCacheAlloc(size * sizeof(InlineCache));
 #endif
-        ScriptContextPolymorphicInlineCache * polymorphicInlineCache = RecyclerNewFinalizedLeaf(scriptContext->GetRecycler(), ScriptContextPolymorphicInlineCache, inlineCaches, size, scriptContext);
-
-        polymorphicInlineCache->prev = nullptr;
-        polymorphicInlineCache->next = polymorphicInlineCache->scriptContext->GetGlobalPICHead();
-        if (polymorphicInlineCache->next)
-        {
-            polymorphicInlineCache->next->prev = polymorphicInlineCache;
-        }
-        polymorphicInlineCache->scriptContext->SetGlobalPICHead(polymorphicInlineCache);
+        ScriptContextPolymorphicInlineCache * polymorphicInlineCache = RecyclerNewFinalized(scriptContext->GetRecycler(), ScriptContextPolymorphicInlineCache, inlineCaches, size, javascriptLibrary);
 
         return polymorphicInlineCache;
     }
@@ -1029,7 +1022,7 @@ namespace Js
         if (size == 0)
         {
             // Already finalized
-            Assert(!inlineCaches && !prev && !next);
+            Assert(!inlineCaches);
             return;
         }
 
@@ -1063,43 +1056,17 @@ namespace Js
                     unregisteredInlineCacheCount++;
                 }
             }
-
-            AllocatorDeleteArray(InlineCacheAllocator, this->scriptContext->GetInlineCacheAllocator(), size, inlineCaches);
+            AllocatorDeleteArray(InlineCacheAllocator, this->javascriptLibrary->scriptContext->GetInlineCacheAllocator(), size, inlineCaches);
 #ifdef POLY_INLINE_CACHE_SIZE_STATS
-            this->scriptContext->GetInlineCacheAllocator()->LogPolyCacheFree(size * sizeof(InlineCache));
+            this->javascriptLibrary->scriptContext->GetInlineCacheAllocator()->LogPolyCacheFree(size * sizeof(InlineCache));
 #endif
         }
 
-        // Remove this PolymorphicInlineCache from the list
-        if (this == this->scriptContext->GetGlobalPICHead())
-        {
-            Assert(!prev);
-            if (next)
-            {
-                Assert(next->prev == this);
-                next->prev = nullptr;
-            }
-            this->scriptContext->SetGlobalPICHead(next);
-        }
-        else
-        {
-            if (prev)
-            {
-                Assert(prev->next == this);
-                prev->next = next;
-            }
-            if (next)
-            {
-                Assert(next->prev == this);
-                next->prev = prev;
-            }
-        }
-        prev = next = nullptr;
         inlineCaches = nullptr;
         size = 0;
         if (unregisteredInlineCacheCount > 0)
         {
-            this->scriptContext->GetThreadContext()->NotifyInlineCacheBatchUnregistered(unregisteredInlineCacheCount);
+            this->javascriptLibrary->scriptContext->GetThreadContext()->NotifyInlineCacheBatchUnregistered(unregisteredInlineCacheCount);
         }
     }
 
@@ -1146,7 +1113,7 @@ namespace Js
 
     ScriptContext* ScriptContextPolymorphicInlineCache::GetScriptContext() const
     {
-        return this->scriptContext;
+        return this->javascriptLibrary->scriptContext;
     }
 #endif
 
diff --git a/lib/Runtime/Language/InlineCache.h b/lib/Runtime/Language/InlineCache.h
@@ -543,7 +543,7 @@ namespace Js
     class FunctionBodyPolymorphicInlineCache sealed : public PolymorphicInlineCache
     {
     private:
-        Field(FunctionBody *) functionBody;
+        FunctionBody * functionBody;
 
         // DList chaining all polymorphic inline caches of a FunctionBody together.
         // Since PolymorphicInlineCache is a leaf object, these references do not keep
@@ -552,8 +552,8 @@ namespace Js
         // We maintain this linked list of polymorphic caches because when we allocate a larger cache,
         // the old one might still be used by some code on the stack.  Consequently, we can't release
         // the inline cache array back to the arena allocator.
-        Field(FunctionBodyPolymorphicInlineCache *) next;
-        Field(FunctionBodyPolymorphicInlineCache *) prev;
+        FunctionBodyPolymorphicInlineCache * next;
+        FunctionBodyPolymorphicInlineCache * prev;
 
         FunctionBodyPolymorphicInlineCache(InlineCache * inlineCaches, uint16 size, FunctionBody * functionBody)
             : PolymorphicInlineCache(inlineCaches, size), functionBody(functionBody), next(nullptr), prev(nullptr)
@@ -575,20 +575,17 @@ namespace Js
     class ScriptContextPolymorphicInlineCache sealed : public PolymorphicInlineCache
     {
     private:
-        FieldNoBarrier(ScriptContext *) scriptContext;
+        Field(JavascriptLibrary*) javascriptLibrary;
 
-        Field(ScriptContextPolymorphicInlineCache *) next;
-        Field(ScriptContextPolymorphicInlineCache *) prev;
-
-        ScriptContextPolymorphicInlineCache(InlineCache * inlineCaches, uint16 size, ScriptContext * scriptContext)
-            : PolymorphicInlineCache(inlineCaches, size), scriptContext(scriptContext), next(nullptr), prev(nullptr)
+        ScriptContextPolymorphicInlineCache(InlineCache * inlineCaches, uint16 size, JavascriptLibrary * javascriptLibrary)
+            : PolymorphicInlineCache(inlineCaches, size), javascriptLibrary(javascriptLibrary)
         {
             Assert((size == 0 && inlineCaches == nullptr) ||
                 (inlineCaches != nullptr && size >= MinPropertyStringInlineCacheSize && size <= MaxPolymorphicInlineCacheSize));
         }
 
     public:
-        static ScriptContextPolymorphicInlineCache * New(uint16 size, ScriptContext * scriptContext);
+        static ScriptContextPolymorphicInlineCache * New(uint16 size, JavascriptLibrary * javascriptLibrary);
 
 #ifdef INLINE_CACHE_STATS
         virtual void PrintStats(InlineCacheData *data) const override;
diff --git a/lib/Runtime/Library/JavascriptLibrary.h b/lib/Runtime/Library/JavascriptLibrary.h
@@ -66,7 +66,6 @@ namespace Js
     struct Cache
     {
         Field(PropertyStringMap*) propertyStrings[80];
-        Field(ScriptContextPolymorphicInlineCache *) globalPICHead;
         Field(JavascriptString *) lastNumberToStringRadix10String;
         Field(EnumeratedObjectCache) enumObjCache;
         Field(JavascriptString *) lastUtcTimeFromStrString;
diff --git a/lib/Runtime/Library/PropertyString.cpp b/lib/Runtime/Library/PropertyString.cpp
@@ -21,8 +21,9 @@ namespace Js
     PropertyString* PropertyString::New(StaticType* type, const Js::PropertyRecord* propertyRecord, Recycler *recycler)
     {
         PropertyString * propertyString = RecyclerNewZ(recycler, PropertyString, type, propertyRecord);
-        propertyString->ldElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetScriptContext());
-        propertyString->stElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetScriptContext());
+        // TODO: in future, might be worth putting these inline to avoid extra allocations. PIC copy API needs to be updated to support this though
+        propertyString->ldElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetLibrary());
+        propertyString->stElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetLibrary());
         return propertyString;
     }
 
@@ -68,7 +69,7 @@ namespace Js
         uint16 polymorphicInlineCacheSize = polymorphicInlineCache->GetSize();
         uint16 newPolymorphicInlineCacheSize = PolymorphicInlineCache::GetNextSize(polymorphicInlineCacheSize);
         Assert(newPolymorphicInlineCacheSize > polymorphicInlineCacheSize);
-        PolymorphicInlineCache * newPolymorphicInlineCache = ScriptContextPolymorphicInlineCache::New(newPolymorphicInlineCacheSize, GetScriptContext());
+        PolymorphicInlineCache * newPolymorphicInlineCache = ScriptContextPolymorphicInlineCache::New(newPolymorphicInlineCacheSize, GetLibrary());
         polymorphicInlineCache->CopyTo(this->propertyRecord->GetPropertyId(), GetScriptContext(), newPolymorphicInlineCache);
         if (isLdElem)
         {

Original file line number	Diff line number	Diff line change
`@@ -256,8 +256,6 @@ namespace Js`
`256`	`256`	`memset(byteCodeHistogram, 0, sizeof(byteCodeHistogram));`
`257`	`257`	`#endif`
`258`	`258`
`259`		`- ClearArray(this->Cache()->propertyStrings, 80);`
`260`		`-`
`261`	`259`	`#if DBG \|\| defined(RUNTIME_DATA_COLLECTION)`
`262`	`260`	`this->allocId = threadContext->GetScriptContextCount();`
`263`	`261`	`#endif`
`@@ -415,10 +413,6 @@ namespace Js`
`415`	`413`	`ClearInlineCaches();`
`416`	`414`	`Assert(!this->hasProtoOrStoreFieldInlineCache);`
`417`	`415`	`}`
`418`		`- while (this->GetGlobalPICHead())`
`419`		`- {`
`420`		`- this->GetGlobalPICHead()->Finalize(true);`
`421`		`- }`
`422`	`416`
`423`	`417`	`if (this->hasIsInstInlineCache)`
`424`	`418`	`{`
`@@ -1209,6 +1203,7 @@ namespace Js`
`1209`	`1203`	`#endif`
`1210`	`1204`	`#endif`
`1211`	`1205`
	`1206`	`+ ClearArray(this->Cache()->propertyStrings, 80);`
`1212`	`1207`	`this->Cache()->dynamicRegexMap =`
`1213`	`1208`	`RegexPatternMruMap::New(`
`1214`	`1209`	`recycler,`
`@@ -1597,18 +1592,17 @@ namespace Js`
`1597`	`1592`	`{`
`1598`	`1593`	`const char16* buf = propString->GetBuffer();`
`1599`	`1594`	`const uint i = PropertyStringMap::PStrMapIndex(buf[0]);`
`1600`		`- PropertyStringMap* strMap = this->Cache()->propertyStrings[i];`
`1601`		`- if (strMap == NULL)`
	`1595`	`+ if (this->Cache()->propertyStrings[i] == NULL)`
`1602`	`1596`	`{`
`1603`	`1597`	`InitPropertyStringMap(i);`
`1604`	`1598`	`}`
`1605`	`1599`	`const uint j = PropertyStringMap::PStrMapIndex(buf[1]);`
`1606`		`- if (strMap->strLen2[j] == NULL && !isClosed)`
	`1600`	`+ if (this->Cache()->propertyStrings[i]->strLen2[j] == NULL && !isClosed)`
`1607`	`1601`	`{`
`1608`		`- strMap->strLen2[j] = GetLibrary()->CreatePropertyString(propString);`
	`1602`	`+ this->Cache()->propertyStrings[i]->strLen2[j] = GetLibrary()->CreatePropertyString(propString);`
`1609`	`1603`	`this->TrackPid(propString);`
`1610`	`1604`	`}`
`1611`		`- return strMap->strLen2[j];`
	`1605`	`+ return this->Cache()->propertyStrings[i]->strLen2[j];`
`1612`	`1606`	`}`
`1613`	`1607`
`1614`	`1608`	`PropertyString* ScriptContext::CachePropertyString2(const PropertyRecord* propString)`
`@@ -1683,7 +1677,7 @@ namespace Js`
`1683`	`1677`	`cache->GetInlineCaches()[cache->GetInlineCacheIndexForType(type)].Clear();`
`1684`	`1678`	`}`
`1685`	`1679`	`cache = string->GetStElemInlineCache();`
`1686`		`- if (cache->PretendTryGetProperty(type, &info))`
	`1680`	`+ if (cache->PretendTrySetProperty(type, type, &info))`
`1687`	`1681`	`{`
`1688`	`1682`	`#ifdef ENABLE_DEBUG_CONFIG_OPTIONS`
`1689`	`1683`	`if (PHASE_TRACE1(PropertyStringCachePhase))`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,6 @@ namespace Js`
`66`	`66`	`struct Cache`
`67`	`67`	`{`
`68`	`68`	`Field(PropertyStringMap*) propertyStrings[80];`
`69`		`- Field(ScriptContextPolymorphicInlineCache *) globalPICHead;`
`70`	`69`	`Field(JavascriptString *) lastNumberToStringRadix10String;`
`71`	`70`	`Field(EnumeratedObjectCache) enumObjCache;`
`72`	`71`	`Field(JavascriptString *) lastUtcTimeFromStrString;`
Original file line number	Diff line number	Diff line change
`@@ -21,8 +21,9 @@ namespace Js`
`21`	`21`	`PropertyString* PropertyString::New(StaticType* type, const Js::PropertyRecord* propertyRecord, Recycler *recycler)`
`22`	`22`	`{`
`23`	`23`	`PropertyString * propertyString = RecyclerNewZ(recycler, PropertyString, type, propertyRecord);`
`24`		`- propertyString->ldElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetScriptContext());`
`25`		`- propertyString->stElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetScriptContext());`
	`24`	`+ // TODO: in future, might be worth putting these inline to avoid extra allocations. PIC copy API needs to be updated to support this though`
	`25`	`+ propertyString->ldElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetLibrary());`
	`26`	`+ propertyString->stElemInlineCache = ScriptContextPolymorphicInlineCache::New(MinPropertyStringInlineCacheSize, type->GetLibrary());`
`26`	`27`	`return propertyString;`
`27`	`28`	`}`
`28`	`29`
`@@ -68,7 +69,7 @@ namespace Js`
`68`	`69`	`uint16 polymorphicInlineCacheSize = polymorphicInlineCache->GetSize();`
`69`	`70`	`uint16 newPolymorphicInlineCacheSize = PolymorphicInlineCache::GetNextSize(polymorphicInlineCacheSize);`
`70`	`71`	`Assert(newPolymorphicInlineCacheSize > polymorphicInlineCacheSize);`
`71`		`- PolymorphicInlineCache * newPolymorphicInlineCache = ScriptContextPolymorphicInlineCache::New(newPolymorphicInlineCacheSize, GetScriptContext());`
	`72`	`+ PolymorphicInlineCache * newPolymorphicInlineCache = ScriptContextPolymorphicInlineCache::New(newPolymorphicInlineCacheSize, GetLibrary());`
`72`	`73`	`polymorphicInlineCache->CopyTo(this->propertyRecord->GetPropertyId(), GetScriptContext(), newPolymorphicInlineCache);`
`73`	`74`	`if (isLdElem)`
`74`	`75`	`{`