Browse Source

[1.10>master] [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.
Tom Care 7 years ago
parent
commit
de80038b6c
1 changed files with 37 additions and 23 deletions
  1. 37 23
      lib/Runtime/Language/JavascriptOperators.cpp

+ 37 - 23
lib/Runtime/Language/JavascriptOperators.cpp

@@ -109,37 +109,51 @@ using namespace Js;
                 return IndexType_PropertyId;
             }
         }
-        else
+
+        if (JavascriptNumber::Is_NoTaggedIntCheck(indexVar))
         {
-            JavascriptSymbol * symbol = JavascriptOperators::TryFromVar<JavascriptSymbol>(indexVar);
-            if (symbol)
+            // If this double can be a positive integer index, convert it.
+            int32 value = 0;
+            bool isInt32 = false;
+            if (JavascriptNumber::TryGetInt32OrUInt32Value(JavascriptNumber::GetValue(indexVar), &value, &isInt32)
+                && !isInt32
+                && static_cast<uint32>(value) < JavascriptArray::InvalidIndex)
             {
-                // JavascriptSymbols cannot add a new PropertyRecord - they correspond to one and only one existing PropertyRecord.
-                // We already know what the PropertyRecord is since it is stored in the JavascriptSymbol itself so just return it.
-
-                *propertyRecord = symbol->GetValue();
-                return IndexType_PropertyId;
+                *index = static_cast<uint32>(value);
+                return IndexType_Number;
             }
-            else
-            {
-                JavascriptString* indexStr = JavascriptConversion::ToString(indexVar, scriptContext);
 
-                char16 const * propertyName = indexStr->GetString();
-                charcount_t const propertyLength = indexStr->GetLength();
+            // Fall through to slow string conversion.
+        }
 
-                if (!createIfNotFound && preferJavascriptStringOverPropertyRecord)
-                {
-                    if (JavascriptOperators::TryConvertToUInt32(propertyName, propertyLength, index) &&
-                        (*index != JavascriptArray::InvalidIndex))
-                    {
-                        return IndexType_Number;
-                    }
+        JavascriptSymbol * symbol = JavascriptOperators::TryFromVar<JavascriptSymbol>(indexVar);
+        if (symbol)
+        {
+            // JavascriptSymbols cannot add a new PropertyRecord - they correspond to one and only one existing PropertyRecord.
+            // We already know what the PropertyRecord is since it is stored in the JavascriptSymbol itself so just return it.
 
-                    *propertyNameString = indexStr;
-                    return IndexType_JavascriptString;
+            *propertyRecord = symbol->GetValue();
+            return IndexType_PropertyId;
+        }
+        else
+        {
+            JavascriptString* indexStr = JavascriptConversion::ToString(indexVar, scriptContext);
+
+            char16 const * propertyName = indexStr->GetString();
+            charcount_t const propertyLength = indexStr->GetLength();
+
+            if (!createIfNotFound && preferJavascriptStringOverPropertyRecord)
+            {
+                if (JavascriptOperators::TryConvertToUInt32(propertyName, propertyLength, index) &&
+                    (*index != JavascriptArray::InvalidIndex))
+                {
+                    return IndexType_Number;
                 }
-                return GetIndexTypeFromString(propertyName, propertyLength, scriptContext, index, propertyRecord, createIfNotFound);
+
+                *propertyNameString = indexStr;
+                return IndexType_JavascriptString;
             }
+            return GetIndexTypeFromString(propertyName, propertyLength, scriptContext, index, propertyRecord, createIfNotFound);
         }
     }