Skip to content

Commit 05caa1e

Browse files
committed
[MERGE #5448 @tcare] Add JavascriptNumber fastpath in JavascriptOperators::GetIndexTypeFromPrimitive
Merge pull request #5448 from tcare:index Mitigates OS: 17348829. We recently reactivated the JS Built In implementation of indexOf. It seems there is some bad codegen in the JIT that is passing an unsigned integer as a double through this path, causing a slow ToString of the index integer and causing a significant perf hit. Regardless of the JIT issue, there's no reason for us to do a ToString on a JavascriptNumber if we don't have to. Added a check to see if we can use it as an index, otherwise go through the ToString as usual.
2 parents f023b77 + 046531b commit 05caa1e

File tree

1 file changed

+37
-23
lines changed

1 file changed

+37
-23
lines changed

lib/Runtime/Language/JavascriptOperators.cpp

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -109,37 +109,51 @@ using namespace Js;
109109
return IndexType_PropertyId;
110110
}
111111
}
112-
else
112+
113+
if (JavascriptNumber::Is_NoTaggedIntCheck(indexVar))
113114
{
114-
JavascriptSymbol * symbol = JavascriptOperators::TryFromVar<JavascriptSymbol>(indexVar);
115-
if (symbol)
115+
// If this double can be a positive integer index, convert it.
116+
int32 value = 0;
117+
bool isInt32 = false;
118+
if (JavascriptNumber::TryGetInt32OrUInt32Value(JavascriptNumber::GetValue(indexVar), &value, &isInt32)
119+
&& !isInt32
120+
&& static_cast<uint32>(value) < JavascriptArray::InvalidIndex)
116121
{
117-
// JavascriptSymbols cannot add a new PropertyRecord - they correspond to one and only one existing PropertyRecord.
118-
// We already know what the PropertyRecord is since it is stored in the JavascriptSymbol itself so just return it.
119-
120-
*propertyRecord = symbol->GetValue();
121-
return IndexType_PropertyId;
122+
*index = static_cast<uint32>(value);
123+
return IndexType_Number;
122124
}
123-
else
124-
{
125-
JavascriptString* indexStr = JavascriptConversion::ToString(indexVar, scriptContext);
126125

127-
char16 const * propertyName = indexStr->GetString();
128-
charcount_t const propertyLength = indexStr->GetLength();
126+
// Fall through to slow string conversion.
127+
}
129128

130-
if (!createIfNotFound && preferJavascriptStringOverPropertyRecord)
131-
{
132-
if (JavascriptOperators::TryConvertToUInt32(propertyName, propertyLength, index) &&
133-
(*index != JavascriptArray::InvalidIndex))
134-
{
135-
return IndexType_Number;
136-
}
129+
JavascriptSymbol * symbol = JavascriptOperators::TryFromVar<JavascriptSymbol>(indexVar);
130+
if (symbol)
131+
{
132+
// JavascriptSymbols cannot add a new PropertyRecord - they correspond to one and only one existing PropertyRecord.
133+
// We already know what the PropertyRecord is since it is stored in the JavascriptSymbol itself so just return it.
137134

138-
*propertyNameString = indexStr;
139-
return IndexType_JavascriptString;
135+
*propertyRecord = symbol->GetValue();
136+
return IndexType_PropertyId;
137+
}
138+
else
139+
{
140+
JavascriptString* indexStr = JavascriptConversion::ToString(indexVar, scriptContext);
141+
142+
char16 const * propertyName = indexStr->GetString();
143+
charcount_t const propertyLength = indexStr->GetLength();
144+
145+
if (!createIfNotFound && preferJavascriptStringOverPropertyRecord)
146+
{
147+
if (JavascriptOperators::TryConvertToUInt32(propertyName, propertyLength, index) &&
148+
(*index != JavascriptArray::InvalidIndex))
149+
{
150+
return IndexType_Number;
140151
}
141-
return GetIndexTypeFromString(propertyName, propertyLength, scriptContext, index, propertyRecord, createIfNotFound);
152+
153+
*propertyNameString = indexStr;
154+
return IndexType_JavascriptString;
142155
}
156+
return GetIndexTypeFromString(propertyName, propertyLength, scriptContext, index, propertyRecord, createIfNotFound);
143157
}
144158
}
145159

0 commit comments

Comments
 (0)