From 4cae9e440dcfac64052cf694b2ab7006c94aef2b Mon Sep 17 00:00:00 2001 From: Oguz Bastemur Date: Wed, 24 Jan 2018 03:08:29 -0800 Subject: [PATCH] deps: update ChakraCore to Microsoft/ChakraCore@4d96e78734 [1.8>1.9] [MERGE #4580 @obastemur] Update PropertyRecord interface to preserve the record life time Merge pull request #4580 from obastemur:props_swb Reviewed-By: chakrabot --- .../core/lib/Runtime/Base/ScriptContext.cpp | 2 +- .../core/lib/Runtime/Base/ThreadContext.cpp | 5 ++- .../Runtime/Language/JavascriptConversion.cpp | 2 +- .../Runtime/Language/JavascriptOperators.cpp | 32 +++++++++++-------- .../core/lib/Runtime/Library/ConcatString.cpp | 29 +++++++++++------ .../core/lib/Runtime/Library/ConcatString.h | 3 +- .../Runtime/Library/ForInObjectEnumerator.cpp | 4 +-- .../lib/Runtime/Library/JSONStringifier.cpp | 2 +- .../lib/Runtime/Library/JavascriptObject.cpp | 2 +- .../lib/Runtime/Library/JavascriptString.cpp | 10 +++--- .../lib/Runtime/Library/JavascriptString.h | 6 ++-- .../lib/Runtime/Library/LazyJSONString.cpp | 6 ++-- .../core/lib/Runtime/Library/PropertyString.h | 9 ++++-- .../Types/DynamicObjectPropertyEnumerator.cpp | 2 +- .../Types/SimpleDictionaryTypeHandler.cpp | 5 +-- 15 files changed, 70 insertions(+), 49 deletions(-) diff --git a/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp b/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp index 5129241b75c..2d759c43233 100644 --- a/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp +++ b/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp @@ -877,7 +877,7 @@ namespace Js void ScriptContext::GetOrAddPropertyRecord(_In_ Js::JavascriptString * propertyString, _Out_ PropertyRecord const** propertyRecord) { - *propertyRecord = propertyString->GetPropertyRecord(); + propertyString->GetPropertyRecord(propertyRecord); } void ScriptContext::GetOrAddPropertyRecord(JsUtil::CharacterBuffer const& propertyName, PropertyRecord const ** propertyRecord) diff --git a/deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp b/deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp index 40413025d4a..ae0752cc988 100644 --- a/deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp +++ b/deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp @@ -888,10 +888,9 @@ ThreadContext::GetPropertyNameImpl(Js::PropertyId propertyId) void ThreadContext::FindPropertyRecord(Js::JavascriptString *pstName, Js::PropertyRecord const ** propertyRecord) { - const Js::PropertyRecord * propRecord = pstName->GetPropertyRecord(true); - if (propRecord != nullptr) + pstName->GetPropertyRecord(propertyRecord, true); + if (*propertyRecord != nullptr) { - *propertyRecord = propRecord; return; } diff --git a/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp b/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp index c5e439c4071..f3dc867e882 100644 --- a/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp +++ b/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp @@ -293,7 +293,7 @@ namespace Js { // For all other types, convert the key into a string and use that as the property name JavascriptString * propName = JavascriptConversion::ToString(key, scriptContext); - *propertyRecord = propName->GetPropertyRecord(); + propName->GetPropertyRecord(propertyRecord); } if (propString) diff --git a/deps/chakrashim/core/lib/Runtime/Language/JavascriptOperators.cpp b/deps/chakrashim/core/lib/Runtime/Language/JavascriptOperators.cpp index 50a4ba9fbdf..d774cf8ba2a 100644 --- a/deps/chakrashim/core/lib/Runtime/Language/JavascriptOperators.cpp +++ b/deps/chakrashim/core/lib/Runtime/Language/JavascriptOperators.cpp @@ -1914,7 +1914,7 @@ namespace Js { if (value && !WithScopeObject::Is(object) && info->GetPropertyString()) { - PropertyId propertyId = info->GetPropertyString()->GetPropertyRecord()->GetPropertyId(); + PropertyId propertyId = info->GetPropertyString()->GetPropertyId(); CacheOperators::CachePropertyRead(instance, object, false, propertyId, false, info, requestContext); } return JavascriptConversion::PropertyQueryFlagsToBoolean(result); @@ -1928,7 +1928,7 @@ namespace Js if (!PHASE_OFF1(MissingPropertyCachePhase) && info->GetPropertyString() && DynamicObject::Is(instance) && ((DynamicObject*)instance)->GetDynamicType()->GetTypeHandler()->IsPathTypeHandler()) { PropertyValueInfo::Set(info, requestContext->GetLibrary()->GetMissingPropertyHolder(), 0); - CacheOperators::CachePropertyRead(instance, requestContext->GetLibrary()->GetMissingPropertyHolder(), false, info->GetPropertyString()->GetPropertyRecord()->GetPropertyId(), true, info, requestContext); + CacheOperators::CachePropertyRead(instance, requestContext->GetLibrary()->GetMissingPropertyHolder(), false, info->GetPropertyString()->GetPropertyId(), true, info, requestContext); } *value = requestContext->GetMissingPropertyResult(); @@ -2302,7 +2302,7 @@ namespace Js { if (!WithScopeObject::Is(receiver) && info->GetPropertyString()) { - CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), false, object->GetType(), info->GetPropertyString()->GetPropertyRecord()->GetPropertyId(), info, requestContext); + CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), false, object->GetType(), info->GetPropertyString()->GetPropertyId(), info, requestContext); } receiver = (RecyclableObject::FromVar(receiver))->GetThisObjectOrUnWrap(); RecyclableObject* func = RecyclableObject::FromVar(setterValueOrProxy); @@ -2358,12 +2358,12 @@ namespace Js { if (!JavascriptProxy::Is(receiver) && info->GetPropertyString() && info->GetFlags() != InlineCacheSetterFlag && !object->CanHaveInterceptors()) { - CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), false, typeWithoutProperty, info->GetPropertyString()->GetPropertyRecord()->GetPropertyId(), info, requestContext); + CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), false, typeWithoutProperty, info->GetPropertyString()->GetPropertyId(), info, requestContext); if (info->GetInstance() == receiverObject) { PropertyValueInfo::SetCacheInfo(info, info->GetPropertyString(), info->GetPropertyString()->GetLdElemInlineCache(), info->AllowResizingPolymorphicInlineCache()); - CacheOperators::CachePropertyRead(object, receiverObject, false, info->GetPropertyString()->GetPropertyRecord()->GetPropertyId(), false, info, requestContext); + CacheOperators::CachePropertyRead(object, receiverObject, false, info->GetPropertyString()->GetPropertyId(), false, info, requestContext); } } return TRUE; @@ -3778,7 +3778,8 @@ namespace Js LiteralStringWithPropertyStringPtr * strWithPtr = (LiteralStringWithPropertyStringPtr *)temp; if (!strWithPtr->HasPropertyRecord()) { - strWithPtr->GetPropertyRecord(); // lookup-cache propertyRecord + PropertyRecord const * propertyRecord; + strWithPtr->GetPropertyRecord(&propertyRecord); // lookup-cache propertyRecord } else { @@ -3798,7 +3799,8 @@ namespace Js JavascriptString::FromVar(index)->GetSz()); } - PropertyRecord const * propertyRecord = propertyString->GetPropertyRecord(); + PropertyRecord const * propertyRecord; + propertyString->GetPropertyRecord(&propertyRecord); Var value; if (propertyRecord->IsNumeric()) @@ -4430,13 +4432,13 @@ namespace Js LiteralStringWithPropertyStringPtr * strWithPtr = LiteralStringWithPropertyStringPtr::TryFromVar(index); if (strWithPtr != nullptr) { - propertyString = strWithPtr->GetPropertyString(); // do not force create the PropertyString, - // if it wasn't there, it won't be efficient for now. - propertyRecord = strWithPtr->GetPropertyRecord(true /* dontLookupFromDictionary */); + propertyString = strWithPtr->GetPropertyString(); // do not force create the PropertyString, + // if it wasn't there, it won't be efficient for now. + strWithPtr->GetPropertyRecord(&propertyRecord, true /* dontLookupFromDictionary */); if (propertyRecord == nullptr) { - propertyRecord = strWithPtr->GetPropertyRecord(); // lookup-cache propertyRecord - // later this call, there will be a lookup anyways! + strWithPtr->GetPropertyRecord(&propertyRecord); // lookup-cache propertyRecord + // later this call, there will be a lookup anyways! } else if (propertyString == nullptr) { @@ -4448,7 +4450,7 @@ namespace Js } else { - propertyRecord = propertyString->GetPropertyRecord(); + propertyString->GetPropertyRecord(&propertyRecord); } if (propertyRecord != nullptr) @@ -10529,7 +10531,9 @@ namespace Js BOOL JavascriptOperators::CheckPrototypesForAccessorOrNonWritableProperty(RecyclableObject* instance, JavascriptString* propertyNameString, Var* setterValue, DescriptorFlags* flags, PropertyValueInfo* info, ScriptContext* scriptContext) { - PropertyId propertyId = propertyNameString->GetPropertyRecord()->GetPropertyId(); + Js::PropertyRecord const * localPropertyRecord; + propertyNameString->GetPropertyRecord(&localPropertyRecord); + PropertyId propertyId = localPropertyRecord->GetPropertyId(); return CheckPrototypesForAccessorOrNonWritableProperty(instance, propertyId, setterValue, flags, info, scriptContext); } diff --git a/deps/chakrashim/core/lib/Runtime/Library/ConcatString.cpp b/deps/chakrashim/core/lib/Runtime/Library/ConcatString.cpp index ac985f54ef4..d666ee0e3fc 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/ConcatString.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/ConcatString.cpp @@ -151,7 +151,9 @@ namespace Js this->propertyString = propStr; if (propStr != nullptr) { - this->propertyRecord = propStr->GetPropertyRecord(); + Js::PropertyRecord const * localPropertyRecord; + propStr->GetPropertyRecord(&localPropertyRecord); + this->propertyRecord = localPropertyRecord; } } @@ -167,20 +169,29 @@ namespace Js return RecyclableObject::Is(var) && LiteralStringWithPropertyStringPtr::Is(RecyclableObject::UnsafeFromVar(var)); } - Js::PropertyRecord const * LiteralStringWithPropertyStringPtr::GetPropertyRecord(bool dontLookupFromDictionary) + void LiteralStringWithPropertyStringPtr::GetPropertyRecord(_Out_ PropertyRecord const** propRecord, bool dontLookupFromDictionary) { + *propRecord = nullptr; ScriptContext * scriptContext = this->GetScriptContext(); - if (this->propertyRecord == nullptr && !dontLookupFromDictionary) + if (this->propertyRecord == nullptr) { - Js::PropertyRecord const * localPropertyRecord; - scriptContext->GetOrAddPropertyRecord(this->GetSz(), - static_cast(this->GetLength()), - &localPropertyRecord); - this->propertyRecord = localPropertyRecord; + if (!dontLookupFromDictionary) + { + // cache PropertyRecord + Js::PropertyRecord const * localPropertyRecord; + scriptContext->GetOrAddPropertyRecord(this->GetSz(), + static_cast(this->GetLength()), + &localPropertyRecord); + this->propertyRecord = localPropertyRecord; + } + else + { + return; + } } - return this->propertyRecord; + *propRecord = this->propertyRecord; } /////////////////////// ConcatStringBase ////////////////////////// diff --git a/deps/chakrashim/core/lib/Runtime/Library/ConcatString.h b/deps/chakrashim/core/lib/Runtime/Library/ConcatString.h index 9790c1b49b7..2c996e63c59 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/ConcatString.h +++ b/deps/chakrashim/core/lib/Runtime/Library/ConcatString.h @@ -13,7 +13,8 @@ namespace Js Field(const Js::PropertyRecord*) propertyRecord; public: - virtual Js::PropertyRecord const * GetPropertyRecord(bool dontLookupFromDictionary = false) override; + virtual void GetPropertyRecord(_Out_ PropertyRecord const** propRecord, bool dontLookupFromDictionary = false) override; + bool HasPropertyRecord() const { return propertyRecord != nullptr; } PropertyString * GetPropertyString() const; diff --git a/deps/chakrashim/core/lib/Runtime/Library/ForInObjectEnumerator.cpp b/deps/chakrashim/core/lib/Runtime/Library/ForInObjectEnumerator.cpp index e5d15011239..afb8fd87c1a 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/ForInObjectEnumerator.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/ForInObjectEnumerator.cpp @@ -184,10 +184,10 @@ namespace Js // Property Id does not exist. if (propertyId == Constants::NoProperty) { - propRecord = currentIndex->GetPropertyRecord(true); + currentIndex->GetPropertyRecord(&propRecord, true); if (propRecord == nullptr) { - propRecord = currentIndex->GetPropertyRecord(false); // will create + currentIndex->GetPropertyRecord(&propRecord, false); // will create // We keep the track of what is enumerated using a bit vector of propertyID. // so the propertyId can't be collected until the end of the for in enumerator // Keep a list of the property string. diff --git a/deps/chakrashim/core/lib/Runtime/Library/JSONStringifier.cpp b/deps/chakrashim/core/lib/Runtime/Library/JSONStringifier.cpp index bf01faa5dcc..1b49c51374a 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/JSONStringifier.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/JSONStringifier.cpp @@ -235,7 +235,7 @@ JSONStringifier::ReadValue(_In_ JavascriptString* key, _In_opt_ const PropertyRe if (propertyRecord == nullptr) { - propertyRecord = key->GetPropertyRecord(); + key->GetPropertyRecord(&propertyRecord); } JavascriptOperators::GetProperty(holder, propertyRecord->GetPropertyId(), &value, this->scriptContext, &info); return value; diff --git a/deps/chakrashim/core/lib/Runtime/Library/JavascriptObject.cpp b/deps/chakrashim/core/lib/Runtime/Library/JavascriptObject.cpp index 3d50790a8e5..1da31f9a312 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/JavascriptObject.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/JavascriptObject.cpp @@ -1733,7 +1733,7 @@ namespace Js } else { - propertyRecord = ((PropertyString*)propertyName)->GetPropertyRecord(); + propertyName->GetPropertyRecord(&propertyRecord); } if (descCount == descSize) diff --git a/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.cpp b/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.cpp index 6e3e416dade..c7d55b776b1 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.cpp @@ -224,17 +224,15 @@ namespace Js return JavascriptOperators::GetTypeId(aValue) == TypeIds_String; } - Js::PropertyRecord const * JavascriptString::GetPropertyRecord(bool dontLookupFromDictionary) + void JavascriptString::GetPropertyRecord(_Out_ Js::PropertyRecord const ** propertyRecord, bool dontLookupFromDictionary) { + *propertyRecord = nullptr; if (dontLookupFromDictionary) { - return nullptr; + return; } - Js::PropertyRecord const * propertyRecord; - GetScriptContext()->GetOrAddPropertyRecord(GetString(), GetLength(), &propertyRecord); - - return propertyRecord; + GetScriptContext()->GetOrAddPropertyRecord(GetString(), GetLength(), propertyRecord); } JavascriptString* JavascriptString::FromVar(Var aValue) diff --git a/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.h b/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.h index a10c7488272..20444a2e080 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.h +++ b/deps/chakrashim/core/lib/Runtime/Library/JavascriptString.h @@ -50,7 +50,7 @@ namespace Js BOOL GetItemAt(charcount_t idxChar, Var* value); char16 GetItem(charcount_t index); - virtual Js::PropertyRecord const * GetPropertyRecord(bool dontLookupFromDictionary = false); + virtual void GetPropertyRecord(_Out_ PropertyRecord const** propertyRecord, bool dontLookupFromDictionary = false); _Ret_range_(m_charLength, m_charLength) charcount_t GetLength() const; virtual size_t GetAllocatedByteCount() const; @@ -354,8 +354,8 @@ namespace Js { inline static bool Equals(JavascriptString * str1, JavascriptString * str2) { - // We want to pin the strings str1 and str2 because flattening of any of these strings could cause a GC and result in the other string getting collected if it was optimized - // away by the compiler. We would normally have called the EnterPinnedScope/LeavePinnedScope methods here but it adds extra call instructions to the assembly code. As Equals + // We want to pin the strings str1 and str2 because flattening of any of these strings could cause a GC and result in the other string getting collected if it was optimized + // away by the compiler. We would normally have called the EnterPinnedScope/LeavePinnedScope methods here but it adds extra call instructions to the assembly code. As Equals // methods could get called a lot of times this can show up as regressions in benchmarks. volatile Js::JavascriptString** keepAliveString1 = (volatile Js::JavascriptString**)& str1; volatile Js::JavascriptString** keepAliveString2 = (volatile Js::JavascriptString**)& str2; diff --git a/deps/chakrashim/core/lib/Runtime/Library/LazyJSONString.cpp b/deps/chakrashim/core/lib/Runtime/Library/LazyJSONString.cpp index 801cfed5ba9..a5e2eb2e3bd 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/LazyJSONString.cpp +++ b/deps/chakrashim/core/lib/Runtime/Library/LazyJSONString.cpp @@ -57,8 +57,10 @@ LazyJSONString::ReconstructObject(_In_ JSONObject* valueList) const PropertyValueInfo info; if (!propertyString || !propertyString->TrySetPropertyFromCache(obj, propertyValue, this->GetScriptContext(), PropertyOperation_None, &info)) { - const PropertyRecord* propertyRecord = propertyName->GetPropertyRecord(); - JavascriptOperators::SetProperty(obj, obj, propertyRecord->GetPropertyId(), propertyValue, &info, this->GetScriptContext()); + Js::PropertyRecord const * localPropertyRecord; + propertyName->GetPropertyRecord(&localPropertyRecord); + JavascriptOperators::SetProperty(obj, obj, localPropertyRecord->GetPropertyId(), + propertyValue, &info, this->GetScriptContext()); } } NEXT_SLISTCOUNTED_ENTRY; diff --git a/deps/chakrashim/core/lib/Runtime/Library/PropertyString.h b/deps/chakrashim/core/lib/Runtime/Library/PropertyString.h index f532adacec7..0b5e4cf2b71 100644 --- a/deps/chakrashim/core/lib/Runtime/Library/PropertyString.h +++ b/deps/chakrashim/core/lib/Runtime/Library/PropertyString.h @@ -18,9 +18,14 @@ class PropertyString : public JavascriptString PropertyString(StaticType* type, const Js::PropertyRecord* propertyRecord); public: - virtual Js::PropertyRecord const * GetPropertyRecord(bool dontLookupFromDictionary = false) override + virtual void GetPropertyRecord(_Out_ PropertyRecord const** propertyRecord, bool dontLookupFromDictionary = false) override { - return this->propertyRecord; + *propertyRecord = this->propertyRecord; + } + + Js::PropertyId GetPropertyId() + { + return this->propertyRecord->GetPropertyId(); } PolymorphicInlineCache * GetLdElemInlineCache() const; diff --git a/deps/chakrashim/core/lib/Runtime/Types/DynamicObjectPropertyEnumerator.cpp b/deps/chakrashim/core/lib/Runtime/Types/DynamicObjectPropertyEnumerator.cpp index f78030f5175..ba581489d04 100644 --- a/deps/chakrashim/core/lib/Runtime/Types/DynamicObjectPropertyEnumerator.cpp +++ b/deps/chakrashim/core/lib/Runtime/Types/DynamicObjectPropertyEnumerator.cpp @@ -180,7 +180,7 @@ namespace Js { PropertyString * propertyString = cachedData->strings[enumeratedCount]; propertyStringName = propertyString; - propertyId = propertyString->GetPropertyRecord()->GetPropertyId(); + propertyId = propertyString->GetPropertyId(); #if DBG PropertyId tempPropertyId; diff --git a/deps/chakrashim/core/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp b/deps/chakrashim/core/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp index 5228c3e9589..b59ff1f4a81 100644 --- a/deps/chakrashim/core/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp +++ b/deps/chakrashim/core/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp @@ -91,7 +91,7 @@ namespace Js PropertyString * propertyString = PropertyString::TryFromVar(key); if (propertyString != nullptr) { - propertyRecord = propertyString->GetPropertyRecord(); + propertyString->GetPropertyRecord(&propertyRecord); } else { @@ -104,7 +104,8 @@ namespace Js bool TPropertyKey_IsInternalPropertyId(JavascriptString* key) { // WARNING: This will return false for PropertyStrings that are actually InternalPropertyIds - Assert(!PropertyString::Is(key) || !IsInternalPropertyId(((PropertyString*)key)->GetPropertyRecord()->GetPropertyId())); + Assert(!PropertyString::Is(key) || !IsInternalPropertyId(((PropertyString*)key)->GetPropertyId())); + return false; }