Changeset 246709 in webkit


Ignore:
Timestamp:
Jun 22, 2019 12:48:08 AM (5 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] Strict, Sloppy and Arrow functions should have different classInfo
​https://bugs.webkit.org/show_bug.cgi?id=197631

Reviewed by Saam Barati.

JSTests:

  • stress/has-own-property-arguments.js: Added.

(shouldBe):
(A):

Source/JavaScriptCore:

If a constructor inherits a builtin class, it creates a Structure which is subclassing the builtin class.
This is done by using InternalFunction::createSubclassStructure. But to accelerate the common cases, we
cache the created structure in InternalFunctionAllocationProfile. Whether the cache is valid is checked
by comparing classInfo of the cached structure and the given base structure. This implicitly assume that
each builtin class's InternalFunction creates an instance based on one structure.

However, Function constructor is an exception: Function constructor creates an instance which has different
structures based on a parameter. If a strict code is given (e.g. "'use strict'"), it creates a function
instance with strict function structure.

As a result, InternalFunctionAllocationProfile incorrectly caches the structure. Consider the following code.

class A extends Function { };
let a = new A("'use strict'");
let b = new A("");

While a and b should have different structures, A caches the structure for a, and reuse it even the given
code is not a strict code. This is problematic: We are separating structures of strict, sloppy, and arrow functions
because they have different properties. However, in the above case, a and b have the same structure while they have
different properties. So it causes incorrect structure-based caching in JSC. One of the example is HasOwnPropertyCache.

In this patch, we introduce JSStrictFunction, JSSloppyFunction, and JSArrowFunction classes and classInfos. This design
works well and already partially accepted for JSGeneratorFunction, JSAsyncGeneratorFunction, and JSAsyncFunction. Each
structure now has a different classInfo so that InternalFunctionAllocationProfile correctly caches and invalidates the
cached one based on the classInfo. Since we already have different structures for these instances, and DFG and FTL
optimizations are based on JSFunctionType (not classInfo), introducing these three classInfo do not break the optimization.

Note that structures on ArrayConstructor does not cause the same problem. It only uses Undecided indexing typed array
structure in InternalFunctionAllocationProfile, and once haveABadTime happens, it clears InternalFunctionAllocationProfile.

  • runtime/JSAsyncFunction.h: This subspaceFor is not necessary since it is defined in JSFunction. And we already ensure that

sizeof(JSAsyncFunction) == sizeof(JSFunction).

  • runtime/JSAsyncGeneratorFunction.cpp:
  • runtime/JSAsyncGeneratorFunction.h: Ditto.
  • runtime/JSFunction.cpp:
  • runtime/JSFunction.h:
  • runtime/JSGeneratorFunction.h: Ditto.
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r246708 r246709  
     12019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] Strict, Sloppy and Arrow functions should have different classInfo
     4        https://bugs.webkit.org/show_bug.cgi?id=197631
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/has-own-property-arguments.js: Added.
     9        (shouldBe):
     10        (A):
     11
    1122019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
    213

  • trunk/Source/JavaScriptCore/ChangeLog

    r246708 r246709  
     12019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] Strict, Sloppy and Arrow functions should have different classInfo
     4        https://bugs.webkit.org/show_bug.cgi?id=197631
     5
     6        Reviewed by Saam Barati.
     7
     8        If a constructor inherits a builtin class, it creates a Structure which is subclassing the builtin class.
     9        This is done by using InternalFunction::createSubclassStructure. But to accelerate the common cases, we
     10        cache the created structure in InternalFunctionAllocationProfile. Whether the cache is valid is checked
     11        by comparing classInfo of the cached structure and the given base structure. This implicitly assume that
     12        each builtin class's InternalFunction creates an instance based on one structure.
     13
     14        However, Function constructor is an exception: Function constructor creates an instance which has different
     15        structures based on a parameter. If a strict code is given (e.g. "'use strict'"), it creates a function
     16        instance with strict function structure.
     17
     18        As a result, InternalFunctionAllocationProfile incorrectly caches the structure. Consider the following code.
     19
     20            class A extends Function { };
     21            let a = new A("'use strict'");
     22            let b = new A("");
     23
     24        While `a` and `b` should have different structures, `A` caches the structure for `a`, and reuse it even the given
     25        code is not a strict code. This is problematic: We are separating structures of strict, sloppy, and arrow functions
     26        because they have different properties. However, in the above case, a and b have the same structure while they have
     27        different properties. So it causes incorrect structure-based caching in JSC. One of the example is HasOwnPropertyCache.
     28
     29        In this patch, we introduce JSStrictFunction, JSSloppyFunction, and JSArrowFunction classes and classInfos. This design
     30        works well and already partially accepted for JSGeneratorFunction, JSAsyncGeneratorFunction, and JSAsyncFunction. Each
     31        structure now has a different classInfo so that InternalFunctionAllocationProfile correctly caches and invalidates the
     32        cached one based on the classInfo. Since we already have different structures for these instances, and DFG and FTL
     33        optimizations are based on JSFunctionType (not classInfo), introducing these three classInfo do not break the optimization.
     34
     35        Note that structures on ArrayConstructor does not cause the same problem. It only uses Undecided indexing typed array
     36        structure in InternalFunctionAllocationProfile, and once haveABadTime happens, it clears InternalFunctionAllocationProfile.
     37
     38        * runtime/JSAsyncFunction.h: This subspaceFor is not necessary since it is defined in JSFunction. And we already ensure that
     39        sizeof(JSAsyncFunction) == sizeof(JSFunction).
     40        * runtime/JSAsyncGeneratorFunction.cpp:
     41        * runtime/JSAsyncGeneratorFunction.h: Ditto.
     42        * runtime/JSFunction.cpp:
     43        * runtime/JSFunction.h:
     44        * runtime/JSGeneratorFunction.h: Ditto.
     45        * runtime/JSGlobalObject.cpp:
     46        (JSC::JSGlobalObject::init):
     47
    1482019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
    249

  • trunk/Source/JavaScriptCore/runtime/JSAsyncFunction.h

    r240965 r246709  
    3939    const static unsigned StructureFlags = Base::StructureFlags;
    4040
    41     template<typename CellType, SubspaceAccess>
    42     static IsoSubspace* subspaceFor(VM& vm)
    43     {
    44         return &vm.functionSpace;
    45     }
    46 
    4741    DECLARE_EXPORT_INFO;
    4842

  • trunk/Source/JavaScriptCore/runtime/JSAsyncGeneratorFunction.cpp

    r240796 r246709  
    3838namespace JSC {
    3939
    40 const ClassInfo JSAsyncGeneratorFunction    ::s_info = { "JSAsyncGeneratorFunction",  &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSAsyncGeneratorFunction) };
     40const ClassInfo JSAsyncGeneratorFunction::s_info = { "JSAsyncGeneratorFunction",  &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSAsyncGeneratorFunction) };
    4141
    4242JSAsyncGeneratorFunction::JSAsyncGeneratorFunction(VM& vm, FunctionExecutable* executable, JSScope* scope, Structure* structure)

  • trunk/Source/JavaScriptCore/runtime/JSAsyncGeneratorFunction.h

    r240965 r246709  
    3939    const static unsigned StructureFlags = Base::StructureFlags;
    4040
    41     template<typename CellType, SubspaceAccess>
    42     static IsoSubspace* subspaceFor(VM& vm)
    43     {
    44         return &vm.functionSpace;
    45     }
    46 
    4741    DECLARE_EXPORT_INFO;
    4842

  • trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

    r246707 r246709  
    6161
    6262const ClassInfo JSFunction::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSFunction) };
     63const ClassInfo JSStrictFunction::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSStrictFunction) };
     64const ClassInfo JSSloppyFunction::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSSloppyFunction) };
     65const ClassInfo JSArrowFunction::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSArrowFunction) };
    6366
    6467bool JSFunction::isHostFunctionNonInline() const

  • trunk/Source/JavaScriptCore/runtime/JSFunction.h

    r246707 r246709  
    227227};
    228228
     229class JSStrictFunction final : public JSFunction {
     230public:
     231    using Base = JSFunction;
     232
     233    DECLARE_EXPORT_INFO;
     234
     235    static constexpr unsigned StructureFlags = Base::StructureFlags;
     236
     237    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     238    {
     239        ASSERT(globalObject);
     240        return Structure::create(vm, globalObject, prototype, TypeInfo(JSFunctionType, StructureFlags), info());
     241    }
     242};
     243static_assert(sizeof(JSStrictFunction) == sizeof(JSFunction), "Allocated in JSFunction IsoSubspace");
     244
     245class JSSloppyFunction final : public JSFunction {
     246public:
     247    using Base = JSFunction;
     248
     249    DECLARE_EXPORT_INFO;
     250
     251    static constexpr unsigned StructureFlags = Base::StructureFlags;
     252
     253    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     254    {
     255        ASSERT(globalObject);
     256        return Structure::create(vm, globalObject, prototype, TypeInfo(JSFunctionType, StructureFlags), info());
     257    }
     258};
     259static_assert(sizeof(JSSloppyFunction) == sizeof(JSFunction), "Allocated in JSFunction IsoSubspace");
     260
     261class JSArrowFunction final : public JSFunction {
     262public:
     263    using Base = JSFunction;
     264
     265    DECLARE_EXPORT_INFO;
     266
     267    static constexpr unsigned StructureFlags = Base::StructureFlags;
     268
     269    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     270    {
     271        ASSERT(globalObject);
     272        return Structure::create(vm, globalObject, prototype, TypeInfo(JSFunctionType, StructureFlags), info());
     273    }
     274};
     275static_assert(sizeof(JSArrowFunction) == sizeof(JSFunction), "Allocated in JSFunction IsoSubspace");
     276
    229277} // namespace JSC

  • trunk/Source/JavaScriptCore/runtime/JSGeneratorFunction.h

    r240965 r246709  
    6767    const static unsigned StructureFlags = Base::StructureFlags;
    6868
    69     template<typename CellType, SubspaceAccess>
    70     static IsoSubspace* subspaceFor(VM& vm)
    71     {
    72         return &vm.functionSpace;
    73     }
    74 
    7569    DECLARE_EXPORT_INFO;
    7670

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r246567 r246709  
    473473
    474474    auto initFunctionStructures = [&] (FunctionStructures& structures) {
    475         structures.strictFunctionStructure.set(vm, this, JSFunction::createStructure(vm, this, m_functionPrototype.get()));
    476         structures.sloppyFunctionStructure.set(vm, this, JSFunction::createStructure(vm, this, m_functionPrototype.get()));
    477         structures.arrowFunctionStructure.set(vm, this, JSFunction::createStructure(vm, this, m_functionPrototype.get()));
     475        structures.strictFunctionStructure.set(vm, this, JSStrictFunction::createStructure(vm, this, m_functionPrototype.get()));
     476        structures.sloppyFunctionStructure.set(vm, this, JSSloppyFunction::createStructure(vm, this, m_functionPrototype.get()));
     477        structures.arrowFunctionStructure.set(vm, this, JSArrowFunction::createStructure(vm, this, m_functionPrototype.get()));
    478478    };
    479479    initFunctionStructures(m_builtinFunctions);
Note: See TracChangeset for help on using the changeset viewer.