Skip to content

Commit 5ac0a3d

Browse files
MikeHolmanMikeHolman
committed
[MERGE #3802 @MikeHolman] Add cache for RegExp.test
Merge pull request #3802 from MikeHolman:regextest It seems to be a common enough pattern where people will create a RegExp and test a small number of unique strings many times. For this, I added a simple cache of size 8 on the RegExp object for RegExp.test, which caches the result for some given input string. I disable the cache it in case of using global or sticky, since the result can change depending on lastIndex and caching likely wouldn't help those patterns. After experimentation I decided on size 8, because there were rapidly diminishing returns at size 16 and at size 4 I saw a big increase in cache misses. I tried dynamic sizing as well, but megamorphic cases would quickly max out the cache and the polymorphic cases didn't seem to get much benefit above size 8, so it seemed better to avoid the overhead of resizing and stick with a fixed sized cache. Improves perf of angular by about 6%.
2 parents 5f22a9a + 94491e8 commit 5ac0a3d

15 files changed

+156
-111
lines changed

‎lib/Runtime/Base/ScriptContext.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ namespace Js
7878
#ifdef FAULT_INJECTION
7979
disposeScriptByFaultInjectionEventHandler(nullptr),
8080
#endif
81-
integerStringMap(this->GeneralAllocator()),
8281
guestArena(nullptr),
8382
#ifdef ENABLE_SCRIPT_DEBUGGING
8483
diagnosticArena(nullptr),
@@ -1760,7 +1759,12 @@ namespace Js
17601759

17611760
// TODO: (obastemur) Could this be dynamic instead of compile time?
17621761
#ifndef CC_LOW_MEMORY_TARGET // we don't need this on a target with low memory
1763-
if (!this->integerStringMap.TryGetValue(value, &string))
1762+
if (this->Cache()->integerStringMap == nullptr)
1763+
{
1764+
this->Cache()->integerStringMap = RecyclerNew(GetRecycler(), StringMap, GetRecycler());
1765+
}
1766+
StringMap * integerStringMap = this->Cache()->integerStringMap;
1767+
if (!integerStringMap->TryGetValue(value, &string))
17641768
{
17651769
// Add the string to hash table cache
17661770
// Don't add if table is getting too full. We'll be holding on to
@@ -1770,7 +1774,7 @@ namespace Js
17701774
// a solution; count the number of times we couldn't use cache
17711775
// after cache is full. If it's bigger than X ?? the discard the
17721776
// previous cache?
1773-
if (this->integerStringMap.Count() > 512)
1777+
if (integerStringMap->Count() > 512)
17741778
{
17751779
#endif
17761780
// Use recycler memory
@@ -1783,9 +1787,8 @@ namespace Js
17831787
char16 stringBuffer[22];
17841788

17851789
int pos = TaggedInt::ToBuffer(value, stringBuffer, _countof(stringBuffer));
1786-
string = JavascriptString::NewCopySzFromArena(stringBuffer + pos,
1787-
this, this->GeneralAllocator(), (_countof(stringBuffer) - 1) - pos);
1788-
this->integerStringMap.AddNew(value, string);
1790+
string = JavascriptString::NewCopyBuffer(stringBuffer + pos, (_countof(stringBuffer) - 1) - pos, this);
1791+
integerStringMap->AddNew(value, string);
17891792
}
17901793
}
17911794
#endif

‎lib/Runtime/Base/ScriptContext.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,8 +835,6 @@ namespace Js
835835
EventHandler disposeScriptByFaultInjectionEventHandler;
836836
#endif
837837

838-
JsUtil::BaseDictionary<uint, JavascriptString *, ArenaAllocator> integerStringMap;
839-
840838
double lastNumberToStringRadix10;
841839
double lastUtcTimeFromStr;
842840

‎lib/Runtime/Debug/DiagObjectModel.cpp

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,13 @@ namespace Js
270270
}
271271
else
272272
{
273-
Js::JavascriptString *builtInName = ParseFunctionName(returnValue->calledFunction->GetDisplayName(), pResolvedObject->scriptContext);
274-
swprintf_s(finalName, RETURN_VALUE_MAX_NAME, _u("[%s returned]"), builtInName->GetSz());
273+
ENTER_PINNED_SCOPE(JavascriptString, displayName);
274+
displayName = returnValue->calledFunction->GetDisplayName();
275+
276+
const char16 *builtInName = ParseFunctionName(displayName->GetString(), displayName->GetLength(), pResolvedObject->scriptContext);
277+
swprintf_s(finalName, RETURN_VALUE_MAX_NAME, _u("[%s returned]"), builtInName);
278+
279+
LEAVE_PINNED_SCOPE();
275280
}
276281
pResolvedObject->obj = returnValue->returnedValue;
277282
defaultAttributes |= DBGPROP_ATTRIB_VALUE_READONLY;
@@ -289,42 +294,34 @@ namespace Js
289294
// The debugger uses the functionNameId field instead of the "name" property to get the name of the funtion. The functionNameId field is overloaded and may contain the display name if
290295
// toString() has been called on the function object. For built-in or external functions the display name can be something like "function Echo() { native code }". We will try to parse the
291296
// function name out of the display name so the user will see just the function name e.g. "Echo" instead of the full display name in debugger.
292-
JavascriptString * VariableWalkerBase::ParseFunctionName(JavascriptString* displayName, ScriptContext* scriptContext)
297+
const char16 * VariableWalkerBase::ParseFunctionName(const char16 * displayNameBuffer, const charcount_t displayNameBufferLength, ScriptContext* scriptContext)
293298
{
294-
Assert(displayName);
295-
const char16 * displayNameBuffer = displayName->GetString();
296-
const charcount_t displayNameBufferLength = displayName->GetLength();
297299
const charcount_t funcStringLength = _countof(JS_DISPLAY_STRING_FUNCTION_HEADER) - 1; // discount the ending null character in string literal
298300
const charcount_t templateStringLength = funcStringLength + _countof(JS_DISPLAY_STRING_FUNCTION_BODY) - 1; // discount the ending null character in string literal
299301
// If the string doesn't meet our expected format; return the original string.
300302
if (displayNameBufferLength <= templateStringLength || (wmemcmp(displayNameBuffer, JS_DISPLAY_STRING_FUNCTION_HEADER, funcStringLength) != 0))
301303
{
302-
return displayName;
304+
return displayNameBuffer;
303305
}
304306

305307
// Look for the left parenthesis, if we don't find one; return the original string.
306308
const char16* parenChar = wcschr(displayNameBuffer, '(');
307309
if (parenChar == nullptr)
308310
{
309-
return displayName;
311+
return displayNameBuffer;
310312
}
311313

312314
charcount_t actualFunctionNameLength = displayNameBufferLength - templateStringLength;
313315
uint byteLengthForCopy = sizeof(char16) * actualFunctionNameLength;
314316
char16 * actualFunctionNameBuffer = AnewArray(GetArenaFromContext(scriptContext), char16, actualFunctionNameLength + 1); // The last character will be the null character.
315317
if (actualFunctionNameBuffer == nullptr)
316318
{
317-
return displayName;
319+
return displayNameBuffer;
318320
}
319321
js_memcpy_s(actualFunctionNameBuffer, byteLengthForCopy, displayNameBuffer + funcStringLength, byteLengthForCopy);
320322
actualFunctionNameBuffer[actualFunctionNameLength] = _u('\0');
321323

322-
JavascriptString * actualFunctionName = JavascriptString::NewWithArenaSz(actualFunctionNameBuffer, scriptContext);
323-
if (actualFunctionName == nullptr)
324-
{
325-
return displayName;
326-
}
327-
return actualFunctionName;
324+
return actualFunctionNameBuffer;
328325
}
329326

330327
/*static*/

‎lib/Runtime/Debug/DiagObjectModel.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ namespace Js
218218
bool IsPropertyValid(PropertyId propertyId, RegSlot location, bool *isPropertyInDebuggerScope, bool* isConst, bool* isInDeadZone) const;
219219

220220
private:
221-
static JavascriptString * ParseFunctionName(JavascriptString* displayName, ScriptContext* scriptContext);
221+
static const char16 * ParseFunctionName(const char16* displayNameBuffer, const charcount_t displayNameBufferLength, ScriptContext* scriptContext);
222222
};
223223

224224

‎lib/Runtime/Library/JavascriptLibrary.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ namespace Js
4343
typedef TwoLevelHashDictionary<FastEvalMapString, ScriptFunction*, EvalMapRecord, EvalCacheTopLevelDictionary, EvalMapString> EvalCacheDictionary;
4444

4545
typedef JsUtil::BaseDictionary<JavascriptMethod, JavascriptFunction*, Recycler, PrimeSizePolicy> BuiltInLibraryFunctionMap;
46+
typedef JsUtil::BaseDictionary<uint, JavascriptString *, Recycler> StringMap;
4647

4748
// valid if object!= NULL
4849
struct EnumeratedObjectCache
@@ -74,6 +75,7 @@ namespace Js
7475
Field(EvalCacheDictionary*) evalCacheDictionary;
7576
Field(EvalCacheDictionary*) indirectEvalCacheDictionary;
7677
Field(NewFunctionCache*) newFunctionCache;
78+
Field(StringMap *) integerStringMap;
7779
Field(RegexPatternMruMap *) dynamicRegexMap;
7880
Field(SourceContextInfoMap*) sourceContextInfoMap; // maps host provided context cookie to the URL of the script buffer passed.
7981
Field(DynamicSourceContextInfoMap*) dynamicSourceContextInfoMap;
@@ -649,6 +651,8 @@ namespace Js
649651
SCACHE_FUNCTION_PROXY(GetObjectNumberDisplayString)
650652
SCACHE_FUNCTION_PROXY(GetObjectRegExpDisplayString)
651653
SCACHE_FUNCTION_PROXY(GetObjectStringDisplayString)
654+
SCACHE_FUNCTION_PROXY(GetObjectNullDisplayString)
655+
SCACHE_FUNCTION_PROXY(GetObjectUndefinedDisplayString)
652656
SCACHE_FUNCTION_PROXY(GetUndefinedDisplayString)
653657
SCACHE_FUNCTION_PROXY(GetNaNDisplayString)
654658
SCACHE_FUNCTION_PROXY(GetNullDisplayString)

‎lib/Runtime/Library/JavascriptObject.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,12 @@ namespace Js
365365
// 1. If the this value is undefined, return "[object Undefined]".
366366
if (type == TypeIds_Undefined)
367367
{
368-
return library->CreateStringFromCppLiteral(_u("[object Undefined]"));
368+
return library->GetObjectUndefinedDisplayString();
369369
}
370370
// 2. If the this value is null, return "[object Null]".
371371
if (type == TypeIds_Null)
372372
{
373-
return library->CreateStringFromCppLiteral(_u("[object Null]"));
373+
return library->GetObjectNullDisplayString();
374374
}
375375

376376
// 3. Let O be ToObject(this value).

‎lib/Runtime/Library/JavascriptRegularExpression.cpp

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ namespace Js
1515
pattern(pattern),
1616
splitPattern(nullptr),
1717
lastIndexVar(nullptr),
18-
lastIndexOrFlag(0)
18+
lastIndexOrFlag(0),
19+
testCache(nullptr)
1920
{
2021
Assert(type->GetTypeId() == TypeIds_RegEx);
2122
Assert(!this->GetType()->AreThisAndPrototypesEnsuredToHaveOnlyWritableDataProperties());
@@ -41,7 +42,8 @@ namespace Js
4142
pattern(nullptr),
4243
splitPattern(nullptr),
4344
lastIndexVar(nullptr),
44-
lastIndexOrFlag(0)
45+
lastIndexOrFlag(0),
46+
testCache(nullptr)
4547
{
4648
Assert(type->GetTypeId() == TypeIds_RegEx);
4749

@@ -59,7 +61,8 @@ namespace Js
5961
pattern(instance->GetPattern()),
6062
splitPattern(instance->GetSplitPattern()),
6163
lastIndexVar(instance->lastIndexVar),
62-
lastIndexOrFlag(instance->lastIndexOrFlag)
64+
lastIndexOrFlag(instance->lastIndexOrFlag),
65+
testCache(nullptr)
6366
{
6467
// For boxing stack instance
6568
Assert(ThreadContext::IsOnStack(instance));
@@ -607,6 +610,7 @@ namespace Js
607610
thisRegularExpression->SetPattern(pattern);
608611
thisRegularExpression->SetSplitPattern(splitPattern);
609612
thisRegularExpression->SetLastIndex(0);
613+
thisRegularExpression->ClearTestCache();
610614
return thisRegularExpression;
611615
}
612616

@@ -1552,6 +1556,58 @@ namespace Js
15521556
: specialPropertyIdsWithoutUnicode;
15531557
}
15541558

1559+
Field(RegExpTestCache*) JavascriptRegExp::EnsureTestCache()
1560+
{
1561+
if (this->testCache != nullptr)
1562+
{
1563+
return this->testCache;
1564+
}
1565+
1566+
this->testCache = RecyclerNewArrayZ(GetRecycler(), RegExpTestCache, TestCacheSize);
1567+
return this->testCache;
1568+
}
1569+
1570+
/* static */
1571+
uint JavascriptRegExp::GetTestCacheIndex(JavascriptString* str)
1572+
{
1573+
return (uint)(((uintptr_t)str) >> PolymorphicInlineCacheShift) & (TestCacheSize - 1);
1574+
}
1575+
1576+
void JavascriptRegExp::ClearTestCache()
1577+
{
1578+
this->testCache = nullptr;
1579+
}
1580+
1581+
#if ENABLE_REGEX_CONFIG_OPTIONS
1582+
/* static */
1583+
void JavascriptRegExp::TraceTestCache(bool cacheHit, JavascriptString* input, JavascriptString* cachedValue, bool disabled)
1584+
{
1585+
if (REGEX_CONFIG_FLAG(RegexTracing))
1586+
{
1587+
if (disabled)
1588+
{
1589+
Output::Print(_u("Regexp Test Cache Disabled.\n"));
1590+
}
1591+
else if (cacheHit)
1592+
{
1593+
Output::Print(_u("Regexp Test Cache Hit.\n"));
1594+
}
1595+
else
1596+
{
1597+
Output::Print(_u("Regexp Test Cache Miss. "));
1598+
if (cachedValue != nullptr)
1599+
{
1600+
Output::Print(_u("Input: (%p); Cached String: (%p) '%s'\n"), input, cachedValue, cachedValue->GetString());
1601+
}
1602+
else
1603+
{
1604+
Output::Print(_u("Cache was empty\n"));
1605+
}
1606+
}
1607+
}
1608+
}
1609+
#endif
1610+
15551611
#if ENABLE_TTD
15561612
TTD::NSSnapObjects::SnapObjectType JavascriptRegExp::GetSnapTag_TTD() const
15571613
{

‎lib/Runtime/Library/JavascriptRegularExpression.h

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66

77
namespace Js
88
{
9+
struct RegExpTestCache
10+
{
11+
Field(bool) result;
12+
Field(RecyclerWeakReference<JavascriptString> *) input;
13+
};
914
class JavascriptRegExp : public DynamicObject
1015
{
1116
friend class JavascriptRegularExpressionType;
@@ -25,14 +30,15 @@ namespace Js
2530
// Initialization of this pattern is deferred until split() is called, or it's copied from another
2631
// RegExp object.
2732
Field(UnifiedRegex::RegexPattern*) splitPattern;
28-
2933
Field(Var) lastIndexVar; // null => must build lastIndexVar from current lastIndex
34+
Field(RegExpTestCache*) testCache;
3035

3136
public:
32-
3337
// Three states for lastIndex value:
3438
// 1. lastIndexVar has been updated, we must calculate lastIndex from it when we next need it
3539
static const CharCount NotCachedValue = (CharCount)-2;
40+
41+
static const uint TestCacheSize = 8;
3642
private:
3743
// 2. ToNumber(lastIndexVar) yields +inf or -inf or an integer not in range [0, MaxCharCount]
3844
static const CharCount InvalidValue = CharCountFlag;
@@ -134,6 +140,14 @@ namespace Js
134140

135141
JavascriptString *ToString(bool sourceOnly = false);
136142

143+
Field(RegExpTestCache*) EnsureTestCache();
144+
void ClearTestCache();
145+
static uint GetTestCacheIndex(JavascriptString* str);
146+
147+
#if ENABLE_REGEX_CONFIG_OPTIONS
148+
static void TraceTestCache(bool cacheHit, JavascriptString* input, JavascriptString* cachedValue, bool disabled);
149+
#endif
150+
137151
class EntryInfo
138152
{
139153
public:

‎lib/Runtime/Library/JavascriptString.cpp

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -102,22 +102,11 @@ namespace Js
102102
return NewWithBuffer(content, GetBufferLength(content), scriptContext);
103103
}
104104

105-
JavascriptString* JavascriptString::NewWithArenaSz(__in_z const char16 * content, ScriptContext * scriptContext)
106-
{
107-
AssertMsg(content != nullptr, "NULL value passed to JavascriptString::New");
108-
return NewWithArenaBuffer(content, GetBufferLength(content), scriptContext);
109-
}
110-
111105
JavascriptString* JavascriptString::NewWithBuffer(__in_ecount(cchUseLength) const char16 * content, charcount_t cchUseLength, ScriptContext * scriptContext)
112106
{
113107
return NewWithBufferT<LiteralString, false>(content, cchUseLength, scriptContext);
114108
}
115109

116-
JavascriptString* JavascriptString::NewWithArenaBuffer(__in_ecount(cchUseLength) const char16* content, charcount_t cchUseLength, ScriptContext* scriptContext)
117-
{
118-
return NewWithBufferT<ArenaLiteralString, false>(content, cchUseLength, scriptContext);
119-
}
120-
121110
JavascriptString* JavascriptString::NewCopySz(__in_z const char16* content, ScriptContext* scriptContext)
122111
{
123112
return NewCopyBuffer(content, GetBufferLength(content), scriptContext);
@@ -128,21 +117,6 @@ namespace Js
128117
return NewWithBufferT<LiteralString, true>(content, cchUseLength, scriptContext);
129118
}
130119

131-
JavascriptString* JavascriptString::NewCopySzFromArena(__in_z const char16* content,
132-
ScriptContext* scriptContext, ArenaAllocator *arena, charcount_t cchUseLength)
133-
{
134-
AssertMsg(content != nullptr, "NULL value passed to JavascriptString::New");
135-
136-
if (!cchUseLength)
137-
{
138-
cchUseLength = JavascriptString::GetBufferLength(content);
139-
}
140-
141-
char16* buffer = JavascriptString::AllocateAndCopySz(arena, content, cchUseLength);
142-
return ArenaLiteralString::New(scriptContext->GetLibrary()->GetStringTypeStatic(),
143-
buffer, cchUseLength, arena);
144-
}
145-
146120
Var JavascriptString::NewInstance(RecyclableObject* function, CallInfo callInfo, ...)
147121
{
148122
PROBE_STACK(function->GetScriptContext(), Js::Constants::MinStackDefault);

‎lib/Runtime/Library/JavascriptString.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,6 @@ namespace Js
172172
static JavascriptString* NewCopySz(__in_z const char16* content, ScriptContext* scriptContext);
173173
static JavascriptString* NewCopyBuffer(__in_ecount(charLength) const char16* content, charcount_t charLength, ScriptContext* scriptContext);
174174

175-
static JavascriptString* NewWithArenaSz(__in_z const char16 * content, ScriptContext* scriptContext);
176-
static JavascriptString* NewWithArenaBuffer(__in_ecount(charLength) const char16 * content, charcount_t charLength, ScriptContext * scriptContext);
177-
178-
static JavascriptString* NewCopySzFromArena(__in_z const char16* content, ScriptContext* scriptContext, ArenaAllocator *arena, charcount_t cchUseLength = 0);
179-
180175
static __ecount(length+1) char16* AllocateLeafAndCopySz(__in Recycler* recycler, __in_ecount(length) const char16* content, charcount_t length);
181176
static __ecount(length+1) char16* AllocateAndCopySz(__in ArenaAllocator* arena, __in_ecount(length) const char16* content, charcount_t length);
182177
static void CopyHelper(__out_ecount(countNeeded) char16 *dst, __in_ecount(countNeeded) const char16 * str, charcount_t countNeeded);

0 commit comments

Comments
 (0)