Checkpoint - updates to concat string cache

lib/Backend/JnHelperMethodList.h

@@ -99,6 +99,7 @@ HELPERCALL_FULL_OR_INPLACE_MATH(Op_Negate, Js::JavascriptMath::Negate, Js::SSE2:
 HELPERCALL_FULL_OR_INPLACE_MATH(Op_Not, Js::JavascriptMath::Not, Js::SSE2::JavascriptMath::Not, AttrCanThrow)
 
 HELPERCALL_MATH(Op_AddLeftDead, Js::JavascriptMath::AddLeftDead, Js::SSE2::JavascriptMath::AddLeftDead, AttrCanThrow)
+HELPERCALL(Op_AddString, Js::JavascriptString::AddString, AttrCanThrow)
 HELPERCALL_FULL_OR_INPLACE_MATH(Op_Add, Js::JavascriptMath::Add, Js::SSE2::JavascriptMath::Add, AttrCanThrow)
 HELPERCALL_FULL_OR_INPLACE_MATH(Op_Divide, Js::JavascriptMath::Divide, Js::SSE2::JavascriptMath::Divide, AttrCanThrow)
 HELPERCALL_FULL_OR_INPLACE_MATH(Op_Modulus, Js::JavascriptMath::Modulus, Js::SSE2::JavascriptMath::Modulus, AttrCanThrow)

lib/Backend/Lower.cpp

@@ -1269,6 +1269,13 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
                 Assert(instr->GetDst()->GetType() == instr->GetSrc2()->GetType());
                 m_lowererMD.LowerToFloat(instr);
             }
+            else if (
+                !PHASE_OFF(Js::ConcatStringCachePhase, this->m_func) &&
+                instr->GetSrc1()->GetValueType().IsLikelyString() &&
+                instr->GetSrc2()->GetValueType().IsLikelyString())
+            {
+                this->LowerBinaryHelperMem(instr, IR::HelperOp_AddString);
+            }
             else if (PHASE_OFF(Js::MathFastPathPhase, this->m_func) || noMathFastPath)
             {
                 this->LowerBinaryHelperMem(instr, IR::HelperOp_Add);

lib/Common/ConfigFlagsList.h

@@ -304,6 +304,7 @@ PHASE(All)
         PHASE(PropertyStringCache)
         PHASE(CloneCacheInCollision)
         PHASE(ConstructorCache)
+        PHASE(ConcatStringCache)
         PHASE(InlineCandidate)
         PHASE(InlineHostCandidate)
         PHASE(ScriptFunctionWithInlineCache)
@@ -404,6 +405,7 @@ PHASE(All)
 #define DEFAULT_CONFIG_BgJitPendingFuncCap  (31)
 #define DEFAULT_CONFIG_CurrentSourceInfo    (true)
 #define DEFAULT_CONFIG_CreateFunctionProxy  (true)
+#define DEFAULT_CONFIG_ConcatCacheSize      (16)
 #define DEFAULT_CONFIG_HybridFgJit          (false)
 #define DEFAULT_CONFIG_HybridFgJitBgQueueLengthThreshold (32)
 #define DEFAULT_CONFIG_Prejit               (false)
@@ -898,6 +900,8 @@ FLAGNR(Number,  BgJitDelayFgBuffer    , "When speculatively jitting in the foreg
 FLAGNR(Number,  BgJitPendingFuncCap   , "Disable delay if pending function count larger then cap", DEFAULT_CONFIG_BgJitPendingFuncCap)
 
 FLAGNR(Boolean, CreateFunctionProxy   , "Create function proxies instead of full function bodies", DEFAULT_CONFIG_CreateFunctionProxy)
+FLAGNR(Number,  ConcatStringCacheSize , "Size of the cache for string concat results", DEFAULT_CONFIG_ConcatCacheSize)
+
 FLAGNR(Boolean, HybridFgJit           , "When background JIT is enabled, enable jitting in the foreground based on heuristics. This flag is only effective when OptimizeForManyInstances is disabled (UI threads).", DEFAULT_CONFIG_HybridFgJit)
 FLAGNR(Number,  HybridFgJitBgQueueLengthThreshold, "The background job queue length must exceed this threshold to consider jitting in the foreground", DEFAULT_CONFIG_HybridFgJitBgQueueLengthThreshold)
 FLAGNR(Boolean, BytecodeHist          , "Provide a histogram of the bytecodes run by the script. (NoNative required).", false)

lib/Common/DataStructures/MruDictionary.h

@@ -201,6 +201,13 @@ namespace JsUtil
                 AddToDictionary(entry);
         }
 
+        void Clear()
+        {
+            dictionary.Clear();
+            entries.Clear();
+            mruListCount = 0;
+        }
+
         PREVENT_COPY(MruDictionary);
     };
 }

lib/Runtime/Base/Chakra.Runtime.Base.vcxproj

@@ -77,6 +77,7 @@
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="CompactCounters.h" />
+    <ClInclude Include="ConcatStringCache.h" />
     <ClInclude Include="ittnotify_config.h" />
     <ClInclude Include="ittnotify_types.h" />
     <ClInclude Include="jitprofiling.h" />
@@ -130,4 +131,4 @@
   </ItemGroup>
   <Import Project="$(BuildConfigPropsPath)Chakra.Build.targets" Condition="exists('$(BuildConfigPropsPath)Chakra.Build.targets')" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
-</Project>
+</Project>

lib/Runtime/Base/ConcatStringCache.h

@@ -0,0 +1,30 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+#pragma once
+
+namespace Js
+{
+    struct ConcatStringCacheKey
+    {
+        Js::JavascriptString* left;
+        Js::JavascriptString* right;
+    };
+
+    typedef JsUtil::MruDictionary<ConcatStringCacheKey, Js::JavascriptString*> ConcatStringCache;
+};
+
+template <>
+struct DefaultComparer<Js::ConcatStringCacheKey>
+{
+    inline static bool Equals(Js::ConcatStringCacheKey x, Js::ConcatStringCacheKey y)
+    {
+        return x.left == y.left && x.right == y.right;
+    }
+
+    inline static hash_t GetHashCode(Js::ConcatStringCacheKey d)
+    {
+        return (HeapBlockMap32::GetLevel2Id(d.left) << 16 | HeapBlockMap32::GetLevel2Id(d.right));
+    }
+};

lib/Runtime/Base/ScriptContext.cpp

@@ -1222,6 +1222,8 @@ namespace Js
         srcInfo->sourceContextInfo = this->Cache()->noContextSourceContextInfo;
         srcInfo->moduleID = kmodGlobal;
         this->Cache()->noContextGlobalSourceInfo = srcInfo;
+
+        this->Cache()->concatStringCache = nullptr;
     }
 
     void ScriptContext::InitializePreGlobal()
@@ -4485,6 +4487,9 @@ namespace Js
             GetDynamicRegexMap()->RemoveRecentlyUnusedItems();
         }
 
+        // This list is on the arena so after a GC, it could
+        // contain invalid items. For simplicity, just clear the list
+        Cache()->concatStringCache = nullptr;
         CleanSourceListInternal(true);
     }
 
@@ -5064,6 +5069,46 @@ void ScriptContext::RegisterPrototypeChainEnsuredToHaveOnlyWritableDataPropertie
         return this->charClassifier;
     }
 
+    Js::JavascriptString* ScriptContext::GetConcatCacheString(const Js::ConcatStringCacheKey& key)
+    {
+        auto concatCache = Cache()->concatStringCache;
+        if (concatCache != nullptr)
+        {
+            Js::JavascriptString* value;
+            if (concatCache->TryGetValue(key, &value))
+            {
+                PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"Cache hit [(0x%p, 0x%p), 0x%p]\n", key.left, key.right, value);
+                PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"\tLeft: %s\n", key.left->GetSz());
+                PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"\tRight: %s\n", key.right->GetSz());
+                return value;
+            }
+        }
+
+        return nullptr;
+    }
+
+    void ScriptContext::AddConcatCacheString(const Js::ConcatStringCacheKey& key, Js::JavascriptString* concat)
+    {
+        auto concatCache = Cache()->concatStringCache;
+        if (concatCache == nullptr)
+        {
+#if ENABLE_DEBUG_CONFIG_OPTIONS
+            const int maxCacheSize = CONFIG_FLAG(ConcatStringCacheSize);
+#else
+            const int maxCacheSize = 16;
+#endif
+            concatCache = Js::ConcatStringCache::New(this->recycler, 
+                maxCacheSize <= 0 ? 16 : maxCacheSize);
+            Cache()->concatStringCache = concatCache;
+        }
+
+        // TODO: confirm that GeneralAllocator::Alloc throws
+        concatCache->Add(key, concat);
+        concatCache->RemoveRecentlyUnusedItems();
+
+        PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"Cache add [(0x%p, 0x%p), 0x%p]\n", key.left, key.right, concat);
+    }
+
     void ScriptContext::OnStartupComplete()
     {
         JS_ETW(EventWriteJSCRIPT_ON_STARTUP_COMPLETE(this));

lib/Runtime/Base/ScriptContext.h

@@ -1044,6 +1044,9 @@ namespace Js
         ScriptConfiguration const * GetConfig(void) const { return &config; }
         CharClassifier const * GetCharClassifier(void) const;
 
+        Js::JavascriptString* GetConcatCacheString(const Js::ConcatStringCacheKey& key);
+        void AddConcatCacheString(const Js::ConcatStringCacheKey& key, Js::JavascriptString* concat);
+
         ThreadContext * GetThreadContext() const { return threadContext; }
 
         static const int MaxEvalSourceSize = 400;

lib/Runtime/Library/ConcatString.cpp

@@ -29,7 +29,8 @@ namespace Js
 
     /////////////////////// ConcatStringBase //////////////////////////
 
-    ConcatStringBase::ConcatStringBase(StaticType* stringType) : LiteralString(stringType)
+    ConcatStringBase::ConcatStringBase(StaticType* stringType) : LiteralString(stringType),
+        hasOnlyLiterals(true)
     {
     }
 
@@ -118,6 +119,12 @@ namespace Js
         m_slots[0] = a;
         m_slots[1] = b;
 
+        if (!VirtualTableInfo<Js::LiteralString>::HasVirtualTable(a) ||
+            !VirtualTableInfo<Js::LiteralString>::HasVirtualTable(b))
+        {
+            this->SetHasNonLiteral();
+        }
+
         this->SetLength(a->GetLength() + b->GetLength()); // does not include null character
     }
 
@@ -234,6 +241,10 @@ namespace Js
 
         len += str->GetLength();
         this->SetLength(len);
+        if (!VirtualTableInfo<Js::LiteralString>::HasVirtualTable(str))
+        {
+            this->SetHasNonLiteral();
+        }
     }
 
     // Allocate slots, set m_slots and m_slotCount.
@@ -376,6 +387,11 @@ namespace Js
         value = CompoundString::GetImmutableOrScriptUnreferencedString(value);
         this->SetLength(this->GetLength() + value->GetLength());
         m_slots[index] = value;
+
+        if (!VirtualTableInfo<Js::LiteralString>::HasVirtualTable(value))
+        {
+            this->SetHasNonLiteral();
+        }
     }
 
 #if DBG

lib/Runtime/Library/ConcatString.h

@@ -44,6 +44,8 @@ namespace Js
     class ConcatStringBase _ABSTRACT : public LiteralString // vtable will be switched to LiteralString's vtable after flattening
     {
         friend JavascriptString;
+    private:
+        bool hasOnlyLiterals;
 
     protected:
         ConcatStringBase(StaticType* stringTypeStatic);
@@ -61,6 +63,16 @@ namespace Js
         virtual const char16* GetSz() = 0;     // Force subclass to call GetSzImpl with the real type to avoid virtual calls
         using JavascriptString::Copy;
         virtual bool IsTree() const override sealed;
+
+        bool HasOnlyLiterals()
+        {
+            return hasOnlyLiterals;
+        }
+
+        void SetHasNonLiteral()
+        {
+            hasOnlyLiterals = false;
+        }
     };
 
     // Concat string with N (or less) child nodes.

lib/Runtime/Library/ConcatString.inl

@@ -81,6 +81,11 @@ namespace Js
         m_slots[index] = value;
         charcount_t newTotalLen = this->GetLength() - oldItemLen + newItemLen;
         this->SetLength(newTotalLen);
+
+        if (!VirtualTableInfo<Js::LiteralString>::HasVirtualTable(value))
+        {
+            this->SetHasNonLiteral();
+        }
     }
 
     template<int N>

lib/Runtime/Library/JavascriptLibrary.h

@@ -84,6 +84,7 @@ namespace Js
         Field(DynamicProfileInfoList*) profileInfoList;
 #endif
 #endif
+        Field(ConcatStringCache *) concatStringCache;
     };
 
     class MissingPropertyTypeHandler;

lib/Runtime/Library/JavascriptString.cpp

@@ -12,6 +12,8 @@
 #include "../Backend/JITRecyclableObject.h"
 #endif
 
+#include "JavascriptMath.h"
+
 namespace Js
 {
     // White Space characters are defined in ES 2017 Section 11.2 #sec-white-space
@@ -2739,6 +2741,40 @@ namespace Js
         return string->GetLength() == 2 && wmemcmp(string->GetString(), _u("-0"), 2) == 0;
     }
 
+    Var JavascriptString::AddString(Var string1, Var string2, Js::ScriptContext* scriptContext)
+    {
+        if (RecyclableObject::Is(string1) &&
+            RecyclableObject::Is(string2))
+        {
+            if (VirtualTableInfo<Js::LiteralString>::HasVirtualTable(string1) &&
+                // VirtualTableInfo<Js::LiteralString>::HasVirtualTable(string2) &&
+                JavascriptString::Is(string1) &&
+                JavascriptString::Is(string2))
+            {
+                Js::ConcatStringCacheKey key;
+                key.left = Js::JavascriptString::FromVar(string1);
+                key.right = Js::JavascriptString::FromVar(string2);
+
+                Js::JavascriptString* concatString = scriptContext->GetConcatCacheString(key);
+
+                if (concatString == nullptr)
+                {
+                    PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"Cache miss [(0x%p, 0x%p)]\n", string1, string2);
+                    PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"\tLeft: %s\n", key.left->GetSz());
+                    PHASE_PRINT_TRACE1(Js::ConcatStringCachePhase, L"\tRight: %s\n", key.right->GetSz());
+                    concatString = JavascriptString::Concat(key.left, key.right);
+                    scriptContext->AddConcatCacheString(key, concatString);
+                }
+
+                Assert(concatString != nullptr);
+
+                return concatString;
+            }
+        }
+
+        return JavascriptMath::Add(string1, string2, scriptContext);
+    }
+
     void JavascriptString::FinishCopy(__inout_xcount(m_charLength) char16 *const buffer, StringCopyInfoStack &nestedStringTreeCopyInfos)
     {
         while (!nestedStringTreeCopyInfos.IsEmpty())

lib/Runtime/Library/JavascriptString.h

@@ -145,6 +145,7 @@ namespace Js
         static bool Equals(Var aLeft, Var aRight);
         static bool LessThan(Var aLeft, Var aRight);
         static bool IsNegZero(JavascriptString *string);
+        static Var  AddString(Var string1, Var string2, Js::ScriptContext* scriptContext);
 
         static uint strstr(JavascriptString *string, JavascriptString *substring, bool useBoyerMoore, uint start=0);
         static int strcmp(JavascriptString *string1, JavascriptString *string2);

lib/Runtime/Runtime.h

@@ -437,6 +437,7 @@ enum tagDEBUG_EVENT_INFO_TYPE
 #include "Library/CustomExternalIterator.h"
 
 #include "Base/CharStringCache.h"
+#include "Base/ConcatStringCache.h"
 
 #include "Library/JavascriptObject.h"
 #include "Library/BuiltInFlags.h"