Commit 67192765 authored by oliver@apple.com's avatar oliver@apple.com

2010-01-31 Oliver Hunt <oliver@apple.com>

        Reviewed by Maciej Stachowiak.

        JSC is failing to propagate anonymous slot count on some transitions
        https://bugs.webkit.org/show_bug.cgi?id=34321

        Remove secondary Structure constructor, and make Structure store a copy
        of the number of anonymous slots directly so saving an immediate allocation
        of a property map for all structures with anonymous storage, which also
        avoids the leaked property map on new property transition in the original
        version of this patch.

        We need to propagate the the anonymous slot count otherwise we can end up
        with a structure recording incorrect information about the available and
        needed space for property storage, or alternatively incorrectly reusing
        some slots.

        * JavaScriptCore.exp:
        * runtime/Structure.cpp:
        (JSC::Structure::Structure):
        (JSC::Structure::materializePropertyMap):
        (JSC::Structure::addPropertyTransition):
        (JSC::Structure::changePrototypeTransition):
        (JSC::Structure::despecifyFunctionTransition):
        (JSC::Structure::getterSetterTransition):
        (JSC::Structure::toDictionaryTransition):
        (JSC::Structure::flattenDictionaryStructure):
        (JSC::Structure::copyPropertyTable):
        (JSC::Structure::put):
        (JSC::Structure::remove):
        (JSC::Structure::insertIntoPropertyMapHashTable):
        (JSC::Structure::createPropertyMapHashTable):
        * runtime/Structure.h:
        (JSC::Structure::create):
        (JSC::Structure::hasAnonymousSlots):
        (JSC::Structure::anonymousSlotCount):
2010-02-01  Oliver Hunt  <oliver@apple.com>

        Reviewed by Maciej Stachowiak.

        JSC is failing to propagate anonymous slot count on some transitions
        https://bugs.webkit.org/show_bug.cgi?id=34321

        Add test case for modifying DOM objects with anonymous storage.

        * fast/dom/Window/anonymous-slot-with-changes-expected.txt: Added.
        * fast/dom/Window/anonymous-slot-with-changes.html: Added.

git-svn-id: svn://svn.chromium.org/blink/trunk@54129 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent e932cfd9
2010-01-31 Oliver Hunt <oliver@apple.com>
Reviewed by Maciej Stachowiak.
JSC is failing to propagate anonymous slot count on some transitions
https://bugs.webkit.org/show_bug.cgi?id=34321
Remove secondary Structure constructor, and make Structure store a copy
of the number of anonymous slots directly so saving an immediate allocation
of a property map for all structures with anonymous storage, which also
avoids the leaked property map on new property transition in the original
version of this patch.
We need to propagate the the anonymous slot count otherwise we can end up
with a structure recording incorrect information about the available and
needed space for property storage, or alternatively incorrectly reusing
some slots.
* JavaScriptCore.exp:
* runtime/Structure.cpp:
(JSC::Structure::Structure):
(JSC::Structure::materializePropertyMap):
(JSC::Structure::addPropertyTransition):
(JSC::Structure::changePrototypeTransition):
(JSC::Structure::despecifyFunctionTransition):
(JSC::Structure::getterSetterTransition):
(JSC::Structure::toDictionaryTransition):
(JSC::Structure::flattenDictionaryStructure):
(JSC::Structure::copyPropertyTable):
(JSC::Structure::put):
(JSC::Structure::remove):
(JSC::Structure::insertIntoPropertyMapHashTable):
(JSC::Structure::createPropertyMapHashTable):
* runtime/Structure.h:
(JSC::Structure::create):
(JSC::Structure::hasAnonymousSlots):
(JSC::Structure::anonymousSlotCount):
2010-01-31 Patrick Gansterer <paroga@paroga.com>
Reviewed by Darin Adler.
......
......@@ -292,7 +292,7 @@ __ZN3JSC9Structure27despecifyFunctionTransitionEPS0_RKNS_10IdentifierE
__ZN3JSC9Structure28addPropertyWithoutTransitionERKNS_10IdentifierEjPNS_6JSCellE
__ZN3JSC9Structure3getEPKNS_11UStringImplERjRPNS_6JSCellE
__ZN3JSC9Structure40addPropertyTransitionToExistingStructureEPS0_RKNS_10IdentifierEjPNS_6JSCellERm
__ZN3JSC9StructureC1ENS_7JSValueERKNS_8TypeInfoE
__ZN3JSC9StructureC1ENS_7JSValueERKNS_8TypeInfoEj
__ZN3JSC9StructureD1Ev
__ZN3JSC9constructEPNS_9ExecStateENS_7JSValueENS_13ConstructTypeERKNS_13ConstructDataERKNS_7ArgListE
__ZN3JSCeqERKNS_7UStringEPKc
......
......@@ -123,7 +123,7 @@ void Structure::dumpStatistics()
#endif
}
Structure::Structure(JSValue prototype, const TypeInfo& typeInfo)
Structure::Structure(JSValue prototype, const TypeInfo& typeInfo, unsigned anonymousSlotCount)
: m_typeInfo(typeInfo)
, m_prototype(prototype)
, m_specificValueInPrevious(0)
......@@ -135,6 +135,7 @@ Structure::Structure(JSValue prototype, const TypeInfo& typeInfo)
, m_hasGetterSetterProperties(false)
, m_attributesInPrevious(0)
, m_specificFunctionThrashCount(0)
, m_anonymousSlotCount(anonymousSlotCount)
{
ASSERT(m_prototype);
ASSERT(m_prototype.isObject() || m_prototype.isNull());
......@@ -271,7 +272,8 @@ void Structure::materializePropertyMap()
if (sizeForKeyCount(m_offset + 1) > m_propertyTable->size)
rehashPropertyMapHashTable(sizeForKeyCount(m_offset + 1)); // This could be made more efficient by combining with the copy above.
}
m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
for (ptrdiff_t i = structures.size() - 2; i >= 0; --i) {
structure = structures[i];
structure->m_nameInPrevious->ref();
......@@ -366,7 +368,7 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
return transition.release();
}
RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo());
RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo(), structure->anonymousSlotCount());
transition->m_cachedPrototypeChain = structure->m_cachedPrototypeChain;
transition->m_previous = structure;
......@@ -379,6 +381,7 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
transition->m_specificFunctionThrashCount = structure->m_specificFunctionThrashCount;
if (structure->m_propertyTable) {
ASSERT(structure->m_propertyTable->anonymousSlotCount == structure->m_anonymousSlotCount);
if (structure->m_isPinnedPropertyTable)
transition->m_propertyTable = structure->copyPropertyTable();
else {
......@@ -397,7 +400,7 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
transition->growPropertyStorageCapacity();
transition->m_offset = offset;
ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
structure->table.add(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
return transition.release();
}
......@@ -415,7 +418,7 @@ PassRefPtr<Structure> Structure::removePropertyTransition(Structure* structure,
PassRefPtr<Structure> Structure::changePrototypeTransition(Structure* structure, JSValue prototype)
{
RefPtr<Structure> transition = create(prototype, structure->typeInfo());
RefPtr<Structure> transition = create(prototype, structure->typeInfo(), structure->anonymousSlotCount());
transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
transition->m_hasGetterSetterProperties = structure->m_hasGetterSetterProperties;
......@@ -427,14 +430,15 @@ PassRefPtr<Structure> Structure::changePrototypeTransition(Structure* structure,
structure->materializePropertyMapIfNecessary();
transition->m_propertyTable = structure->copyPropertyTable();
transition->m_isPinnedPropertyTable = true;
ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
return transition.release();
}
PassRefPtr<Structure> Structure::despecifyFunctionTransition(Structure* structure, const Identifier& replaceFunction)
{
ASSERT(structure->m_specificFunctionThrashCount < maxSpecificFunctionThrashCount);
RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo());
RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo(), structure->anonymousSlotCount());
transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
transition->m_hasGetterSetterProperties = structure->m_hasGetterSetterProperties;
......@@ -453,13 +457,14 @@ PassRefPtr<Structure> Structure::despecifyFunctionTransition(Structure* structur
bool removed = transition->despecifyFunction(replaceFunction);
ASSERT_UNUSED(removed, removed);
}
ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
return transition.release();
}
PassRefPtr<Structure> Structure::getterSetterTransition(Structure* structure)
{
RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo());
RefPtr<Structure> transition = create(structure->storedPrototype(), structure->typeInfo(), structure->anonymousSlotCount());
transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
transition->m_hasGetterSetterProperties = transition->m_hasGetterSetterProperties;
transition->m_hasNonEnumerableProperties = structure->m_hasNonEnumerableProperties;
......@@ -470,7 +475,8 @@ PassRefPtr<Structure> Structure::getterSetterTransition(Structure* structure)
structure->materializePropertyMapIfNecessary();
transition->m_propertyTable = structure->copyPropertyTable();
transition->m_isPinnedPropertyTable = true;
ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
return transition.release();
}
......@@ -478,7 +484,7 @@ PassRefPtr<Structure> Structure::toDictionaryTransition(Structure* structure, Di
{
ASSERT(!structure->isUncacheableDictionary());
RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo());
RefPtr<Structure> transition = create(structure->m_prototype, structure->typeInfo(), structure->anonymousSlotCount());
transition->m_dictionaryKind = kind;
transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity;
transition->m_hasGetterSetterProperties = structure->m_hasGetterSetterProperties;
......@@ -489,6 +495,7 @@ PassRefPtr<Structure> Structure::toDictionaryTransition(Structure* structure, Di
transition->m_propertyTable = structure->copyPropertyTable();
transition->m_isPinnedPropertyTable = true;
ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
return transition.release();
}
......@@ -522,6 +529,7 @@ PassRefPtr<Structure> Structure::flattenDictionaryStructure(JSObject* object)
// in the order that they are expected to be in, but we need to
// reorder the storage, so we have to copy the current values out
Vector<JSValue> values(propertyCount);
ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
unsigned anonymousSlotCount = m_propertyTable->anonymousSlotCount;
for (unsigned i = 0; i < propertyCount; i++) {
PropertyMapEntry* entry = sortedPropertyEntries[i];
......@@ -612,7 +620,8 @@ PropertyMapHashTable* Structure::copyPropertyTable()
{
if (!m_propertyTable)
return 0;
ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
size_t tableSize = PropertyMapHashTable::allocationSize(m_propertyTable->size);
PropertyMapHashTable* newTable = static_cast<PropertyMapHashTable*>(fastMalloc(tableSize));
memcpy(newTable, m_propertyTable, tableSize);
......@@ -754,6 +763,7 @@ size_t Structure::put(const Identifier& propertyName, unsigned attributes, JSCel
if (!m_propertyTable)
createPropertyMapHashTable();
ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
// FIXME: Consider a fast case for tables with no deleted sentinels.
......@@ -848,7 +858,8 @@ size_t Structure::remove(const Identifier& propertyName)
if (!m_propertyTable)
return notFound;
ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
#if DUMP_PROPERTYMAP_STATS
++numProbes;
++numRemoves;
......@@ -912,7 +923,8 @@ size_t Structure::remove(const Identifier& propertyName)
void Structure::insertIntoPropertyMapHashTable(const PropertyMapEntry& entry)
{
ASSERT(m_propertyTable);
ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
unsigned i = entry.key->existingHash();
unsigned k = 0;
......@@ -962,6 +974,7 @@ void Structure::createPropertyMapHashTable(unsigned newTableSize)
m_propertyTable = static_cast<PropertyMapHashTable*>(fastZeroedMalloc(PropertyMapHashTable::allocationSize(newTableSize)));
m_propertyTable->size = newTableSize;
m_propertyTable->sizeMask = newTableSize - 1;
m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
checkConsistency();
}
......
......@@ -62,15 +62,7 @@ namespace JSC {
friend class StructureTransitionTable;
static PassRefPtr<Structure> create(JSValue prototype, const TypeInfo& typeInfo, unsigned anonymousSlotCount)
{
Structure* structure = (new Structure(prototype, typeInfo));
if (anonymousSlotCount) {
structure->materializePropertyMap();
structure->m_isPinnedPropertyTable = true;
structure->m_propertyTable->anonymousSlotCount = anonymousSlotCount;
// Currently we don't allow more anonymous slots than fit in the inline capacity
ASSERT(structure->propertyStorageSize() <= structure->propertyStorageCapacity());
}
return adoptRef(structure);
return adoptRef(new Structure(prototype, typeInfo, anonymousSlotCount));
}
static void startIgnoringLeaks();
......@@ -134,8 +126,8 @@ namespace JSC {
bool hasNonEnumerableProperties() const { return m_hasNonEnumerableProperties; }
bool hasAnonymousSlots() const { return m_propertyTable && m_propertyTable->anonymousSlotCount; }
unsigned anonymousSlotCount() const { return m_propertyTable ? m_propertyTable->anonymousSlotCount : 0; }
bool hasAnonymousSlots() const { return !!m_anonymousSlotCount; }
unsigned anonymousSlotCount() const { return m_anonymousSlotCount; }
bool isEmpty() const { return m_propertyTable ? !m_propertyTable->keyCount : m_offset == noOffset; }
......@@ -147,12 +139,8 @@ namespace JSC {
void getPropertyNames(PropertyNameArray&, EnumerationMode mode);
private:
static PassRefPtr<Structure> create(JSValue prototype, const TypeInfo& typeInfo)
{
return adoptRef(new Structure(prototype, typeInfo));
}
Structure(JSValue prototype, const TypeInfo&);
Structure(JSValue prototype, const TypeInfo&, unsigned anonymousSlotCount);
typedef enum {
NoneDictionaryKind = 0,
......@@ -231,7 +219,8 @@ namespace JSC {
unsigned m_attributesInPrevious : 7;
#endif
unsigned m_specificFunctionThrashCount : 2;
// 10 free bits
unsigned m_anonymousSlotCount : 5;
// 5 free bits
};
inline size_t Structure::get(const Identifier& propertyName)
......
2010-02-01 Oliver Hunt <oliver@apple.com>
Reviewed by Maciej Stachowiak.
JSC is failing to propagate anonymous slot count on some transitions
https://bugs.webkit.org/show_bug.cgi?id=34321
Add test case for modifying DOM objects with anonymous storage.
* fast/dom/Window/anonymous-slot-with-changes-expected.txt: Added.
* fast/dom/Window/anonymous-slot-with-changes.html: Added.
2010-01-31 Kent Tamura <tkent@chromium.org>
Reviewed by Darin Adler.
......
Tests that we clone object hierarchies
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is undefined of type undefined
PASS: eventData is undefined of type undefined
PASS: eventData is undefined of type undefined
PASS: eventData is undefined of type undefined
PASS: eventData is undefined of type undefined
PASS: eventData is 1 of type number
PASS: eventData is 1 of type number
PASS: eventData is 1 of type number
PASS: eventData is 1 of type number
PASS: eventData is 1 of type number
PASS: eventData is true of type boolean
PASS: eventData is true of type boolean
PASS: eventData is true of type boolean
PASS: eventData is true of type boolean
PASS: eventData is true of type boolean
PASS: eventData is 1 of type string
PASS: eventData is 1 of type string
PASS: eventData is 1 of type string
PASS: eventData is 1 of type string
PASS: eventData is 1 of type string
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is [object Object] of type object
PASS: eventData is of type object
PASS: eventData is of type object
PASS: eventData is of type object
PASS: eventData is of type object
PASS: eventData is of type object
PASS: eventData is 1,2,3 of type object
PASS: eventData is 1,2,3 of type object
PASS: eventData is 1,2,3 of type object
PASS: eventData is 1,2,3 of type object
PASS: eventData is 1,2,3 of type object
PASS: eventData is ,,1 of type object
PASS: eventData is ,,1 of type object
PASS: eventData is ,,1 of type object
PASS: eventData is ,,1 of type object
PASS: eventData is ,,1 of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is null of type object
PASS: eventData is 2009-02-13T23:31:30.000Z of type object
PASS: eventData is 2009-02-13T23:31:30.000Z of type object
PASS: eventData is 2009-02-13T23:31:30.000Z of type object
PASS: eventData is 2009-02-13T23:31:30.000Z of type object
PASS: eventData is 2009-02-13T23:31:30.000Z of type object
PASS: eventData is done of type string
PASS: eventData is done of type string
PASS: eventData is done of type string
PASS: eventData is done of type string
PASS: eventData is done of type string
<html>
<head></head>
<body>
<div id="description"></div>
<div id="console"></div>
<input type="file" id="fileInput"></input>
<script>
if (window.layoutTestController) {
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
var console = document.getElementById("console");
var messages = [];
function equal(actual, expected)
{
if (typeof actual !== typeof expected)
return false;
if (actual === expected)
return true;
if ((actual instanceof Date) || (expected instanceof Date)) {
if ((actual instanceof Date) && (expected instanceof Date))
return (expected instanceof Date) && actual.getTime() == expected.getTime();
return false;
}
if (Array.isArray(actual) || Array.isArray(expected)) {
if (!Array.isArray(actual) || !Array.isArray(expected))
return false;
if (actual.length != expected.length)
return false;
for (var i = 0; i < actual.length; i++) {
if ((i in actual) ^ (i in expected))
return false;
if (!equal(actual[i], expected[i]))
return false;
}
return true;
}
if (actual.constructor !== expected.constructor)
return false;
var keys = Object.keys(actual);
if (!equal(keys, Object.keys(expected)))
return false;
for (var i = 0; i < keys.length; i++) {
if (!equal(actual[keys[i]], expected[keys[i]]))
return false;
}
return true;
}
function safeToString(o) {
if (o instanceof Date)
return o.toISOString();
if (typeof o !== "object" || !o)
return o;
try {
return o.toString();
} catch (e) {
return Object.prototype.toString.call(o) + "(default toString threw "+e+")";
}
}
function shouldBe(actual, expected)
{
var actualValue = eval(actual);
var expectedValue = eval(expected);
if (equal(actualValue, expectedValue))
console.innerHTML += "PASS: " + actual + " is " + safeToString(expectedValue) + " of type " + typeof actualValue + "<br>";
else
console.innerHTML += "FAIL: " + actual + " is " + actualValue + " should be " + expectedValue + " of type " + typeof expectedValue + "<br>";
}
function onmessage(evt) {
evt.foo = "bar"; // add a property.
eventData = evt.data;
shouldBe("eventData", messages[0]);
evt.func = function(){}; // add a function property.
eventData = evt.data;
shouldBe("eventData", messages[0]);
evt.func = ""; // despecialise a function property.
eventData = evt.data;
shouldBe("eventData", messages[0]);
for (var i = 0; i < 1000; i++) // To dictionary transition
evt[i] = i;
eventData = evt.data;
shouldBe("eventData", messages[0]);
delete evt.foo; // To uncachable dictionary
eventData = evt.data;
shouldBe("eventData", messages[0]);
messages.shift();
if (safeToString(evt.data) == 'done' && window.layoutTestController)
layoutTestController.notifyDone();
}
window.addEventListener('message', onmessage, false);
function tryPostMessage(message, shouldThrow, expected) {
try {
var value = eval(message);
postMessage(value, "*");
if (shouldThrow)
console.innerHTML += "FAIL: 'postMessage("+message+")' should throw but didn't<br>";
messages.push(expected || message);
} catch(e) {
if (shouldThrow)
console.innerHTML += "PASS: 'postMessage("+message+")' threw " + e + "<br>";
else
console.innerHTML += "FAIL: 'postMessage("+message+")' should not throw but threw " + e + "<br>";
}
}
document.getElementById("description").innerHTML = "Tests that we clone object hierarchies";
tryPostMessage('null');
tryPostMessage('undefined');
tryPostMessage('1');
tryPostMessage('true');
tryPostMessage('"1"');
tryPostMessage('({})');
tryPostMessage('({a:1})');
tryPostMessage('[]');
tryPostMessage('[1,2,3]');
tryPostMessage('[,,1]');
tryPostMessage('(function(){})', false, 'null');
tryPostMessage('new Date(1234567890000)');
tryPostMessage('"done"');
</script>
</body>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment