Ignore:
Timestamp:
Mar 30, 2022, 1:41:54 PM (3 years ago)
Author:
Chris Dumez
Message:

Optimize the construction of a JSC::Identifier from an ASCIILiteral
​https://bugs.webkit.org/show_bug.cgi?id=238552

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

There are two ways to construct a JSC::Identifier from a string literal and both
were sub-optimal:

  • template<unsigned charactersCount> static Identifier fromString(VM&, const char (&characters)[charactersCount]): Even though it knows the charactersCount at compile time, it doesn't leverage this information at all. Also, it ends up calling AtomStringImpl::add() instead of AtomStringImpl::addLiteral() which means we lose the knowledge that it was a string literal and the optimization that come with it (e.g. not copying the characters over). In this patch, I am deprecating this function in favor of fromString(ASCIILiteral) which is now more efficient. I'll remove it in a follow up.
  • static Identifier fromString(VM&, ASCIILiteral): It ended up calling the Identifier(VM&, String) constructor which meant that we were potentially constructing a String/StringImpl unnecessarily before atomizing it since the string may already be in the AtomString table. We also failed to use the smallStrings cache for literals whose length is 1, which would happen when calling fromString(VM&, const char (&characters)[charactersCount]).

In this patch, I optimized fromString(VM&, ASCIILiteral) to leverage the smallStrings cache
when necessary and call AtomStringImpl::addLiteral() instead of AtomStringImpl::add(). I also
made sure to mark fromString(VM&, ASCIILiteral) and Identifier(VM&, ASCIILiteral) as
ALWAYS_INLINE to make sure the compiler can optimize out the strlen() calls.

According to A/B bots, this is a 1-1.5% progression on Speedometer and and a 1.3% progression on
JetStream on iMac 20,1 (Intel). Sadly, this is perf-neutral on those two benchmarks on Apple
Silicon.

  • API/JSAPIGlobalObject.mm:

(JSC::JSAPIGlobalObject::moduleLoaderCreateImportMetaProperties):

  • API/JSBase.cpp:

(JSGetMemoryUsageStatistics):

  • API/JSObjectRef.cpp:

(JSObjectMakeFunction):

  • builtins/BuiltinNames.cpp:

(JSC::BuiltinNames::BuiltinNames):

  • builtins/BuiltinUtils.h:
  • dynbench.cpp:
  • inspector/JSInjectedScriptHost.cpp:

(Inspector::JSInjectedScriptHost::functionDetails):
(Inspector::constructInternalProperty):
(Inspector::JSInjectedScriptHost::weakMapEntries):
(Inspector::JSInjectedScriptHost::weakSetEntries):
(Inspector::JSInjectedScriptHost::iteratorEntries):

  • inspector/JSJavaScriptCallFrame.cpp:

(Inspector::valueForScopeLocation):
(Inspector::JSJavaScriptCallFrame::scopeDescriptions):

  • inspector/ScriptCallStackFactory.cpp:

(Inspector::extractSourceInformationFromException):

  • inspector/agents/InspectorAuditAgent.cpp:

(Inspector::InspectorAuditAgent::populateAuditObject):

  • jsc.cpp:

(GlobalObject::moduleLoaderCreateImportMetaProperties):
(JSC_DEFINE_HOST_FUNCTION):
(dumpException):

  • runtime/AbstractModuleRecord.cpp:

(JSC::AbstractModuleRecord::hostResolveImportedModule):

  • runtime/AtomicsObject.cpp:

(JSC::AtomicsObject::finishCreation):

  • runtime/CommonIdentifiers.cpp:

(JSC::CommonIdentifiers::CommonIdentifiers):

  • runtime/FinalizationRegistryPrototype.cpp:

(JSC::FinalizationRegistryPrototype::finishCreation):

  • runtime/Identifier.cpp:

(JSC::Identifier::addLiteral):

  • runtime/Identifier.h:

(JSC::Identifier::Identifier):

  • runtime/IdentifierInlines.h:

(JSC::Identifier::fromString):

  • runtime/IntlLocale.cpp:

(JSC::IntlLocale::textInfo):
(JSC::IntlLocale::weekInfo):

  • runtime/IntlNumberFormat.cpp:

(JSC::IntlNumberFormat::initializeNumberFormat):
(JSC::IntlNumberFormat::resolvedOptions const):
(JSC::IntlNumberFormat::formatToPartsInternal):

  • runtime/IntlPluralRules.cpp:

(JSC::IntlPluralRules::resolvedOptions const):

  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::exposeDollarVM):

  • runtime/JSModuleLoader.cpp:

(JSC::JSModuleLoader::finishCreation):

  • runtime/MathObject.cpp:

(JSC::MathObject::finishCreation):

  • runtime/NumberConstructor.cpp:

(JSC::NumberConstructor::finishCreation):

  • runtime/StringPrototype.cpp:

(JSC::StringPrototype::finishCreation):

  • runtime/SymbolConstructor.cpp:
  • tools/JSDollarVM.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):
(JSC::JSDollarVM::finishCreation):

  • wasm/js/JSWebAssemblyGlobal.cpp:

(JSC::JSWebAssemblyGlobal::type):

  • wasm/js/JSWebAssemblyMemory.cpp:

(JSC::JSWebAssemblyMemory::type):

  • wasm/js/JSWebAssemblyTable.cpp:

(JSC::JSWebAssemblyTable::type):

  • wasm/js/WebAssemblyGlobalConstructor.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

  • wasm/js/WebAssemblyMemoryConstructor.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

  • wasm/js/WebAssemblyModuleConstructor.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

  • wasm/js/WebAssemblyTableConstructor.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

  • wasm/js/WebAssemblyTagPrototype.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

Source/WebCore:

Adopt the JSC::Identifier constructor from an ASCIILiteral more widely.

  • Modules/applepay-ams-ui/ApplePayAMSUIPaymentHandler.cpp:

(WebCore::ApplePayAMSUIPaymentHandler::finishSession):

  • Modules/encryptedmedia/legacy/LegacyCDMSessionClearKey.cpp:

(WebCore::CDMSessionClearKey::update):

  • Modules/webaudio/AudioWorkletGlobalScope.cpp:

(WebCore::AudioWorkletGlobalScope::registerProcessor):

  • Modules/webaudio/AudioWorkletProcessor.cpp:

(WebCore::AudioWorkletProcessor::process):

  • bindings/js/JSCustomElementRegistryCustom.cpp:

(WebCore::JSCustomElementRegistry::define):

  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::DialogHandler::dialogCreated):
(WebCore::DialogHandler::returnValue const):
(WebCore::JSDOMWindow::setOpener):
(WebCore::JSDOMWindow::setOpenDatabase):

  • bindings/js/JSEventListener.cpp:

(WebCore::JSEventListener::handleEvent):

  • bindings/js/JSImageDataCustom.cpp:

(WebCore::toJSNewlyCreated):

  • bindings/js/ScriptModuleLoader.cpp:

(WebCore::ScriptModuleLoader::createImportMetaProperties):

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateDictionaryImplementationContent):
(addUnscopableProperties):
(GenerateImplementation):
(GenerateAttributeSetterBodyDefinition):
(GenerateDefaultToJSONOperationDefinition):
(GenerateCallbackImplementationContent):
(GenerateConstructorHelperMethods):

  • bindings/scripts/test/JS/JSExposedStar.cpp:

(WebCore::JSExposedStarPrototype::finishCreation):

  • bindings/scripts/test/JS/JSExposedToWorkerAndWindow.cpp:

(WebCore::convertDictionary<ExposedToWorkerAndWindow::Dict>):
(WebCore::convertDictionaryToJS):

  • bindings/scripts/test/JS/JSTestCEReactions.cpp:

(WebCore::setJSTestCEReactions_stringifierAttributeSetter):
(WebCore::setJSTestCEReactions_stringifierAttributeNotNeededSetter):

  • bindings/scripts/test/JS/JSTestCallbackInterface.cpp:

(WebCore::convertDictionary<TestCallbackInterface::Dictionary>):
(WebCore::JSTestCallbackInterface::callbackWithNoParam):
(WebCore::JSTestCallbackInterface::callbackWithArrayParam):
(WebCore::JSTestCallbackInterface::callbackWithSerializedScriptValueParam):
(WebCore::JSTestCallbackInterface::callbackWithStringList):
(WebCore::JSTestCallbackInterface::callbackWithBoolean):
(WebCore::JSTestCallbackInterface::callbackRequiresThisToPass):
(WebCore::JSTestCallbackInterface::callbackWithAReturnValue):
(WebCore::JSTestCallbackInterface::callbackThatRethrowsExceptions):
(WebCore::JSTestCallbackInterface::callbackWithThisObject):

  • bindings/scripts/test/JS/JSTestConditionalIncludes.cpp:

(WebCore::JSTestConditionalIncludesDOMConstructor::initializeProperties):
(WebCore::JSTestConditionalIncludesPrototype::finishCreation):

  • bindings/scripts/test/JS/JSTestConditionallyReadWrite.cpp:

(WebCore::JSTestConditionallyReadWritePrototype::finishCreation):

  • bindings/scripts/test/JS/JSTestDefaultToJSON.cpp:

(WebCore::JSTestDefaultToJSONPrototype::finishCreation):
(WebCore::jsTestDefaultToJSONPrototypeFunction_toJSONBody):

  • bindings/scripts/test/JS/JSTestDefaultToJSONFilteredByExposed.cpp:

(WebCore::JSTestDefaultToJSONFilteredByExposedPrototype::finishCreation):
(WebCore::jsTestDefaultToJSONFilteredByExposedPrototypeFunction_toJSONBody):

  • bindings/scripts/test/JS/JSTestDefaultToJSONInherit.cpp:

(WebCore::jsTestDefaultToJSONInheritPrototypeFunction_toJSONBody):

  • bindings/scripts/test/JS/JSTestDefaultToJSONInheritFinal.cpp:

(WebCore::jsTestDefaultToJSONInheritFinalPrototypeFunction_toJSONBody):

  • bindings/scripts/test/JS/JSTestDerivedDictionary.cpp:

(WebCore::convertDictionary<TestDerivedDictionary>):
(WebCore::convertDictionaryToJS):

  • bindings/scripts/test/JS/JSTestDerivedDictionary2.cpp:

(WebCore::convertDictionary<TestDerivedDictionary2>):
(WebCore::convertDictionaryToJS):
(WebCore::convertDictionary<TestDerivedDictionary2::Dictionary>):

  • bindings/scripts/test/JS/JSTestDictionary.cpp:

(WebCore::convertDictionary<TestDictionary>):

  • bindings/scripts/test/JS/JSTestDictionaryWithOnlyConditionalMembers.cpp:

(WebCore::convertDictionary<TestDictionaryWithOnlyConditionalMembers>):
(WebCore::convertDictionaryToJS):

  • bindings/scripts/test/JS/JSTestEnabledBySetting.cpp:

(WebCore::JSTestEnabledBySettingDOMConstructor::initializeProperties):
(WebCore::JSTestEnabledBySettingPrototype::finishCreation):

  • bindings/scripts/test/JS/JSTestEventConstructor.cpp:

(WebCore::convertDictionary<TestEventConstructor::Init>):

  • bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:

(WebCore::JSTestGenerateIsReachablePrototype::finishCreation):

  • bindings/scripts/test/JS/JSTestInheritedDictionary.cpp:

(WebCore::convertDictionary<TestInheritedDictionary>):
(WebCore::convertDictionaryToJS):

  • bindings/scripts/test/JS/JSTestInheritedDictionary2.cpp:

(WebCore::convertDictionary<TestInheritedDictionary2>):
(WebCore::convertDictionaryToJS):

  • bindings/scripts/test/JS/JSTestInterface.cpp:

(WebCore::JSTestInterfacePrototype::finishCreation):

  • bindings/scripts/test/JS/JSTestNamespaceObject.cpp:

(WebCore::JSTestNamespaceObjectDOMConstructor::initializeProperties):

  • bindings/scripts/test/JS/JSTestNode.cpp:

(WebCore::JSTestNodePrototype::finishCreation):
(WebCore::jsTestNodePrototypeFunction_toJSONBody):

  • bindings/scripts/test/JS/JSTestObj.cpp:

(WebCore::convertDictionary<TestObj::Dictionary>):
(WebCore::convertDictionaryToJS):
(WebCore::convertDictionary<TestObj::DictionaryThatShouldNotTolerateNull>):
(WebCore::convertDictionary<TestObj::DictionaryThatShouldTolerateNull>):
(WebCore::convertDictionary<AlternateDictionaryName>):
(WebCore::convertDictionary<TestObj::ParentDictionary>):
(WebCore::convertDictionary<TestObj::ChildDictionary>):
(WebCore::convertDictionary<TestObj::ConditionalDictionaryA>):
(WebCore::convertDictionary<TestObj::ConditionalDictionaryB>):
(WebCore::convertDictionary<TestObj::ConditionalDictionaryC>):
(WebCore::JSTestObjDOMConstructor::initializeProperties):
(WebCore::JSTestObjPrototype::finishCreation):
(WebCore::setJSTestObj_putForwardsAttributeSetter):
(WebCore::setJSTestObj_putForwardsNullableAttributeSetter):

  • bindings/scripts/test/JS/JSTestPromiseRejectionEvent.cpp:

(WebCore::convertDictionary<TestPromiseRejectionEvent::Init>):

  • bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp:

(WebCore::convertDictionary<DictionaryImplName>):
(WebCore::convertDictionaryToJS):

  • html/HTMLMediaElement.cpp:

(WebCore::controllerJSValue):
(WebCore::HTMLMediaElement::updateCaptionContainer):
(WebCore::HTMLMediaElement::ensureMediaControlsInjectedScript):
(WebCore::HTMLMediaElement::didAddUserAgentShadowRoot):
(WebCore::HTMLMediaElement::updateMediaControlsAfterPresentationModeChange):
(WebCore::HTMLMediaElement::getCurrentMediaControlsStatus):

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/jsc.cpp

    r291842 r292118  
    573573#endif
    574574
    575         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "OSRExit"), 0, functionUndefined1, OSRExitIntrinsic, DontEnum);
    576         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isFinalTier"), 0, functionFalse, IsFinalTierIntrinsic, DontEnum);
    577         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "predictInt32"), 0, functionUndefined2, SetInt32HeapPredictionIntrinsic, DontEnum);
    578         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isInt32"), 0, functionIsInt32, CheckInt32Intrinsic, DontEnum);
    579         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isPureNaN"), 0, functionIsPureNaN, CheckInt32Intrinsic, DontEnum);
    580         putDirectNativeFunction(vm, this, Identifier::fromString(vm, "fiatInt52"), 0, functionIdentity, FiatInt52Intrinsic, DontEnum);
     575        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "OSRExit"_s), 0, functionUndefined1, OSRExitIntrinsic, DontEnum);
     576        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isFinalTier"_s), 0, functionFalse, IsFinalTierIntrinsic, DontEnum);
     577        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "predictInt32"_s), 0, functionUndefined2, SetInt32HeapPredictionIntrinsic, DontEnum);
     578        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isInt32"_s), 0, functionIsInt32, CheckInt32Intrinsic, DontEnum);
     579        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "isPureNaN"_s), 0, functionIsPureNaN, CheckInt32Intrinsic, DontEnum);
     580        putDirectNativeFunction(vm, this, Identifier::fromString(vm, "fiatInt52"_s), 0, functionIdentity, FiatInt52Intrinsic, DontEnum);
    581581       
    582582        addFunction(vm, "effectful42", functionEffectful42, 0);
    … …  
    635635            for (size_t i = 0; i < arguments.size(); ++i)
    636636                array->putDirectIndex(this, i, jsString(vm, arguments[i]));
    637             putDirect(vm, Identifier::fromString(vm, "arguments"), array, DontEnum);
    638         }
    639 
    640         putDirect(vm, Identifier::fromString(vm, "console"), jsUndefined(), DontEnum);
     637            putDirect(vm, Identifier::fromString(vm, "arguments"_s), array, DontEnum);
     638        }
     639
     640        putDirect(vm, Identifier::fromString(vm, "console"_s), jsUndefined(), DontEnum);
    641641       
    642642        Structure* plainObjectStructure = JSFinalObject::createStructure(vm, this, objectPrototype(), 0);
    643643       
    644644        JSObject* dollar = JSFinalObject::create(vm, plainObjectStructure);
    645         putDirect(vm, Identifier::fromString(vm, "$"), dollar, DontEnum);
    646         putDirect(vm, Identifier::fromString(vm, "$262"), dollar, DontEnum);
     645        putDirect(vm, Identifier::fromString(vm, "$"_s), dollar, DontEnum);
     646        putDirect(vm, Identifier::fromString(vm, "$262"_s), dollar, DontEnum);
    647647       
    648648        addFunction(vm, dollar, "createRealm", functionDollarCreateRealm, 0, static_cast<unsigned>(PropertyAttribute::None));
    … …  
    654654        addFunction(vm, dollar, "isRemoteFunction", functionDollarIsRemoteFunction, 1, static_cast<unsigned>(PropertyAttribute::None));
    655655       
    656         dollar->putDirect(vm, Identifier::fromString(vm, "global"), globalThis());
    657         dollar->putDirectCustomAccessor(vm, Identifier::fromString(vm, "IsHTMLDDA"),
     656        dollar->putDirect(vm, Identifier::fromString(vm, "global"_s), globalThis());
     657        dollar->putDirectCustomAccessor(vm, Identifier::fromString(vm, "IsHTMLDDA"_s),
    658658            CustomGetterSetter::create(vm, accessorMakeMasquerader, nullptr),
    659659            static_cast<unsigned>(PropertyAttribute::CustomValue)
    … …  
    661661
    662662        JSObject* agent = JSFinalObject::create(vm, plainObjectStructure);
    663         dollar->putDirect(vm, Identifier::fromString(vm, "agent"), agent);
     663        dollar->putDirect(vm, Identifier::fromString(vm, "agent"_s), agent);
    664664       
    665665        // The test262 INTERPRETING.md document says that some of these functions are just in the main
    … …  
    692692            {
    693693                CustomGetterSetter* custom = CustomGetterSetter::create(vm, nullptr, testCustomAccessorSetter);
    694                 Identifier identifier = Identifier::fromString(vm, "testCustomAccessorSetter");
     694                Identifier identifier = Identifier::fromString(vm, "testCustomAccessorSetter"_s);
    695695                this->putDirectCustomAccessor(vm, identifier, custom, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::CustomAccessor);
    696696            }
    … …  
    698698            {
    699699                CustomGetterSetter* custom = CustomGetterSetter::create(vm, nullptr, testCustomValueSetter);
    700                 Identifier identifier = Identifier::fromString(vm, "testCustomValueSetter");
     700                Identifier identifier = Identifier::fromString(vm, "testCustomValueSetter"_s);
    701701                this->putDirectCustomAccessor(vm, identifier, custom, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::CustomValue);
    702702            }
    … …  
    12431243    RETURN_IF_EXCEPTION(scope, nullptr);
    12441244
    1245     metaProperties->putDirect(vm, Identifier::fromString(vm, "filename"), key);
     1245    metaProperties->putDirect(vm, Identifier::fromString(vm, "filename"_s), key);
    12461246    RETURN_IF_EXCEPTION(scope, nullptr);
    12471247
    … …  
    20312031    VM& vm = globalObject->vm();
    20322032    GlobalObject* result = GlobalObject::create(vm, GlobalObject::createStructure(vm, jsNull()), Vector<String>());
    2033     return JSValue::encode(result->getDirect(vm, Identifier::fromString(vm, "$")));
     2033    return JSValue::encode(result->getDirect(vm, Identifier::fromString(vm, "$"_s)));
    20342034}
    20352035
    … …  
    20422042    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    20432043   
    2044     JSValue global = callFrame->thisValue().get(globalObject, Identifier::fromString(vm, "global"));
     2044    JSValue global = callFrame->thisValue().get(globalObject, Identifier::fromString(vm, "global"_s));
    20452045    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    20462046    while (global.inherits<JSProxy>(vm))
    … …  
    23722372    JSObject* optionsObject = constructEmptyObject(globalObject);
    23732373#define READ_OPTION(type_, name_, defaultValue_, availability_, description_) \
    2374     addOption(vm, optionsObject, Identifier::fromString(vm, #name_), Options::name_());
     2374    addOption(vm, optionsObject, Identifier::fromString(vm, #name_ ""_s), Options::name_());
    23752375    FOR_EACH_JSC_OPTION(READ_OPTION)
    23762376#undef READ_OPTION
    … …  
    30513051        printf("Exception: <out of memory while extracting exception string>\n");
    30523052
    3053     Identifier nameID = Identifier::fromString(vm, "name");
     3053    Identifier nameID = Identifier::fromString(vm, "name"_s);
    30543054    CHECK_EXCEPTION();
    3055     Identifier fileNameID = Identifier::fromString(vm, "sourceURL");
     3055    Identifier fileNameID = Identifier::fromString(vm, "sourceURL"_s);
    30563056    CHECK_EXCEPTION();
    3057     Identifier lineNumberID = Identifier::fromString(vm, "line");
     3057    Identifier lineNumberID = Identifier::fromString(vm, "line"_s);
    30583058    CHECK_EXCEPTION();
    3059     Identifier stackID = Identifier::fromString(vm, "stack");
     3059    Identifier stackID = Identifier::fromString(vm, "stack"_s);
    30603060    CHECK_EXCEPTION();
    30613061
Note: See TracChangeset for help on using the changeset viewer.