Improve BuiltInPropertyRecord string comparison

lib/Runtime/Base/PropertyRecord.h

@@ -121,8 +121,46 @@ namespace Js
 
         bool Equals(JsUtil::CharacterBuffer<WCHAR> const & str) const
         {
-            return (LEN - 1 == str.GetLength() &&
-                JsUtil::CharacterBuffer<WCHAR>::StaticEquals(buffer, str.GetBuffer(), LEN - 1));
+#ifndef _NTBUILD
+            AssertMsg(false, "Do you really have to use this interface?");
+#endif
+            return Equals(str.GetBuffer(), str.GetLength());
+        }
+
+        bool Equals(JavascriptString * str) const
+        {
+            PropertyString * propString = PropertyString::TryFromVar(str);
+            const PropertyRecord * propRecord = nullptr;
+            if (propString == nullptr)
+            {
+                LiteralStringWithPropertyStringPtr * lstr = LiteralStringWithPropertyStringPtr::TryFromVar(str);
+                propRecord = lstr->GetPropertyRecord();
+            }
+            else
+            {
+                propRecord = propString->GetPropertyRecord();
+            }
+
+
+            if (propRecord == nullptr)
+            {
+                return Equals(str->GetString(), str->GetLength());
+            }
+            else
+            {
+                return Equals(propRecord->GetPropertyId());
+            }
+        }
+
+        bool Equals(const PropertyId & propertyId) const
+        {
+            return propertyId == propertyRecord.GetPropertyId();
+        }
+
+        bool Equals(const WCHAR * str, const charcount_t length) const
+        {
+            return (LEN - 1 == length &&
+                JsUtil::CharacterBuffer<WCHAR>::StaticEquals(buffer, str, LEN - 1));
         }
     };
 

lib/Runtime/Language/JavascriptOperators.cpp

@@ -1351,7 +1351,7 @@ CommonNumber:
         {
             return TRUE;
         }
-       
+
         JavascriptProxy* proxy = JavascriptOperators::TryFromVar<JavascriptProxy>(instance);
         if (proxy)
         {
@@ -4466,45 +4466,39 @@ CommonNumber:
 
         if (indexType == IndexType_Number)
         {
+SetElementIHelper_INDEX_TYPE_IS_NUMBER:
             return JavascriptOperators::SetItem(receiver, object, indexVal, value, scriptContext, flags);
         }
         else if (indexType == IndexType_JavascriptString)
         {
             Assert(propertyNameString);
-            JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
 
-            if (BuiltInPropertyRecords::NaN.Equals(propertyName))
-            {
-                // Follow SetProperty convention for NaN
-                return JavascriptOperators::SetProperty(receiver, object, PropertyIds::NaN, value, scriptContext, flags);
-            }
-            else if (BuiltInPropertyRecords::Infinity.Equals(propertyName))
-            {
-                // Follow SetProperty convention for Infinity
-                return JavascriptOperators::SetProperty(receiver, object, PropertyIds::Infinity, value, scriptContext, flags);
-            }
-#ifdef ENABLE_DEBUG_CONFIG_OPTIONS
-            if (PHASE_TRACE1(PropertyStringCachePhase))
+            // At this point, we know that the propertyNameString is neither PropertyString
+            // or LiteralStringWithPropertyStringPtr.. Get PropertyRecord!
+            // we will get it anyways otherwise. (Also, 1:1 string comparison for Builtin types will be expensive.)
+
+            if (propertyRecord == nullptr)
             {
-                Output::Print(_u("PropertyCache: SetElem No property string for '%s'\n"), propertyNameString->GetString());
+                scriptContext->GetOrAddPropertyRecord(propertyNameString, &propertyRecord);
+                if (propertyRecord->IsNumeric())
+                {
+                    indexVal = propertyRecord->GetNumericValue();
+                    goto SetElementIHelper_INDEX_TYPE_IS_NUMBER;
+                }
             }
-#endif
-            return SetPropertyWPCache(receiver, object, propertyNameString, value, scriptContext, flags, &propertyValueInfo);
         }
-        else
+
+        Assert(indexType == IndexType_PropertyId || indexType == IndexType_JavascriptString);
+        Assert(propertyRecord);
+        PropertyId propId = propertyRecord->GetPropertyId();
+        if (propId == PropertyIds::NaN || propId == PropertyIds::Infinity)
         {
-            Assert(indexType == IndexType_PropertyId);
-            Assert(propertyRecord);
-            PropertyId propId = propertyRecord->GetPropertyId();
-            if (propId == PropertyIds::NaN || propId == PropertyIds::Infinity)
-            {
-                // As we no longer convert o[x] into o.x for NaN and Infinity, we need to follow SetProperty convention for these,
-                // which would check for read-only properties, strict mode, etc.
-                // Note that "-Infinity" does not qualify as property name, so we don't have to take care of it.
-                return JavascriptOperators::SetProperty(receiver, object, propId, value, scriptContext, flags);
-            }
-            return SetPropertyWPCache(receiver, object, propId, value, scriptContext, flags, &propertyValueInfo);
+            // As we no longer convert o[x] into o.x for NaN and Infinity, we need to follow SetProperty convention for these,
+            // which would check for read-only properties, strict mode, etc.
+            // Note that "-Infinity" does not qualify as property name, so we don't have to take care of it.
+            return JavascriptOperators::SetProperty(receiver, object, propId, value, scriptContext, flags);
         }
+        return SetPropertyWPCache(receiver, object, propId, value, scriptContext, flags, &propertyValueInfo);
     }
 
     BOOL JavascriptOperators::OP_SetNativeIntElementI(

lib/Runtime/Library/BoundFunction.cpp

@@ -438,8 +438,7 @@ namespace Js
 
     BOOL BoundFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::length.Equals(propertyName))
+        if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             return false;
         }

lib/Runtime/Library/JavascriptArray.cpp

@@ -3140,7 +3140,7 @@ namespace Js
                     JS_REENTRANT_NO_MUTATE(jsReentLock, CopyNativeIntArrayElementsToVar(pDestArray, BigIndex(idxDest).GetSmallIndex(), pIntItemArray));
                     idxDest = idxDest + pIntItemArray->length;
                 }
-                else 
+                else
                 {
                     JavascriptNativeFloatArray *pFloatItemArray = JavascriptOperators::TryFromVar<JavascriptNativeFloatArray>(aItem);
                     if (pFloatItemArray)
@@ -3390,7 +3390,7 @@ namespace Js
 
                     idxDest = idxDest + pIntItemArray->length;
                 }
-                else 
+                else
                 {
                     JavascriptNativeFloatArray * pFloatItemArray = JavascriptOperators::TryFromVar<JavascriptNativeFloatArray>(aItem);
                     if (pFloatItemArray && !isFillFromPrototypes)
@@ -5380,7 +5380,7 @@ Case0:
             {
                 RecyclableObject* protoObj = prototype;
 
-                if (!(DynamicObject::IsAnyArray(protoObj) || JavascriptOperators::IsObject(protoObj)) 
+                if (!(DynamicObject::IsAnyArray(protoObj) || JavascriptOperators::IsObject(protoObj))
                     || JavascriptProxy::Is(protoObj)
                     || protoObj->IsExternal())
                 {
@@ -6095,7 +6095,7 @@ Case0:
             *isIntArray = true;
 #endif
         }
-        else 
+        else
         {
             JavascriptNativeFloatArray* nativeFloatArray = JavascriptOperators::TryFromVar<JavascriptNativeFloatArray>(this);
             if (nativeFloatArray)
@@ -12162,8 +12162,7 @@ Case0:
 
     BOOL JavascriptArray::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::length.Equals(propertyName))
+        if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             return false;
         }

lib/Runtime/Library/JavascriptFunction.cpp

@@ -3071,8 +3071,7 @@ LABEL1:
 
     BOOL JavascriptFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::caller.Equals(propertyName) || BuiltInPropertyRecords::arguments.Equals(propertyName))
+        if (BuiltInPropertyRecords::caller.Equals(propertyNameString) || BuiltInPropertyRecords::arguments.Equals(propertyNameString))
         {
             if (this->HasRestrictedProperties())
             {
@@ -3080,7 +3079,7 @@ LABEL1:
                 return false;
             }
         }
-        else if (BuiltInPropertyRecords::length.Equals(propertyName))
+        else if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             if (this->IsScriptFunction())
             {
@@ -3091,7 +3090,7 @@ LABEL1:
 
         BOOL result = DynamicObject::DeleteProperty(propertyNameString, flags);
 
-        if (result && (BuiltInPropertyRecords::prototype.Equals(propertyName) || BuiltInPropertyRecords::_symbolHasInstance.Equals(propertyName)))
+        if (result && (BuiltInPropertyRecords::prototype.Equals(propertyNameString) || BuiltInPropertyRecords::_symbolHasInstance.Equals(propertyNameString)))
         {
             InvalidateConstructorCacheOnPrototypeChange();
             this->GetScriptContext()->GetThreadContext()->InvalidateIsInstInlineCachesForFunction(this);

lib/Runtime/Library/JavascriptGeneratorFunction.cpp

@@ -479,13 +479,12 @@ namespace Js
 
     BOOL JavascriptGeneratorFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::length.Equals(propertyName))
+        if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             return false;
         }
 
-        if (BuiltInPropertyRecords::caller.Equals(propertyName) || BuiltInPropertyRecords::arguments.Equals(propertyName))
+        if (BuiltInPropertyRecords::caller.Equals(propertyNameString) || BuiltInPropertyRecords::arguments.Equals(propertyNameString))
         {
             // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
             return DynamicObject::DeleteProperty(propertyNameString, flags);

lib/Runtime/Library/JavascriptRegExpConstructor.cpp

@@ -391,27 +391,30 @@ namespace Js
 
     BOOL JavascriptRegExpConstructor::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::input.Equals(propertyName)
-            || BuiltInPropertyRecords::$_.Equals(propertyName)
-            || BuiltInPropertyRecords::lastMatch.Equals(propertyName)
-            || BuiltInPropertyRecords::$Ampersand.Equals(propertyName)
-            || BuiltInPropertyRecords::lastParen.Equals(propertyName)
-            || BuiltInPropertyRecords::$Plus.Equals(propertyName)
-            || BuiltInPropertyRecords::leftContext.Equals(propertyName)
-            || BuiltInPropertyRecords::$BackTick.Equals(propertyName)
-            || BuiltInPropertyRecords::rightContext.Equals(propertyName)
-            || BuiltInPropertyRecords::$Tick.Equals(propertyName)
-            || BuiltInPropertyRecords::$1.Equals(propertyName)
-            || BuiltInPropertyRecords::$2.Equals(propertyName)
-            || BuiltInPropertyRecords::$3.Equals(propertyName)
-            || BuiltInPropertyRecords::$4.Equals(propertyName)
-            || BuiltInPropertyRecords::$5.Equals(propertyName)
-            || BuiltInPropertyRecords::$6.Equals(propertyName)
-            || BuiltInPropertyRecords::$7.Equals(propertyName)
-            || BuiltInPropertyRecords::$8.Equals(propertyName)
-            || BuiltInPropertyRecords::$9.Equals(propertyName)
-            || BuiltInPropertyRecords::index.Equals(propertyName))
+        PropertyRecord const * propertyRecord = nullptr;
+        propertyNameString->GetScriptContext()->GetOrAddPropertyRecord(propertyNameString, &propertyRecord);
+        PropertyId propertyId = propertyRecord->GetPropertyId();
+
+        if (BuiltInPropertyRecords::input.Equals(propertyId)
+            || BuiltInPropertyRecords::$_.Equals(propertyId)
+            || BuiltInPropertyRecords::lastMatch.Equals(propertyId)
+            || BuiltInPropertyRecords::$Ampersand.Equals(propertyId)
+            || BuiltInPropertyRecords::lastParen.Equals(propertyId)
+            || BuiltInPropertyRecords::$Plus.Equals(propertyId)
+            || BuiltInPropertyRecords::leftContext.Equals(propertyId)
+            || BuiltInPropertyRecords::$BackTick.Equals(propertyId)
+            || BuiltInPropertyRecords::rightContext.Equals(propertyId)
+            || BuiltInPropertyRecords::$Tick.Equals(propertyId)
+            || BuiltInPropertyRecords::$1.Equals(propertyId)
+            || BuiltInPropertyRecords::$2.Equals(propertyId)
+            || BuiltInPropertyRecords::$3.Equals(propertyId)
+            || BuiltInPropertyRecords::$4.Equals(propertyId)
+            || BuiltInPropertyRecords::$5.Equals(propertyId)
+            || BuiltInPropertyRecords::$6.Equals(propertyId)
+            || BuiltInPropertyRecords::$7.Equals(propertyId)
+            || BuiltInPropertyRecords::$8.Equals(propertyId)
+            || BuiltInPropertyRecords::$9.Equals(propertyId)
+            || BuiltInPropertyRecords::index.Equals(propertyId))
         {
             JavascriptError::ThrowCantDeleteIfStrictMode(flags, GetScriptContext(), propertyNameString->GetString());
             return false;

lib/Runtime/Library/JavascriptRegularExpression.cpp

@@ -1321,7 +1321,6 @@ namespace Js
     BOOL JavascriptRegExp::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
     {
         const ScriptConfiguration* scriptConfig = this->GetScriptContext()->GetConfig();
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
 
 #define DELETE_PROPERTY(ownProperty) \
         if (ownProperty) \
@@ -1331,23 +1330,23 @@ namespace Js
         } \
         return DynamicObject::DeleteProperty(propertyNameString, flags);
 
-        if (BuiltInPropertyRecords::lastIndex.Equals(propertyName))
+        if (BuiltInPropertyRecords::lastIndex.Equals(propertyNameString))
         {
             DELETE_PROPERTY(true);
         }
-        else if (BuiltInPropertyRecords::global.Equals(propertyName)
-            || BuiltInPropertyRecords::multiline.Equals(propertyName)
-            || BuiltInPropertyRecords::ignoreCase.Equals(propertyName)
-            || BuiltInPropertyRecords::source.Equals(propertyName)
-            || BuiltInPropertyRecords::options.Equals(propertyName))
+        else if (BuiltInPropertyRecords::global.Equals(propertyNameString)
+            || BuiltInPropertyRecords::multiline.Equals(propertyNameString)
+            || BuiltInPropertyRecords::ignoreCase.Equals(propertyNameString)
+            || BuiltInPropertyRecords::source.Equals(propertyNameString)
+            || BuiltInPropertyRecords::options.Equals(propertyNameString))
         {
             DELETE_PROPERTY(!scriptConfig->IsES6RegExPrototypePropertiesEnabled());
         }
-        else if (BuiltInPropertyRecords::unicode.Equals(propertyName))
+        else if (BuiltInPropertyRecords::unicode.Equals(propertyNameString))
         {
             DELETE_PROPERTY(scriptConfig->IsES6UnicodeExtensionsEnabled() && !scriptConfig->IsES6RegExPrototypePropertiesEnabled());
         }
-        else if (BuiltInPropertyRecords::sticky.Equals(propertyName))
+        else if (BuiltInPropertyRecords::sticky.Equals(propertyNameString))
         {
             DELETE_PROPERTY(scriptConfig->IsES6RegExStickyEnabled() && !scriptConfig->IsES6RegExPrototypePropertiesEnabled());
         }

lib/Runtime/Library/JavascriptString.cpp

@@ -3863,8 +3863,7 @@ case_2:
 
     BOOL JavascriptString::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags propertyOperationFlags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::length.Equals(propertyName))
+        if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             JavascriptError::ThrowCantDeleteIfStrictMode(propertyOperationFlags, this->GetScriptContext(), propertyNameString->GetString());
 

lib/Runtime/Library/JavascriptStringObject.cpp

@@ -342,8 +342,7 @@ namespace Js
 
     BOOL JavascriptStringObject::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags propertyOperationFlags)
     {
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (BuiltInPropertyRecords::length.Equals(propertyName))
+        if (BuiltInPropertyRecords::length.Equals(propertyNameString))
         {
             JavascriptError::ThrowCantDeleteIfStrictMode(propertyOperationFlags, this->GetScriptContext(), propertyNameString->GetString());
 

lib/Runtime/Library/ObjectPrototypeObject.cpp

@@ -107,8 +107,7 @@ namespace Js
     {
         const BOOL result = __super::DeleteProperty(propertyNameString, flags);
 
-        JsUtil::CharacterBuffer<WCHAR> propertyName(propertyNameString->GetString(), propertyNameString->GetLength());
-        if (result && BuiltInPropertyRecords::__proto__.Equals(propertyName))
+        if (result && BuiltInPropertyRecords::__proto__.Equals(propertyNameString))
         {
             this->__proto__Enabled = false;
         }

lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp

@@ -191,12 +191,11 @@ namespace Js
         {
             return propertyId;
         }
-        JsUtil::CharacterBuffer<WCHAR> propertyStr(propertyKey->GetString(), propertyKey->GetLength());
-        if (BuiltInPropertyRecords::valueOf.Equals(propertyStr))
+        if (BuiltInPropertyRecords::valueOf.Equals(propertyKey))
         {
             return PropertyIds::valueOf;
         }
-        if (BuiltInPropertyRecords::toString.Equals(propertyStr))
+        if (BuiltInPropertyRecords::toString.Equals(propertyKey))
         {
            return PropertyIds::toString;
         }

lib/Runtime/Types/TypeHandler.cpp

@@ -572,15 +572,15 @@ namespace Js
         if (possibleSideEffects)
         {
             ScriptContext* scriptContext = instance->GetScriptContext();
-            if (BuiltInPropertyRecords::valueOf.Equals(propertyName))
+            if (BuiltInPropertyRecords::valueOf.Equals(propertyName.GetBuffer(), propertyName.GetLength()))
             {
                 scriptContext->optimizationOverrides.SetSideEffects((SideEffects)(SideEffects_ValueOf & possibleSideEffects));
             }
-            else if (BuiltInPropertyRecords::toString.Equals(propertyName))
+            else if (BuiltInPropertyRecords::toString.Equals(propertyName.GetBuffer(), propertyName.GetLength()))
             {
                 scriptContext->optimizationOverrides.SetSideEffects((SideEffects)(SideEffects_ToString & possibleSideEffects));
             }
-            else if (BuiltInPropertyRecords::Math.Equals(propertyName))
+            else if (BuiltInPropertyRecords::Math.Equals(propertyName.GetBuffer(), propertyName.GetLength()))
             {
                 if (instance == scriptContext->GetLibrary()->GetGlobalObject())
                 {