Changeset 201825 in webkit


Ignore:
Timestamp:
Jun 8, 2016, 12:40:34 PM (9 years ago)
Author:
keith_miller@apple.com
Message:

We should be able to lookup symbols by identifier in builtins
​https://bugs.webkit.org/show_bug.cgi?id=158530

Reviewed by Mark Lam.

This patch allows us to lookup the value of a symbol property on a
object by identifier in builtins. Before, it was only possible to
do so if we were directly emitting the bytecodes, such as in a
for-of loop looking for Symbol.iterator. As we tier up we convert
the builtin's get_by_val symbol lookups into get_by_id
lookups. However, there is still a significant performance
difference between get_by_id and get_by_val in the LLInt, where
this transformation does not take place.

In order to make this work we hijack BuiltinNames'
m_publicToPrivateMap so that it points the @<symbol>Symbol to the
appropriate vm symbol. This way when we lex the identifier it will
become the appropriate symbol's identifier. Currently, if the
symbol is used to name a property in an object literal we will not
keep a cache of the Symbol objects we have already seen. We could
add a map for symbols but since we can only load symbols by
identifier in builtins its likely not worth it. Additionally, even
in builtins it is extremely rare to use Symbols in object
literals.

  • builtins/ArrayConstructor.js:

(from):

  • builtins/ArrayPrototype.js:

(filter):
(map):

  • builtins/BuiltinNames.h:

(JSC::BuiltinNames::BuiltinNames):

  • builtins/BuiltinUtils.h:
  • builtins/GlobalObject.js:

(speciesConstructor):

  • builtins/StringPrototype.js:

(match):
(intrinsic.StringPrototypeReplaceIntrinsic.replace):
(search):
(split):

  • builtins/TypedArrayConstructor.js:

(from):

  • builtins/TypedArrayPrototype.js:

(map):
(filter):

  • bytecode/BytecodeIntrinsicRegistry.cpp:

(JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry): Deleted.

  • bytecode/BytecodeIntrinsicRegistry.h:
  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitLoad):

  • parser/Parser.cpp:

(JSC::Parser<LexerType>::parseInner):

Location:
trunk/Source/JavaScriptCore
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r201810 r201825  
     12016-06-08  Keith Miller  <keith_miller@apple.com>
     2
     3        We should be able to lookup symbols by identifier in builtins
     4        https://bugs.webkit.org/show_bug.cgi?id=158530
     5
     6        Reviewed by Mark Lam.
     7
     8        This patch allows us to lookup the value of a symbol property on a
     9        object by identifier in builtins. Before, it was only possible to
     10        do so if we were directly emitting the bytecodes, such as in a
     11        for-of loop looking for Symbol.iterator. As we tier up we convert
     12        the builtin's get_by_val symbol lookups into get_by_id
     13        lookups. However, there is still a significant performance
     14        difference between get_by_id and get_by_val in the LLInt, where
     15        this transformation does not take place.
     16
     17        In order to make this work we hijack BuiltinNames'
     18        m_publicToPrivateMap so that it points the @<symbol>Symbol to the
     19        appropriate vm symbol. This way when we lex the identifier it will
     20        become the appropriate symbol's identifier.  Currently, if the
     21        symbol is used to name a property in an object literal we will not
     22        keep a cache of the Symbol objects we have already seen. We could
     23        add a map for symbols but since we can only load symbols by
     24        identifier in builtins its likely not worth it. Additionally, even
     25        in builtins it is extremely rare to use Symbols in object
     26        literals.
     27
     28        * builtins/ArrayConstructor.js:
     29        (from):
     30        * builtins/ArrayPrototype.js:
     31        (filter):
     32        (map):
     33        * builtins/BuiltinNames.h:
     34        (JSC::BuiltinNames::BuiltinNames):
     35        * builtins/BuiltinUtils.h:
     36        * builtins/GlobalObject.js:
     37        (speciesConstructor):
     38        * builtins/StringPrototype.js:
     39        (match):
     40        (intrinsic.StringPrototypeReplaceIntrinsic.replace):
     41        (search):
     42        (split):
     43        * builtins/TypedArrayConstructor.js:
     44        (from):
     45        * builtins/TypedArrayPrototype.js:
     46        (map):
     47        (filter):
     48        * bytecode/BytecodeIntrinsicRegistry.cpp:
     49        (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry): Deleted.
     50        * bytecode/BytecodeIntrinsicRegistry.h:
     51        * bytecompiler/BytecodeGenerator.cpp:
     52        (JSC::BytecodeGenerator::emitLoad):
     53        * parser/Parser.cpp:
     54        (JSC::Parser<LexerType>::parseInner):
     55
    1562016-06-08  Rawinder Singh  <rawinder.singh-webkit@cisra.canon.com.au>
    257

  • trunk/Source/JavaScriptCore/builtins/ArrayConstructor.js

    r196022 r201825  
    5858        throw new @TypeError("Array.from requires an array-like object - not null or undefined");
    5959
    60     var iteratorMethod = items[@symbolIterator];
     60    var iteratorMethod = items.@iteratorSymbol;
    6161    if (iteratorMethod != null) {
    6262        if (typeof iteratorMethod !== "function")
    … …  
    7272        // it could be observable if the user defines a getter for @@iterator.
    7373        // To avoid this situation, we define a wrapper object that @@iterator just returns a given iterator.
    74         var wrapper = {
    75             [@symbolIterator]() {
    76                 return iterator;
    77             }
    78         };
     74        var wrapper = {}
     75        wrapper.@iteratorSymbol = function() { return iterator; };
    7976
    8077        for (var value of wrapper) {

  • trunk/Source/JavaScriptCore/builtins/ArrayPrototype.js

    r201049 r201825  
    227227            constructor = @undefined;
    228228        if (@isObject(constructor)) {
    229             constructor = constructor[@symbolSpecies];
     229            constructor = constructor.@speciesSymbol;
    230230            if (constructor === null)
    231231                constructor = @undefined;
    … …  
    279279            constructor = @undefined;
    280280        if (@isObject(constructor)) {
    281             constructor = constructor[@symbolSpecies];
     281            constructor = constructor.@speciesSymbol;
    282282            if (constructor === null)
    283283                constructor = @undefined;

  • trunk/Source/JavaScriptCore/builtins/BuiltinNames.h

    r198855 r201825  
    3636#define INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY(name) m_publicToPrivateMap.add(m_##name.impl(), &m_##name##PrivateName);
    3737
     38// We commandeer the publicToPrivateMap to allow us to convert private symbol names into the appropriate symbol.
     39// e.g. @iteratorSymbol points to Symbol.iterator in this map rather than to a an actual private name.
     40// FIXME: This is a weird hack and we shouldn't need to do this.
     41#define INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY(name) m_publicToPrivateMap.add(m_##name##SymbolPrivateIdentifier.impl(), &m_##name##Symbol);
     42
    3843class BuiltinNames {
    3944    WTF_MAKE_NONCOPYABLE(BuiltinNames); WTF_MAKE_FAST_ALLOCATED;
    … …  
    5560        JSC_FOREACH_BUILTIN_FUNCTION_NAME(INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY)
    5661        JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY)
     62        JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY)
    5763        m_privateToPublicMap.add(m_dollarVMPrivateName.impl(), &m_dollarVMName);
    5864        m_publicToPrivateMap.add(m_dollarVMName.impl(), &m_dollarVMPrivateName);

  • trunk/Source/JavaScriptCore/builtins/BuiltinUtils.h

    r196498 r201825  
    3838    const JSC::Identifier& name##PrivateName() const { return m_##name##PrivateName; }
    3939
    40 #define INITIALIZE_BUILTIN_SYMBOLS(name) , m_##name##Symbol(JSC::Identifier::fromUid(JSC::PrivateName(JSC::PrivateName::Description, ASCIILiteral("Symbol." #name))))
    41 #define DECLARE_BUILTIN_SYMBOLS(name) const JSC::Identifier m_##name##Symbol;
     40#define INITIALIZE_BUILTIN_SYMBOLS(name) , m_##name##Symbol(JSC::Identifier::fromUid(JSC::PrivateName(JSC::PrivateName::Description, ASCIILiteral("Symbol." #name)))), m_##name##SymbolPrivateIdentifier(JSC::Identifier::fromString(vm, #name "Symbol"))
     41#define DECLARE_BUILTIN_SYMBOLS(name) const JSC::Identifier m_##name##Symbol; const JSC::Identifier m_##name##SymbolPrivateIdentifier;
    4242#define DECLARE_BUILTIN_SYMBOL_ACCESSOR(name) \
    4343    const JSC::Identifier& name##Symbol() const { return m_##name##Symbol; }

  • trunk/Source/JavaScriptCore/builtins/GlobalObject.js

    r199731 r201825  
    7070    if (!@isObject(constructor))
    7171        throw new @TypeError("|this|.constructor is not an Object or undefined");
    72     constructor = constructor[@symbolSpecies];
     72    constructor = constructor.@speciesSymbol;
    7373    if (constructor == null)
    7474        return defaultConstructor;

  • trunk/Source/JavaScriptCore/builtins/StringPrototype.js

    r200422 r201825  
    3737
    3838    if (regexp != null) {
    39         var matcher = regexp[@symbolMatch];
     39        var matcher = regexp.@matchSymbol;
    4040        if (matcher != @undefined)
    4141            return matcher.@call(regexp, this);
    … …  
    4444    let thisString = @toString(this);
    4545    let createdRegExp = @regExpCreate(regexp, @undefined);
    46     return createdRegExp[@symbolMatch](thisString);
     46    return createdRegExp.@matchSymbol(thisString);
    4747}
    4848
    … …  
    234234
    235235    if (search != null) {
    236         let replacer = search[@symbolReplace];
     236        let replacer = search.@replaceSymbol;
    237237        if (replacer !== @undefined) {
    238238            if (!@hasObservableSideEffectsForStringReplace(search, replacer))
    … …  
    292292
    293293    if (regexp != null) {
    294         var searcher = regexp[@symbolSearch];
     294        var searcher = regexp.@searchSymbol;
    295295        if (searcher != @undefined)
    296296            return searcher.@call(regexp, this);
    … …  
    299299    var thisString = @toString(this);
    300300    var createdRegExp = @regExpCreate(regexp, @undefined);
    301     return createdRegExp[@symbolSearch](thisString);
     301    return createdRegExp.@searchSymbol(thisString);
    302302}
    303303
    … …  
    313313   
    314314    if (separator != null) {
    315         var splitter = separator[@symbolSplit];
     315        var splitter = separator.@splitSymbol;
    316316        if (splitter != @undefined)
    317317            return splitter.@call(separator, this, limit);

  • trunk/Source/JavaScriptCore/builtins/TypedArrayConstructor.js

    r198713 r201825  
    6464        throw new @TypeError("TypedArray.from requires an array-like object - not null or undefined");
    6565
    66     let iteratorMethod = items[@symbolIterator];
     66    let iteratorMethod = items.@iteratorSymbol;
    6767    if (iteratorMethod != null) {
    6868        if (typeof iteratorMethod !== "function")
    … …  
    7777        // it could be observable if the user defines a getter for @@iterator.
    7878        // To avoid this situation, we define a wrapper object that @@iterator just returns a given iterator.
    79         let wrapper = {
    80             [@symbolIterator]() {
    81                 return iterator;
    82             }
    83         };
    84 
     79        let wrapper = {};
     80        wrapper.@iteratorSymbol = function() { return iterator; }
    8581
    8682        for (let value of wrapper) {

  • trunk/Source/JavaScriptCore/builtins/TypedArrayPrototype.js

    r200499 r201825  
    262262        result = new (@typedArrayGetOriginalConstructor(this))(length);
    263263    else {
    264         var speciesConstructor = @Object(constructor)[@symbolSpecies];
     264        var speciesConstructor = @Object(constructor).@speciesSymbol;
    265265        if (speciesConstructor === null || speciesConstructor === @undefined)
    266266            result = new (@typedArrayGetOriginalConstructor(this))(length);
    … …  
    303303        result = new (@typedArrayGetOriginalConstructor(this))(resultLength);
    304304    else {
    305         var speciesConstructor = @Object(constructor)[@symbolSpecies];
     305        var speciesConstructor = @Object(constructor).@speciesSymbol;
    306306        if (speciesConstructor === null || speciesConstructor === @undefined)
    307307            result = new (@typedArrayGetOriginalConstructor(this))(resultLength);

  • trunk/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp

    r201049 r201825  
    5555    m_promiseStateFulfilled.set(m_vm, jsNumber(static_cast<unsigned>(JSPromise::Status::Fulfilled)));
    5656    m_promiseStateRejected.set(m_vm, jsNumber(static_cast<unsigned>(JSPromise::Status::Rejected)));
    57     m_symbolIterator.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->iteratorSymbol.impl())));
    58     m_symbolMatch.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->matchSymbol.impl())));
    59     m_symbolReplace.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->replaceSymbol.impl())));
    60     m_symbolSearch.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->searchSymbol.impl())));
    61     m_symbolSpecies.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->speciesSymbol.impl())));
    62     m_symbolSplit.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->splitSymbol.impl())));
    6357    m_GeneratorResumeModeNormal.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::NormalMode)));
    6458    m_GeneratorResumeModeThrow.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::ThrowMode)));

  • trunk/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h

    r201668 r201825  
    5858    macro(promiseStateFulfilled) \
    5959    macro(promiseStateRejected) \
    60     macro(symbolIterator) \
    61     macro(symbolMatch) \
    62     macro(symbolReplace) \
    63     macro(symbolSearch) \
    64     macro(symbolSpecies) \
    65     macro(symbolSplit) \
    6660    macro(GeneratorResumeModeNormal) \
    6761    macro(GeneratorResumeModeThrow) \

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r201617 r201825  
    17281728RegisterID* BytecodeGenerator::emitLoad(RegisterID* dst, const Identifier& identifier)
    17291729{
     1730    ASSERT(!identifier.isSymbol());
    17301731    JSString*& stringInMap = m_stringMap.add(identifier.impl(), nullptr).iterator->value;
    17311732    if (!stringInMap)
    17321733        stringInMap = jsOwnedString(vm(), identifier.string());
     1734
    17331735    return emitLoad(dst, JSValue(stringInMap));
    17341736}

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r201725 r201825  
    322322        VariableEnvironment& lexicalVariables = scope->lexicalVariables();
    323323        const HashSet<UniquedStringImpl*>& closedVariableCandidates = scope->closedVariableCandidates();
    324         const BuiltinNames& builtinNames = m_vm->propertyNames->builtinNames();
    325324        for (UniquedStringImpl* candidate : closedVariableCandidates) {
    326             if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate)) {
     325            if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !candidate->isSymbol()) {
    327326                dataLog("Bad global capture in builtin: '", candidate, "'\n");
    328327                dataLog(m_source->view());
Note: See TracChangeset for help on using the changeset viewer.