[MERGE #1198 @suwc] Enable ES6IsConcatSpreadable under experimental

Merge pull request #1198 from suwc:build/suwc/buddy

Enable ES6IsConcatSpreadable under experimental flag.
Optimize fastpath for cases where [@@isConcatSpreadable] is undefined by caching in ThreadContext.
Add cache invalidations when users add [@@isConcatSpreadable] properties.
Add unit tests to cover more extended usage scenarios.

lib/Common/ConfigFlagsList.h

@@ -283,6 +283,7 @@ PHASE(All)
         PHASE(InlineCandidate)
         PHASE(InlineHostCandidate)
         PHASE(ScriptFunctionWithInlineCache)
+        PHASE(IsConcatSpreadableCache)
         PHASE(Arena)
         PHASE(ApplyUsage)
         PHASE(ObjectHeaderInlining)
@@ -499,7 +500,12 @@ PHASE(All)
     #define DEFAULT_CONFIG_ES6FunctionNameFull     (false)
 #endif
 #define DEFAULT_CONFIG_ES6Generators           (true)
-#define DEFAULT_CONFIG_ES6IsConcatSpreadable   (false)
+#ifdef COMPILE_DISABLE_ES6IsConcatSpreadable
+    // If ES6IsConcatSpreadable needs to be disabled by compile flag, COMPILE_DISABLE_ES6IsConcatSpreadable should be false
+    #define DEFAULT_CONFIG_ES6IsConcatSpreadable   (false)
+#else
+    #define DEFAULT_CONFIG_ES6IsConcatSpreadable   (false)
+#endif
 #define DEFAULT_CONFIG_ES6Math                 (true)
 #ifdef COMPILE_DISABLE_ES6Module
     // If ES6Module needs to be disabled by compile flag, DEFAULT_CONFIG_ES6Module should be false
@@ -952,7 +958,10 @@ FLAGPR           (Boolean, ES6, ES7ExponentiationOperator, "Enable ES7 exponenti
 FLAGPR_REGOVR_EXP(Boolean, ES6, ES7Builtins            , "Enable ES7 built-ins"                                     , DEFAULT_CONFIG_ES7Builtins)
 FLAGPR           (Boolean, ES6, ES7ValuesEntries       , "Enable ES7 Object.values and Object.entries"              , DEFAULT_CONFIG_ES7ValuesEntries)
 FLAGPR           (Boolean, ES6, ES7TrailingComma       , "Enable ES7 trailing comma in function"                    , DEFAULT_CONFIG_ES7TrailingComma)
-FLAGPR           (Boolean, ES6, ES6IsConcatSpreadable  , "Enable ES6 isConcatSpreadable Symbol"                     , DEFAULT_CONFIG_ES6IsConcatSpreadable)
+#ifndef COMPILE_DISABLE_ES6IsConcatSpreadable
+    #define COMPILE_DISABLE_ES6IsConcatSpreadable 0
+#endif
+FLAGPR_REGOVR_EXP(Boolean, ES6, ES6IsConcatSpreadable  , "Enable ES6 isConcatSpreadable Symbol"                     , DEFAULT_CONFIG_ES6IsConcatSpreadable)
 FLAGPR           (Boolean, ES6, ES6Math                , "Enable ES6 Math extensions"                               , DEFAULT_CONFIG_ES6Math)
 
 #ifndef COMPILE_DISABLE_ES6Module

lib/Runtime/Base/ThreadContext.h

@@ -721,6 +721,8 @@ class ThreadContext sealed :
     typedef JsUtil::BaseDictionary<Js::Var, Js::IsInstInlineCache*, ArenaAllocator> IsInstInlineCacheListMapByFunction;
     IsInstInlineCacheListMapByFunction isInstInlineCacheByFunction;
 
+    Js::IsConcatSpreadableCache isConcatSpreadableCache;
+
     ArenaAllocator prototypeChainEnsuredToHaveOnlyWritableDataPropertiesAllocator;
     DListBase<Js::ScriptContext *> prototypeChainEnsuredToHaveOnlyWritableDataPropertiesScriptContext;
 
@@ -841,6 +843,8 @@ class ThreadContext sealed :
 
     UCrtC99MathApis* GetUCrtC99MathApis() { return &ucrtC99MathApis; }
 
+    Js::IsConcatSpreadableCache* GetIsConcatSpreadableCache() { return &isConcatSpreadableCache; }
+
 #ifdef ENABLE_GLOBALIZATION
     Js::DelayLoadWinRtString *GetWinRTStringLibrary();
 #ifdef ENABLE_PROJECTION

lib/Runtime/Language/InlineCache.h

@@ -964,6 +964,72 @@ namespace Js
         void Unregister(ScriptContext * scriptContext);
     };
 
+    // Two-entry Type-indexed circular cache
+    //   cache IsConcatSpreadable() result unless user-defined [@@isConcatSpreadable] exists
+    class IsConcatSpreadableCache
+    {
+        Type *type0, *type1;
+        int lastAccess;
+        BOOL result0, result1;
+
+    public:
+        IsConcatSpreadableCache() :
+            type0(nullptr),
+            type1(nullptr),
+            result0(FALSE),
+            result1(FALSE),
+            lastAccess(1)
+        {
+        }
+
+        bool TryGetIsConcatSpreadable(Type *type, _Out_ BOOL *result)
+        {
+            Assert(type != nullptr);
+            Assert(result != nullptr);
+
+            *result = FALSE;
+            if (type0 == type)
+            {
+                *result = result0;
+                lastAccess = 0;
+                return true;
+            }
+
+            if (type1 == type)
+            {
+                *result = result1;
+                lastAccess = 1;
+                return true;
+            }
+
+            return false;
+        }
+
+        void CacheIsConcatSpreadable(Type *type, BOOL result)
+        {
+            Assert(type != nullptr);
+
+            if (lastAccess == 0)
+            {
+                type1 = type;
+                result1 = result;
+                lastAccess = 1;
+            }
+            else
+            {
+                type0 = type;
+                result0 = result;
+                lastAccess = 0;
+            }
+        }
+
+        void Invalidate()
+        {
+            type0 = nullptr;
+            type1 = nullptr;
+        }
+    };
+
 #if defined(_M_IX86_OR_ARM32)
     CompileAssert(sizeof(IsInstInlineCache) == 0x10);
 #else

lib/Runtime/Language/JavascriptOperators.cpp

@@ -1357,17 +1357,45 @@ namespace Js
 
         RecyclableObject* instance = RecyclableObject::FromVar(instanceVar);
         ScriptContext* scriptContext = instance->GetScriptContext();
+
+        if (!PHASE_OFF1(IsConcatSpreadableCachePhase))
+        {
+            BOOL retVal = FALSE;
+            Type *instanceType = instance->GetType();
+            IsConcatSpreadableCache *isConcatSpreadableCache = scriptContext->GetThreadContext()->GetIsConcatSpreadableCache();
+
+            if (isConcatSpreadableCache->TryGetIsConcatSpreadable(instanceType, &retVal))
+            {
+                OUTPUT_TRACE(Phase::IsConcatSpreadableCachePhase, _u("IsConcatSpreadableCache hit: %p\n"), instanceType);
+                return retVal;
+            }
+
+            Var spreadable = nullptr;
+            BOOL hasUserDefinedSpreadable = JavascriptOperators::GetProperty(instance, instance, PropertyIds::_symbolIsConcatSpreadable, &spreadable, scriptContext);
+
+            if (hasUserDefinedSpreadable && spreadable != scriptContext->GetLibrary()->GetUndefined())
+            {
+                return JavascriptConversion::ToBoolean(spreadable, scriptContext);
+            }
+
+            retVal = JavascriptOperators::IsArray(instance);
+
+            if (!hasUserDefinedSpreadable)
+            {
+                OUTPUT_TRACE(Phase::IsConcatSpreadableCachePhase, _u("IsConcatSpreadableCache saved: %p\n"), instanceType);
+                isConcatSpreadableCache->CacheIsConcatSpreadable(instanceType, retVal);
+            }
+
+            return retVal;
+        }
+
         Var spreadable = JavascriptOperators::GetProperty(instance, PropertyIds::_symbolIsConcatSpreadable, scriptContext);
         if (spreadable != scriptContext->GetLibrary()->GetUndefined())
         {
             return JavascriptConversion::ToBoolean(spreadable, scriptContext);
         }
-        if (JavascriptOperators::IsArray(instance))
-        {
-            return true;
-        }
-        return false;
 
+        return JavascriptOperators::IsArray(instance);
     }
 
     Var JavascriptOperators::OP_LdCustomSpreadIteratorList(Var aRight, ScriptContext* scriptContext)

lib/Runtime/Library/JavascriptArray.cpp

@@ -2972,6 +2972,11 @@ namespace Js
                         return;
                     }
 
+                    if (length + idxDest > FiftyThirdPowerOfTwoMinusOne) // 2^53-1: from ECMA 22.1.3.1 Array.prototype.concat(...arguments)
+                    {
+                        JavascriptError::ThrowTypeError(scriptContext, JSERR_IllegalArraySizeAndLength);
+                    }
+
                     RecyclableObject* itemObject = RecyclableObject::FromVar(aItem);
                     Var subItem;
                     uint32 lengthToUin32Max = length.IsSmallIndex() ? length.GetSmallIndex() : MaxArrayLength;
@@ -3044,7 +3049,7 @@ namespace Js
         return true;
     }
 
-    void JavascriptArray::ConcatIntArgs(JavascriptNativeIntArray* pDestArray, TypeId *remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext)
+    JavascriptArray* JavascriptArray::ConcatIntArgs(JavascriptNativeIntArray* pDestArray, TypeId *remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext)
     {
         uint idxDest = 0u;
         for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++)
@@ -3058,7 +3063,7 @@ namespace Js
                 if (!JavascriptNativeIntArray::Is(pDestArray)) // SetItem could convert pDestArray to a var array if aItem is not an integer if so fall back
                 {
                     ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg + 1, idxDest);
-                    return;
+                    return pDestArray;
                 }
                 continue;
             }
@@ -3073,13 +3078,11 @@ namespace Js
                     // Copying the last array forced a conversion, so switch over to the var version
                     // to finish.
                     ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg + 1, idxDest);
-                    return;
+                    return pDestArray;
                 }
             }
-            else
+            else if (!JavascriptArray::IsAnyArray(aItem) && remoteTypeIds[idxArg] != TypeIds_Array)
             {
-                Assert(!JavascriptArray::IsAnyArray(aItem) && remoteTypeIds[idxArg] != TypeIds_Array);
-
                 if (TaggedInt::Is(aItem))
                 {
                     pDestArray->DirectSetItemAt(idxDest, TaggedInt::ToInt32(aItem));
@@ -3096,14 +3099,21 @@ namespace Js
                 }
                 ++idxDest;
             }
+            else
+            {
+                JavascriptArray *pVarDestArray = JavascriptNativeIntArray::ConvertToVarArray(pDestArray);
+                ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest);
+                return pVarDestArray;
+            }
         }
         if (pDestArray->GetLength() != idxDest)
         {
             pDestArray->SetLength(idxDest);
         }
+        return pDestArray;
     }
 
-    void JavascriptArray::ConcatFloatArgs(JavascriptNativeFloatArray* pDestArray, TypeId *remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext)
+    JavascriptArray* JavascriptArray::ConcatFloatArgs(JavascriptNativeFloatArray* pDestArray, TypeId *remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext)
     {
         uint idxDest = 0u;
         for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++)
@@ -3119,7 +3129,7 @@ namespace Js
                 if (!JavascriptNativeFloatArray::Is(pDestArray)) // SetItem could convert pDestArray to a var array if aItem is not an integer if so fall back
                 {
                     ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg + 1, idxDest);
-                    return;
+                    return pDestArray;
                 }
                 continue;
             }
@@ -3133,18 +3143,25 @@ namespace Js
                     converted = CopyNativeIntArrayElementsToFloat(pDestArray, idxDest, pIntArray);
                     idxDest = idxDest + pIntArray->length;
                 }
-                else
+                else if (JavascriptNativeFloatArray::Is(aItem))
                 {
                     JavascriptNativeFloatArray* pItemArray = JavascriptNativeFloatArray::FromVar(aItem);
                     converted = CopyNativeFloatArrayElements(pDestArray, idxDest, pItemArray);
                     idxDest = idxDest + pItemArray->length;
                 }
+                else
+                {
+                    JavascriptArray *pVarDestArray = JavascriptNativeFloatArray::ConvertToVarArray(pDestArray);
+                    ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest);
+                    return pVarDestArray;
+                }
+
                 if (converted)
                 {
                     // Copying the last array forced a conversion, so switch over to the var version
                     // to finish.
                     ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg + 1, idxDest);
-                    return;
+                    return pDestArray;
                 }
             }
             else
@@ -3167,6 +3184,8 @@ namespace Js
         {
             pDestArray->SetLength(idxDest);
         }
+
+        return pDestArray;
     }
 
     bool JavascriptArray::BoxConcatItem(Var aItem, uint idxArg, ScriptContext *scriptContext)
@@ -3318,15 +3337,13 @@ namespace Js
             {
                 JavascriptNativeIntArray *pIntArray = isArray ? JavascriptNativeIntArray::FromVar(pDestObj) : scriptContext->GetLibrary()->CreateNativeIntArray(cDestLength);
                 pIntArray->EnsureHead<int32>();
-                ConcatIntArgs(pIntArray, remoteTypeIds, args, scriptContext);
-                pDestArray = pIntArray;
+                pDestArray = ConcatIntArgs(pIntArray, remoteTypeIds, args, scriptContext);
             }
             else if (isFloat)
             {
                 JavascriptNativeFloatArray *pFArray = isArray ? JavascriptNativeFloatArray::FromVar(pDestObj) : scriptContext->GetLibrary()->CreateNativeFloatArray(cDestLength);
                 pFArray->EnsureHead<double>();
-                ConcatFloatArgs(pFArray, remoteTypeIds, args, scriptContext);
-                pDestArray = pFArray;
+                pDestArray = ConcatFloatArgs(pFArray, remoteTypeIds, args, scriptContext);
             }
             else
             {

lib/Runtime/Library/JavascriptArray.h

@@ -116,6 +116,7 @@ namespace Js
         static uint32 const MaxArrayLength = InvalidIndex;
         static uint32 const MaxInitialDenseLength=1<<18;
         static ushort const MergeSegmentsLengthHeuristics = 128; // If the length is less than MergeSegmentsLengthHeuristics then try to merge the segments
+        static uint64 const FiftyThirdPowerOfTwoMinusOne = 0x1FFFFFFFFFFFFF;  // 2^53-1
 
         static const Var MissingItem;
         template<typename T> static T GetMissingItem();
@@ -759,10 +760,10 @@ namespace Js
         static void ConcatArgs(RecyclableObject* pDestObj, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext, uint start, BigIndex startIdxDest, BOOL firstPromotedItemIsSpreadable, BigIndex firstPromotedItemLength);
         template<typename T>
         static void ConcatArgs(RecyclableObject* pDestObj, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext, uint start = 0, uint startIdxDest = 0u, BOOL FirstPromotedItemIsSpreadable = false, BigIndex FirstPromotedItemLength = 0u);
-        static void ConcatIntArgs(JavascriptNativeIntArray* pDestArray, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext);
+        static JavascriptArray* ConcatIntArgs(JavascriptNativeIntArray* pDestArray, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext);
         static bool PromoteToBigIndex(BigIndex lhs, BigIndex rhs);
         static bool PromoteToBigIndex(BigIndex lhs, uint32 rhs);
-        static void ConcatFloatArgs(JavascriptNativeFloatArray* pDestArray, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext);
+        static JavascriptArray* ConcatFloatArgs(JavascriptNativeFloatArray* pDestArray, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext);
     private:
         template<typename T=uint32>
         static RecyclableObject* ArraySpeciesCreate(Var pThisArray, T length, ScriptContext* scriptContext, bool* pIsIntArray = nullptr, bool* pIsFloatArray = nullptr);

lib/Runtime/Library/JavascriptLibrary.h

@@ -1037,6 +1037,8 @@ namespace Js
         RecyclableObject* TryGetStringTemplateCallsiteObject(ParseNodePtr pnode);
         RecyclableObject* TryGetStringTemplateCallsiteObject(RecyclableObject* callsite);
 
+        static void CheckAndInvalidateIsConcatSpreadableCache(PropertyId propertyId, ScriptContext *scriptContext);
+
 #if DBG_DUMP
         static const char16* GetStringTemplateCallsiteObjectKey(Var callsite);
 #endif

lib/Runtime/Library/JavascriptLibrary.inl

@@ -33,6 +33,15 @@ namespace Js
         return AddFunctionToLibraryObject(object, scriptContext->GetOrAddPropertyIdTracked(propertyName), functionInfo, length);
     }
 
+    inline void JavascriptLibrary::CheckAndInvalidateIsConcatSpreadableCache(PropertyId propertyId, ScriptContext *scriptContext)
+    {
+        if (!PHASE_OFF1(IsConcatSpreadableCachePhase) && propertyId == PropertyIds::_symbolIsConcatSpreadable)
+        {
+            OUTPUT_TRACE(Phase::IsConcatSpreadableCachePhase, _u("IsConcatSpreadableCache invalidated\n"));
+            scriptContext->GetThreadContext()->GetIsConcatSpreadableCache()->Invalidate();
+        }
+    }
+
 #if ENABLE_COPYONACCESS_ARRAY
     template <>
     inline void JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray(const Var instance)

lib/Runtime/Library/JavascriptProxy.cpp

@@ -321,6 +321,9 @@ namespace Js
                 return FALSE;
             JavascriptError::ThrowTypeError(scriptContext, JSERR_ErrorOnRevokedProxy, _u("get"));
         }
+
+        RecyclableObject *target = this->target;
+
         JavascriptFunction* getGetMethod = GetMethodHelper(PropertyIds::get, scriptContext);
         Var getGetResult;
         if (nullptr == getGetMethod || scriptContext->IsHeapEnumInProgress())

lib/Runtime/Types/DictionaryTypeHandler.cpp

@@ -779,6 +779,8 @@ namespace Js
         bool throwIfNotExtensible = (flags & (PropertyOperation_ThrowIfNotExtensible | PropertyOperation_StrictMode)) != 0;
         bool isForce = (flags & PropertyOperation_Force) != 0;
 
+        JavascriptLibrary::CheckAndInvalidateIsConcatSpreadableCache(propertyId, scriptContext);
+
         Assert(propertyId != Constants::NoProperty);
         PropertyRecord const* propertyRecord = scriptContext->GetPropertyName(propertyId);
         if (propertyMap->TryGetReference(propertyRecord, &descriptor))

lib/Runtime/Types/PathTypeHandler.cpp

@@ -252,6 +252,8 @@ namespace Js
         Assert(value != nullptr || IsInternalPropertyId(propertyId));
         PropertyIndex index = PathTypeHandlerBase::GetPropertyIndex(propertyId);
 
+        JavascriptLibrary::CheckAndInvalidateIsConcatSpreadableCache(propertyId, instance->GetScriptContext());
+
         if (index != Constants::NoSlot)
         {
             // If type is shared then the handler must be shared as well.  This is a weaker invariant than in AddPropertyInternal,

lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp

@@ -1302,6 +1302,8 @@ namespace Js
         Assert(propertyId != Constants::NoProperty);
         PropertyRecord const* propertyRecord = instance->GetScriptContext()->GetPropertyName(propertyId);
 
+        JavascriptLibrary::CheckAndInvalidateIsConcatSpreadableCache(propertyId, instance->GetScriptContext());
+
         // It can be the case that propertyRecord is a symbol property and it has the same string description as
         // another property which is in the propertyMap. If we are in a string-keyed type handler, the propertyMap
         // will find that normal property when we call TryGetReference with the symbol propertyRecord. However,