numberToString: re-cache on miss and cache more

Author: obastemur

lib/Runtime/Base/ScriptContext.cpp

@@ -74,7 +74,12 @@ namespace Js
 #ifdef FAULT_INJECTION
         disposeScriptByFaultInjectionEventHandler(nullptr),
 #endif
+
+#ifndef CC_LOW_MEMORY_TARGET
         integerStringMap(this->GeneralAllocator()),
+        integerStringMapCacheMissCount(0),
+        integerStringMapCacheUseCount(0),
+#endif
         guestArena(nullptr),
         raiseMessageToDebuggerFunctionType(nullptr),
         transitionToDebugModeIfFirstSourceFn(nullptr),
@@ -1753,23 +1758,32 @@ namespace Js
 
 // TODO: (obastemur) Could this be dynamic instead of compile time?
 #ifndef CC_LOW_MEMORY_TARGET // we don't need this on a target with low memory
+#define NUMBER_TO_STRING_CACHE_SIZE 1024
+#define NUMBER_TO_STRING_RE_CACHE_LIMIT 1024
+#define NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT 48
         if (!this->integerStringMap.TryGetValue(value, &string))
         {
             // Add the string to hash table cache
-            // Don't add if table is getting too full.  We'll be holding on to
-            // too many strings, and table lookup will become too slow.
-            // TODO: Long term running app, this cache doesn't provide much value?
-            //       i.e. what is the importance of first 512 number to string calls?
-            //       a solution; count the number of times we couldn't use cache
-            //       after cache is full. If it's bigger than X ?? the discard the
-            //       previous cache?
-            if (this->integerStringMap.Count() > 512)
+            // limit the htable size to NUMBER_TO_STRING_CACHE_SIZE and refresh the cache often
+            // however don't re-cache if we didn't use it much! App may not be suitable for caching.
+            // 4% -> NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT is equal to perf loss while we cache the stuff
+            if (integerStringMapCacheMissCount > NUMBER_TO_STRING_RE_CACHE_LIMIT)
+            {
+                integerStringMapCacheMissCount = 0;
+                if (integerStringMapCacheUseCount >= NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT)
+                {
+                    this->integerStringMap.Clear();
+                }
+                integerStringMapCacheUseCount = 0;
+            }
+
+            if (this->integerStringMap.Count() > NUMBER_TO_STRING_CACHE_SIZE)
             {
 #endif
                 // Use recycler memory
                 string = TaggedInt::ToString(value, this);
-
 #ifndef CC_LOW_MEMORY_TARGET
+                integerStringMapCacheMissCount++;
             }
             else
             {
@@ -1781,6 +1795,10 @@ namespace Js
                 this->integerStringMap.AddNew(value, string);
             }
         }
+        else if (integerStringMapCacheUseCount < NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT)
+        {
+            integerStringMapCacheUseCount++;
+        }
 #endif
 
         return string;

lib/Runtime/Base/ScriptContext.h

@@ -830,7 +830,11 @@ namespace Js
         EventHandler disposeScriptByFaultInjectionEventHandler;
 #endif
 
+#ifndef CC_LOW_MEMORY_TARGET
         JsUtil::BaseDictionary<uint, JavascriptString *, ArenaAllocator> integerStringMap;
+        uint integerStringMapCacheMissCount;
+        uint integerStringMapCacheUseCount;
+#endif
 
         double lastNumberToStringRadix10;
         double lastUtcTimeFromStr;