Ignore:
Timestamp:
Sep 17, 2012, 1:56:39 PM (13 years ago)
Author:
[email protected]
Message:

If a prototype has indexed setters and its instances have indexed storage, then all put_by_val's should have a bad time
​https://bugs.webkit.org/show_bug.cgi?id=96596

Reviewed by Gavin Barraclough.

Source/JavaScriptCore:

Added comprehensive support for accessors and read-only indexed properties on the
prototype chain. This is done without any performance regression on benchmarks that
we're aware of, by having the entire VM's strategy with respect to arrays tilted
heavily in favor of:

  • The prototype chain of JSArrays never having any accessors or read-only indexed properties. If that changes, you're going to have a bad time.


  • Prototypes of non-JSArray objects either having no indexed accessors or read-only indexed properties, or, having those indexed accessor thingies inserted before any instance object (i.e. object with that prototype as its prototype) is created. If you add indexed accessors or read-only indexed properties to an object that is already used as a prototype, you're going to have a bad time.


See below for the exact definition of having a bad time.

Put another way, "fair" uses of indexed accessors and read-only indexed properties
are:

  • Put indexed accessors and read-only indexed properties on an object that is never used as a prototype. This will slow down accesses to that object, but will not have any effect on any other object.


  • Put those indexed accessor thingies on an object before it is used as a prototype and then start instantiating objects that claim that object as their prototype. This will slightly slow down indexed stores to the instance objects, and greatly slow down all indexed accesses to the prototype, but will have no other effect.


In short, "fair" uses only affect the object itself and any instance objects. But
if you start using indexed accessors in more eclectic ways, you're going to have
a bad time.

Specifically, if an object that may be used as a prototype has an indexed accessor
added, the VM performs a whole-heap scan to find all objects that belong to the
same global object as the prototype you modified. If any of those objects has
indexed storage, their indexed storage is put into slow-put mode, just as if their
prototype chain had indexed accessors. This will happen even for objects that do
not currently have indexed accessors in their prototype chain. As well, all JSArray
allocations are caused to create arrays with slow-put storage, and all future
allocations of indexed storage for non-JSArray objects are also flipped to slow-put
mode. Note there are two aspects to having a bad time: (i) the whole-heap scan and
(ii) the poisoning of all indexed storage in the entire global object. (i) is
necessary for correctness. If we detect that an object that may be used as a
prototype has had an indexed accessor or indexed read-only property inserted into
it, then we need to ensure that henceforth all instances of that object inspect
the prototype chain whenever an indexed hole is stored to. But by default, indexed
stores do no such checking because doing so would be unnecessarily slow. So, we must
find all instances of the affected object and flip them into a different array
storage mode that omits all hole optimizations. Since prototypes never keep a list
of instance objects, the only way to find those objects is a whole-heap scan. But
(i) alone would be a potential disaster, if a program frequently allocated an
object without indexed accessors, then allocated a bunch of objects that used that
one as their prototype, and then added indexed accessors to the prototype. So, to
prevent massive heap scan storms in such awkward programs, having a bad time also
implies (ii): henceforth *all* objects belonging to that global object will use
slow put indexed storage, so that we don't ever have to scan the heap again. Note
that here we are using the global object as just an approximation of a program
module; it may be worth investigating in the future if other approximations can be
used instead.

  • bytecode/ArrayProfile.h:

(JSC):
(JSC::arrayModeFromStructure):

  • dfg/DFGAbstractState.cpp:

(JSC::DFG::AbstractState::execute):

  • dfg/DFGArrayMode.cpp:

(JSC::DFG::fromObserved):
(JSC::DFG::modeAlreadyChecked):
(JSC::DFG::modeToString):

  • dfg/DFGArrayMode.h:

(DFG):
(JSC::DFG::isSlowPutAccess):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::checkArray):

  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • jit/JIT.h:
  • jit/JITInlineMethods.h:

(JSC::JIT::emitAllocateJSArray):

  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_new_array):

  • runtime/ArrayPrototype.cpp:

(JSC::ArrayPrototype::finishCreation):
(JSC::arrayProtoFuncSort):

  • runtime/IndexingType.h:

(JSC):
(JSC::hasIndexedProperties):
(JSC::hasIndexingHeader):
(JSC::hasArrayStorage):
(JSC::shouldUseSlowPut):

  • runtime/JSArray.cpp:

(JSC::JSArray::pop):
(JSC::JSArray::push):
(JSC::JSArray::fillArgList):
(JSC::JSArray::copyToArguments):

  • runtime/JSArray.h:

(JSC::JSArray::createStructure):

  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::reset):
(JSC):
(JSC::JSGlobalObject::haveABadTime):

  • runtime/JSGlobalObject.h:

(JSGlobalObject):
(JSC::JSGlobalObject::addressOfArrayStructure):
(JSC::JSGlobalObject::havingABadTimeWatchpoint):
(JSC::JSGlobalObject::isHavingABadTime):

  • runtime/JSObject.cpp:

(JSC::JSObject::visitButterfly):
(JSC::JSObject::getOwnPropertySlotByIndex):
(JSC::JSObject::put):
(JSC::JSObject::putByIndex):
(JSC::JSObject::enterDictionaryIndexingMode):
(JSC::JSObject::notifyPresenceOfIndexedAccessors):
(JSC):
(JSC::JSObject::createArrayStorage):
(JSC::JSObject::ensureArrayStorageExistsAndEnterDictionaryIndexingMode):
(JSC::JSObject::switchToSlowPutArrayStorage):
(JSC::JSObject::setPrototype):
(JSC::JSObject::resetInheritorID):
(JSC::JSObject::inheritorID):
(JSC::JSObject::allowsAccessFrom):
(JSC::JSObject::deletePropertyByIndex):
(JSC::JSObject::getOwnPropertyNames):
(JSC::JSObject::unwrappedGlobalObject):
(JSC::JSObject::notifyUsedAsPrototype):
(JSC::JSObject::createInheritorID):
(JSC::JSObject::defineOwnIndexedProperty):
(JSC::JSObject::attemptToInterceptPutByIndexOnHoleForPrototype):
(JSC::JSObject::attemptToInterceptPutByIndexOnHole):
(JSC::JSObject::putByIndexBeyondVectorLength):
(JSC::JSObject::putDirectIndexBeyondVectorLength):
(JSC::JSObject::getNewVectorLength):
(JSC::JSObject::getOwnPropertyDescriptor):

  • runtime/JSObject.h:

(JSC::JSObject::mayBeUsedAsPrototype):
(JSObject):
(JSC::JSObject::mayInterceptIndexedAccesses):
(JSC::JSObject::getArrayLength):
(JSC::JSObject::getVectorLength):
(JSC::JSObject::canGetIndexQuickly):
(JSC::JSObject::getIndexQuickly):
(JSC::JSObject::canSetIndexQuickly):
(JSC::JSObject::setIndexQuickly):
(JSC::JSObject::initializeIndex):
(JSC::JSObject::completeInitialization):
(JSC::JSObject::inSparseIndexingMode):
(JSC::JSObject::arrayStorage):
(JSC::JSObject::arrayStorageOrNull):
(JSC::JSObject::ensureArrayStorage):
(JSC):
(JSC::JSValue::putByIndex):

  • runtime/JSValue.cpp:

(JSC::JSValue::putToPrimitive):
(JSC::JSValue::putToPrimitiveByIndex):
(JSC):

  • runtime/JSValue.h:

(JSValue):

  • runtime/ObjectPrototype.cpp:

(JSC::ObjectPrototype::finishCreation):

  • runtime/SparseArrayValueMap.cpp:

(JSC::SparseArrayValueMap::putEntry):
(JSC::SparseArrayEntry::put):
(JSC):

  • runtime/SparseArrayValueMap.h:

(JSC):
(SparseArrayEntry):

  • runtime/Structure.cpp:

(JSC::Structure::anyObjectInChainMayInterceptIndexedAccesses):
(JSC):
(JSC::Structure::suggestedIndexingTransition):

  • runtime/Structure.h:

(Structure):
(JSC::Structure::mayInterceptIndexedAccesses):

  • runtime/StructureTransitionTable.h:

(JSC::newIndexingType):

LayoutTests:

Removed failing expectation for primitive-property-access-edge-cases, and
added more tests to cover the numerical-setter-on-prototype cases.

  • fast/js/array-bad-time-expected.txt: Added.
  • fast/js/array-bad-time.html: Added.
  • fast/js/array-slow-put-expected.txt: Added.
  • fast/js/array-slow-put.html: Added.
  • fast/js/cross-frame-bad-time-expected.txt: Added.
  • fast/js/cross-frame-bad-time.html: Added.
  • fast/js/jsc-test-list:
  • fast/js/object-bad-time-expected.txt: Added.
  • fast/js/object-bad-time.html: Added.
  • fast/js/object-slow-put-expected.txt: Added.
  • fast/js/object-slow-put.html: Added.
  • fast/js/script-tests/array-bad-time.js: Added.
  • fast/js/script-tests/array-slow-put.js: Added.

(foo):

  • fast/js/script-tests/cross-frame-bad-time.js: Added.

(foo):

  • fast/js/script-tests/object-bad-time.js: Added.

(Cons):

  • fast/js/script-tests/object-slow-put.js: Added.

(Cons):
(foo):

  • platform/mac/fast/js/primitive-property-access-edge-cases-expected.txt: Removed.
File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r128680 r128802  
    100100        return;
    101101    }
    102 
    103     if (!(entry.attributes & Accessor)) {
    104         if (entry.attributes & ReadOnly) {
     102   
     103    entry.put(exec, array, this, value, shouldThrow);
     104}
     105
     106bool SparseArrayValueMap::putDirect(ExecState* exec, JSObject* array, unsigned i, JSValue value, unsigned attributes, PutDirectIndexMode mode)
     107{
     108    AddResult result = add(array, i);
     109    SparseArrayEntry& entry = result.iterator->second;
     110
     111    // To save a separate find & add, we first always add to the sparse map.
     112    // In the uncommon case that this is a new property, and the array is not
     113    // extensible, this is not the right thing to have done - so remove again.
     114    if (mode != PutDirectIndexLikePutDirect && result.isNewEntry && !array->isExtensible()) {
     115        remove(result.iterator);
     116        return reject(exec, mode == PutDirectIndexShouldThrow, "Attempting to define property on object that is not extensible.");
     117    }
     118
     119    entry.attributes = attributes;
     120    entry.set(exec->globalData(), this, value);
     121    return true;
     122}
     123
     124void SparseArrayEntry::get(PropertySlot& slot) const
     125{
     126    JSValue value = Base::get();
     127    ASSERT(value);
     128
     129    if (LIKELY(!value.isGetterSetter())) {
     130        slot.setValue(value);
     131        return;
     132    }
     133
     134    JSObject* getter = asGetterSetter(value)->getter();
     135    if (!getter) {
     136        slot.setUndefined();
     137        return;
     138    }
     139
     140    slot.setGetterSlot(getter);
     141}
     142
     143void SparseArrayEntry::get(PropertyDescriptor& descriptor) const
     144{
     145    descriptor.setDescriptor(Base::get(), attributes);
     146}
     147
     148JSValue SparseArrayEntry::get(ExecState* exec, JSObject* array) const
     149{
     150    JSValue result = Base::get();
     151    ASSERT(result);
     152
     153    if (LIKELY(!result.isGetterSetter()))
     154        return result;
     155
     156    JSObject* getter = asGetterSetter(result)->getter();
     157    if (!getter)
     158        return jsUndefined();
     159
     160    CallData callData;
     161    CallType callType = getter->methodTable()->getCallData(getter, callData);
     162    return call(exec, getter, callType, callData, array, exec->emptyList());
     163}
     164
     165void SparseArrayEntry::put(ExecState* exec, JSValue thisValue, SparseArrayValueMap* map, JSValue value, bool shouldThrow)
     166{
     167    if (!(attributes & Accessor)) {
     168        if (attributes & ReadOnly) {
    105169            if (shouldThrow)
    106170                throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
    … …  
    108172        }
    109173
    110         entry.set(exec->globalData(), this, value);
    111         return;
    112     }
    113 
    114     JSValue accessor = entry.SparseArrayEntry::Base::get();
     174        set(exec->globalData(), map, value);
     175        return;
     176    }
     177
     178    JSValue accessor = Base::get();
    115179    ASSERT(accessor.isGetterSetter());
    116180    JSObject* setter = asGetterSetter(accessor)->setter();
    … …  
    126190    MarkedArgumentBuffer args;
    127191    args.append(value);
    128     call(exec, setter, callType, callData, array, args);
    129 }
    130 
    131 bool SparseArrayValueMap::putDirect(ExecState* exec, JSObject* array, unsigned i, JSValue value, unsigned attributes, PutDirectIndexMode mode)
    132 {
    133     AddResult result = add(array, i);
    134     SparseArrayEntry& entry = result.iterator->second;
    135 
    136     // To save a separate find & add, we first always add to the sparse map.
    137     // In the uncommon case that this is a new property, and the array is not
    138     // extensible, this is not the right thing to have done - so remove again.
    139     if (mode != PutDirectIndexLikePutDirect && result.isNewEntry && !array->isExtensible()) {
    140         remove(result.iterator);
    141         return reject(exec, mode == PutDirectIndexShouldThrow, "Attempting to define property on object that is not extensible.");
    142     }
    143 
    144     entry.attributes = attributes;
    145     entry.set(exec->globalData(), this, value);
    146     return true;
    147 }
    148 
    149 void SparseArrayEntry::get(PropertySlot& slot) const
    150 {
    151     JSValue value = Base::get();
    152     ASSERT(value);
    153 
    154     if (LIKELY(!value.isGetterSetter())) {
    155         slot.setValue(value);
    156         return;
    157     }
    158 
    159     JSObject* getter = asGetterSetter(value)->getter();
    160     if (!getter) {
    161         slot.setUndefined();
    162         return;
    163     }
    164 
    165     slot.setGetterSlot(getter);
    166 }
    167 
    168 void SparseArrayEntry::get(PropertyDescriptor& descriptor) const
    169 {
    170     descriptor.setDescriptor(Base::get(), attributes);
    171 }
    172 
    173 JSValue SparseArrayEntry::get(ExecState* exec, JSObject* array) const
    174 {
    175     JSValue result = Base::get();
    176     ASSERT(result);
    177 
    178     if (LIKELY(!result.isGetterSetter()))
    179         return result;
    180 
    181     JSObject* getter = asGetterSetter(result)->getter();
    182     if (!getter)
    183         return jsUndefined();
    184 
    185     CallData callData;
    186     CallType callType = getter->methodTable()->getCallData(getter, callData);
    187     return call(exec, getter, callType, callData, array, exec->emptyList());
     192    call(exec, setter, callType, callData, thisValue, args);
    188193}
    189194
Note: See TracChangeset for help on using the changeset viewer.