address cr comments

Author: MikeHolman

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))

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);

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
 

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;

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;

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)
  {