Skip to content

Commit 112d0d6

Browse files
committed
[MERGE #4004 @obastemur] numberToString: re-cache on miss and cache more
Merge pull request #4004 from obastemur:bet_num_str_stuff Improve numberToString cache for long running applications while adding more space to hash table cap. ``` // 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 ``` pros; - no point holding a cache that we may never use again cons; - if the application is generating 100% distinct NUMBER_TO_STRING_CACHE_SIZE+1 numbers and stringify them all the time.. well, the perf could be ~4% slower comparing to now. This is due to cache won't have any use yet we will be filling it up again and again. In order to prevent that, we will check the cache usage and do not re-cache if we didn't even use it.
2 parents 345870f + d641f7c commit 112d0d6

File tree

2 files changed

+31
-9
lines changed

2 files changed

+31
-9
lines changed

lib/Runtime/Base/ScriptContext.cpp

+27-9
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,12 @@ namespace Js
7474
#ifdef FAULT_INJECTION
7575
disposeScriptByFaultInjectionEventHandler(nullptr),
7676
#endif
77+
78+
#ifndef CC_LOW_MEMORY_TARGET
7779
integerStringMap(this->GeneralAllocator()),
80+
integerStringMapCacheMissCount(0),
81+
integerStringMapCacheUseCount(0),
82+
#endif
7883
guestArena(nullptr),
7984
raiseMessageToDebuggerFunctionType(nullptr),
8085
transitionToDebugModeIfFirstSourceFn(nullptr),
@@ -1753,23 +1758,32 @@ namespace Js
17531758

17541759
// TODO: (obastemur) Could this be dynamic instead of compile time?
17551760
#ifndef CC_LOW_MEMORY_TARGET // we don't need this on a target with low memory
1761+
#define NUMBER_TO_STRING_CACHE_SIZE 1024
1762+
#define NUMBER_TO_STRING_RE_CACHE_LIMIT 1024
1763+
#define NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT 48
17561764
if (!this->integerStringMap.TryGetValue(value, &string))
17571765
{
17581766
// Add the string to hash table cache
1759-
// Don't add if table is getting too full. We'll be holding on to
1760-
// too many strings, and table lookup will become too slow.
1761-
// TODO: Long term running app, this cache doesn't provide much value?
1762-
// i.e. what is the importance of first 512 number to string calls?
1763-
// a solution; count the number of times we couldn't use cache
1764-
// after cache is full. If it's bigger than X ?? the discard the
1765-
// previous cache?
1766-
if (this->integerStringMap.Count() > 512)
1767+
// limit the htable size to NUMBER_TO_STRING_CACHE_SIZE and refresh the cache often
1768+
// however don't re-cache if we didn't use it much! App may not be suitable for caching.
1769+
// 4% -> NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT is equal to perf loss while we cache the stuff
1770+
if (integerStringMapCacheMissCount > NUMBER_TO_STRING_RE_CACHE_LIMIT)
1771+
{
1772+
integerStringMapCacheMissCount = 0;
1773+
if (integerStringMapCacheUseCount >= NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT)
1774+
{
1775+
this->integerStringMap.Clear();
1776+
}
1777+
integerStringMapCacheUseCount = 0;
1778+
}
1779+
1780+
if (this->integerStringMap.Count() > NUMBER_TO_STRING_CACHE_SIZE)
17671781
{
17681782
#endif
17691783
// Use recycler memory
17701784
string = TaggedInt::ToString(value, this);
1771-
17721785
#ifndef CC_LOW_MEMORY_TARGET
1786+
integerStringMapCacheMissCount++;
17731787
}
17741788
else
17751789
{
@@ -1781,6 +1795,10 @@ namespace Js
17811795
this->integerStringMap.AddNew(value, string);
17821796
}
17831797
}
1798+
else if (integerStringMapCacheUseCount < NUMBER_TO_STRING_RE_CACHE_REASON_LIMIT)
1799+
{
1800+
integerStringMapCacheUseCount++;
1801+
}
17841802
#endif
17851803

17861804
return string;

lib/Runtime/Base/ScriptContext.h

+4
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,11 @@ namespace Js
830830
EventHandler disposeScriptByFaultInjectionEventHandler;
831831
#endif
832832

833+
#ifndef CC_LOW_MEMORY_TARGET
833834
JsUtil::BaseDictionary<uint, JavascriptString *, ArenaAllocator> integerStringMap;
835+
uint integerStringMapCacheMissCount;
836+
uint integerStringMapCacheUseCount;
837+
#endif
834838

835839
double lastNumberToStringRadix10;
836840
double lastUtcTimeFromStr;

0 commit comments

Comments
 (0)