Changeset 87586 in webkit


Ignore:
Timestamp:
May 27, 2011 4:53:36 PM (13 years ago)
Author:
ggaren@apple.com
Message:

2011-05-27 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

Property caching is too aggressive for API objects
https://bugs.webkit.org/show_bug.cgi?id=61677

  • API/JSCallbackObject.h: Opt in to ProhibitsPropertyCaching, since our callback APIs allow the client to change its mind about our propertis at any time.
  • API/tests/testapi.c: (PropertyCatchalls_getProperty): (PropertyCatchalls_setProperty): (PropertyCatchalls_getPropertyNames): (PropertyCatchalls_class): (main):
  • API/tests/testapi.js: Some tests for dynamic API objects.
  • interpreter/Interpreter.cpp: (JSC::Interpreter::tryCachePutByID): (JSC::Interpreter::tryCacheGetByID):
  • jit/JITStubs.cpp: (JSC::JITThunks::tryCachePutByID): (JSC::JITThunks::tryCacheGetByID): (JSC::DEFINE_STUB_FUNCTION): Opt out of property caching if the client requires it.
  • runtime/JSTypeInfo.h: (JSC::TypeInfo::TypeInfo): (JSC::TypeInfo::isFinal): (JSC::TypeInfo::prohibitsPropertyCaching): (JSC::TypeInfo::flags): Added a flag to track opting out of property caching. Fixed an "&&" vs "&" typo that was previously harmless, but is now harmful since m_flags2 can have more than one bit set.
Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSCallbackObject.h

    r87536 r87586  
    150150
    151151protected:
    152     static const unsigned StructureFlags = OverridesGetOwnPropertySlot | ImplementsHasInstance | OverridesHasInstance | OverridesVisitChildren | OverridesGetPropertyNames | Base::StructureFlags;
     152    static const unsigned StructureFlags = ProhibitsPropertyCaching | OverridesGetOwnPropertySlot | ImplementsHasInstance | OverridesHasInstance | OverridesVisitChildren | OverridesGetPropertyNames | Base::StructureFlags;
    153153
    154154private:
  • trunk/Source/JavaScriptCore/API/tests/testapi.c

    r79695 r87586  
    356356}
    357357
     358static JSValueRef PropertyCatchalls_getProperty(JSContextRef context, JSObjectRef object, JSStringRef propertyName, JSValueRef* exception)
     359{
     360    UNUSED_PARAM(context);
     361    UNUSED_PARAM(object);
     362    UNUSED_PARAM(propertyName);
     363    UNUSED_PARAM(exception);
     364
     365    if (JSStringIsEqualToUTF8CString(propertyName, "x")) {
     366        static size_t count;
     367        if (count++ < 5)
     368            return NULL;
     369
     370        // Swallow all .x gets after 5, returning null.
     371        return JSValueMakeNull(context);
     372    }
     373
     374    if (JSStringIsEqualToUTF8CString(propertyName, "y")) {
     375        static size_t count;
     376        if (count++ < 5)
     377            return NULL;
     378
     379        // Swallow all .y gets after 5, returning null.
     380        return JSValueMakeNull(context);
     381    }
     382   
     383    if (JSStringIsEqualToUTF8CString(propertyName, "z")) {
     384        static size_t count;
     385        if (count++ < 5)
     386            return NULL;
     387
     388        // Swallow all .y gets after 5, returning null.
     389        return JSValueMakeNull(context);
     390    }
     391
     392    return NULL;
     393}
     394
     395static bool PropertyCatchalls_setProperty(JSContextRef context, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* exception)
     396{
     397    UNUSED_PARAM(context);
     398    UNUSED_PARAM(object);
     399    UNUSED_PARAM(propertyName);
     400    UNUSED_PARAM(value);
     401    UNUSED_PARAM(exception);
     402
     403    if (JSStringIsEqualToUTF8CString(propertyName, "x")) {
     404        static size_t count;
     405        if (count++ < 5)
     406            return false;
     407
     408        // Swallow all .x sets after 4.
     409        return true;
     410    }
     411
     412    return false;
     413}
     414
     415static void PropertyCatchalls_getPropertyNames(JSContextRef context, JSObjectRef object, JSPropertyNameAccumulatorRef propertyNames)
     416{
     417    UNUSED_PARAM(context);
     418    UNUSED_PARAM(object);
     419
     420    static size_t count;
     421    static const char* numbers[] = { "0", "1", "2", "3", "4", "5", "6", "7", "8", "9" };
     422   
     423    // Provide a property of a different name every time.
     424    JSStringRef propertyName = JSStringCreateWithUTF8CString(numbers[count++ % 10]);
     425    JSPropertyNameAccumulatorAddName(propertyNames, propertyName);
     426    JSStringRelease(propertyName);
     427}
     428
     429JSClassDefinition PropertyCatchalls_definition = {
     430    0,
     431    kJSClassAttributeNone,
     432   
     433    "PropertyCatchalls",
     434    NULL,
     435   
     436    NULL,
     437    NULL,
     438   
     439    NULL,
     440    NULL,
     441    NULL,
     442    PropertyCatchalls_getProperty,
     443    PropertyCatchalls_setProperty,
     444    NULL,
     445    PropertyCatchalls_getPropertyNames,
     446    NULL,
     447    NULL,
     448    NULL,
     449    NULL,
     450};
     451
     452static JSClassRef PropertyCatchalls_class(JSContextRef context)
     453{
     454    UNUSED_PARAM(context);
     455
     456    static JSClassRef jsClass;
     457    if (!jsClass)
     458        jsClass = JSClassCreate(&PropertyCatchalls_definition);
     459   
     460    return jsClass;
     461}
     462
    358463static bool EvilExceptionObject_hasInstance(JSContextRef context, JSObjectRef constructor, JSValueRef possibleValue, JSValueRef* exception)
    359464{
     
    9221027    ASSERT(JSValueGetType(context, jsCFEmptyString) == kJSTypeString);
    9231028    ASSERT(JSValueGetType(context, jsCFEmptyStringWithCharacters) == kJSTypeString);
     1029
     1030    JSObjectRef propertyCatchalls = JSObjectMake(context, PropertyCatchalls_class(context), NULL);
     1031    JSStringRef propertyCatchallsString = JSStringCreateWithUTF8CString("PropertyCatchalls");
     1032    JSObjectSetProperty(context, globalObject, propertyCatchallsString, propertyCatchalls, kJSPropertyAttributeNone, NULL);
     1033    JSStringRelease(propertyCatchallsString);
    9241034
    9251035    JSObjectRef myObject = JSObjectMake(context, MyObject_class(context), NULL);
  • trunk/Source/JavaScriptCore/API/tests/testapi.js

    r53657 r87586  
    247247shouldBe("EmptyObject", "[object CallbackObject]");
    248248
     249for (var i = 0; i < 6; ++i)
     250    PropertyCatchalls.x = i;
     251shouldBe("PropertyCatchalls.x", 4);
     252
     253for (var i = 0; i < 6; ++i)
     254    var x = PropertyCatchalls.x;
     255shouldBe("x", null);
     256
     257for (var i = 0; i < 10; ++i) {
     258    for (var p in PropertyCatchalls) {
     259        if (p == "x")
     260            continue;
     261        shouldBe("p", i % 10);
     262        break;
     263    }
     264}
     265
     266PropertyCatchalls.__proto__ = { y: 1 };
     267for (var i = 0; i < 6; ++i)
     268    var y = PropertyCatchalls.y;
     269shouldBe("y", null);
     270
     271var o = { __proto__: PropertyCatchalls };
     272for (var i = 0; i < 6; ++i)
     273    var z = PropertyCatchalls.z;
     274shouldBe("z", null);
     275
    249276if (failed)
    250277    throw "Some tests failed";
    251 
  • trunk/Source/JavaScriptCore/ChangeLog

    r87580 r87586  
     12011-05-27  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        Property caching is too aggressive for API objects
     6        https://bugs.webkit.org/show_bug.cgi?id=61677
     7
     8        * API/JSCallbackObject.h: Opt in to ProhibitsPropertyCaching, since our
     9        callback APIs allow the client to change its mind about our propertis at
     10        any time.
     11
     12        * API/tests/testapi.c:
     13        (PropertyCatchalls_getProperty):
     14        (PropertyCatchalls_setProperty):
     15        (PropertyCatchalls_getPropertyNames):
     16        (PropertyCatchalls_class):
     17        (main):
     18        * API/tests/testapi.js: Some tests for dynamic API objects.
     19
     20        * interpreter/Interpreter.cpp:
     21        (JSC::Interpreter::tryCachePutByID):
     22        (JSC::Interpreter::tryCacheGetByID):
     23        * jit/JITStubs.cpp:
     24        (JSC::JITThunks::tryCachePutByID):
     25        (JSC::JITThunks::tryCacheGetByID):
     26        (JSC::DEFINE_STUB_FUNCTION): Opt out of property caching if the client
     27        requires it.
     28
     29        * runtime/JSTypeInfo.h:
     30        (JSC::TypeInfo::TypeInfo):
     31        (JSC::TypeInfo::isFinal):
     32        (JSC::TypeInfo::prohibitsPropertyCaching):
     33        (JSC::TypeInfo::flags): Added a flag to track opting out of property
     34        caching. Fixed an "&&" vs "&" typo that was previously harmless, but
     35        is now harmful since m_flags2 can have more than one bit set.
     36
    1372011-05-27  Stephanie Lewis  <slewis@apple.com>
    238
  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r86960 r87586  
    12401240    Structure* structure = baseCell->structure();
    12411241
    1242     if (structure->isUncacheableDictionary()) {
     1242    if (structure->isUncacheableDictionary() || structure->typeInfo().prohibitsPropertyCaching()) {
    12431243        vPC[0] = getOpcode(op_put_by_id_generic);
    12441244        return;
     
    13281328    Structure* structure = baseValue.asCell()->structure();
    13291329
    1330     if (structure->isUncacheableDictionary()) {
     1330    if (structure->isUncacheableDictionary() || structure->typeInfo().prohibitsPropertyCaching()) {
    13311331        vPC[0] = getOpcode(op_get_by_id_generic);
    13321332        return;
  • trunk/Source/JavaScriptCore/jit/JITStubs.cpp

    r87345 r87586  
    820820    Structure* structure = baseCell->structure();
    821821
    822     if (structure->isUncacheableDictionary()) {
     822    if (structure->isUncacheableDictionary() || structure->typeInfo().prohibitsPropertyCaching()) {
    823823        ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(direct ? cti_op_put_by_id_direct_generic : cti_op_put_by_id_generic));
    824824        return;
     
    888888    Structure* structure = baseCell->structure();
    889889
    890     if (structure->isUncacheableDictionary()) {
     890    if (structure->isUncacheableDictionary() || structure->typeInfo().prohibitsPropertyCaching()) {
    891891        ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
    892892        return;
     
    17231723    CHECK_FOR_EXCEPTION();
    17241724
    1725     if (!baseValue.isCell() || !slot.isCacheable() || baseValue.asCell()->structure()->isDictionary()) {
     1725    if (!baseValue.isCell() || !slot.isCacheable() || baseValue.asCell()->structure()->isDictionary() || baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching()) {
    17261726        ctiPatchCallByReturnAddress(callFrame->codeBlock(), STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
    17271727        return JSValue::encode(result);
  • trunk/Source/JavaScriptCore/runtime/JSTypeInfo.h

    r84556 r87586  
    3535namespace JSC {
    3636
    37     // WebCore uses MasqueradesAsUndefined to make document.all and style.filter undetectable.
    38     static const unsigned MasqueradesAsUndefined = 1;
     37    static const unsigned MasqueradesAsUndefined = 1; // WebCore uses MasqueradesAsUndefined to make document.all and style.filter undetectable.
    3938    static const unsigned ImplementsHasInstance = 1 << 1;
    4039    static const unsigned OverridesHasInstance = 1 << 2;
     
    4544    static const unsigned OverridesGetPropertyNames = 1 << 7;
    4645    static const unsigned IsJSFinalObject = 1 << 8;
     46    static const unsigned ProhibitsPropertyCaching = 1 << 9;
    4747
    4848    class TypeInfo {
     
    5050        TypeInfo(JSType type, unsigned flags = 0)
    5151            : m_type(type)
    52             , m_flags(flags & 0xFF)
     52            , m_flags(flags & 0xff)
    5353            , m_flags2(flags >> 8)
    5454        {
    55             ASSERT(flags <= 0x1FF);
    56             ASSERT(type <= 0xFF);
     55            ASSERT(flags <= 0x3ff);
     56            ASSERT(type <= 0xff);
    5757            ASSERT(type >= CompoundType || !(flags & OverridesVisitChildren));
    5858            // ImplementsDefaultHasInstance means (ImplementsHasInstance & !OverridesHasInstance)
     
    7070        bool overridesVisitChildren() const { return m_flags & OverridesVisitChildren; }
    7171        bool overridesGetPropertyNames() const { return m_flags & OverridesGetPropertyNames; }
     72        unsigned isFinal() const { return m_flags2 & (IsJSFinalObject >> 8); }
     73        unsigned prohibitsPropertyCaching() const { return m_flags2 & (ProhibitsPropertyCaching >> 8); }
    7274        unsigned flags() const { return m_flags; }
    73         unsigned isFinal() const { return m_flags2 && (IsJSFinalObject >> 8); }
    7475
    7576        static ptrdiff_t flagsOffset()
Note: See TracChangeset for help on using the changeset viewer.