Commit 907d1a40 authored by barraclough@apple.com's avatar barraclough@apple.com

https://bugs.webkit.org/show_bug.cgi?id=75140

Reviewed by Oliver Hunt.

Source/JavaScriptCore: 

Start cleaning up JSArray construction. JSArray has a set of create methods,
one of which (currently) takes a 'creation mode' enum parameter. Based on that
parameter, the constructor does one of two completely different things. If the
parameter is 'CreateInitialized' it creates an array, setting the length, but
does not eagerly allocate a storage vector of the specified length. A small
(BASE_VECTOR_LEN sized) initial vector will be allocated, and cleared, property
access to the vector will read the hole value (return undefined). The alternate
usage of this method ('CreateCompact') does something very different. It tries
to create an array of the requested length, and also allocates a storage vector
large enough to hold all properties. It does not clear the storage vector,
leaving the memory uninitialized and requiring the user to call a method
'uncheckedSetIndex' to initialize values in the vector.

This patch factors out these two behaviours, moving the 'CreateCompact' mode
into its own method, 'tryCreateUninitialized' (matching the naming for this
functionality in the string classes). 'tryCreateUninitialized' may return 0 if
memory allocation fails during construction of the object. The construction
pattern changes such that values added during initialization will be marked if
a GC is triggered during array allocation. 'CreateInitialized' no longer need
be passed to create a normal, fully constructed array with a length, and this
method is merged with the version of 'create' that does not take an initial
length (length parameter defaults to 0).

* JavaScriptCore.exp:
* runtime/ArrayConstructor.cpp:
(JSC::constructArrayWithSizeQuirk):
    - removed 'CreateInitialized' argument
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncSplice):
    - changed to call 'tryCreateUninitialized'
* runtime/FunctionPrototype.cpp:
(JSC::functionProtoFuncBind):
    - changed to call 'tryCreateUninitialized'
* runtime/JSArray.cpp:
(JSC::JSArray::JSArray):
    - initialize m_storage to null; if construction fails, make destruction safe
(JSC::JSArray::finishCreation):
    - merge versions of this method, takes an initialLength parameter defaulting to zero
(JSC::JSArray::tryFinishCreationUninitialized):
    - version of 'finishCreation' that tries to eagerly allocate storage; may fail & return 0
(JSC::JSArray::~JSArray):
    - check for null m_storage, in case array construction failed.
(JSC::JSArray::increaseVectorPrefixLength):
* runtime/JSArray.h:
(JSC::JSArray::create):
    - merge versions of this method, takes an initialLength parameter defaulting to zero
(JSC::JSArray::tryCreateUninitialized):
    - version of 'create' that tries to eagerly allocate storage; may fail & return 0
(JSC::JSArray::initializeIndex):
(JSC::JSArray::completeInitialization):
    - used in conjunction with 'tryCreateUninitialized' to initialize the array
* runtime/JSGlobalObject.h:
(JSC::constructEmptyArray):
    - removed 'CreateInitialized' argument
* runtime/RegExpConstructor.cpp:
(JSC::RegExpMatchesArray::finishCreation):
    - removed 'CreateInitialized' argument

LayoutTests: 

Added test case.

* fast/js/script-tests/array-splice.js:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@103823 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 710875db
2011-12-29 Gavin Barraclough <barraclough@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=75140
Reviewed by Oliver Hunt.
Added test case.
* fast/js/script-tests/array-splice.js:
2011-12-29 Tony Chang <tony@chromium.org>
[chromium] Unreviewed, unskip http/tests/plugins/geturlnotify-from-npp-destroystream.html
......@@ -31,3 +31,8 @@ shouldBe("arr.splice(2, -1)", "[]")
shouldBe("arr", "['a','b','c']");
shouldBe("arr.splice(2, 100)", "['c']")
shouldBe("arr", "['a','b']");
// Check this doesn't crash.
try {
String(Array(0xFFFFFFFD).splice(0));
} catch (e) { }
2011-12-29 Gavin Barraclough <barraclough@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=75140
Reviewed by Oliver Hunt.
Start cleaning up JSArray construction. JSArray has a set of create methods,
one of which (currently) takes a 'creation mode' enum parameter. Based on that
parameter, the constructor does one of two completely different things. If the
parameter is 'CreateInitialized' it creates an array, setting the length, but
does not eagerly allocate a storage vector of the specified length. A small
(BASE_VECTOR_LEN sized) initial vector will be allocated, and cleared, property
access to the vector will read the hole value (return undefined). The alternate
usage of this method ('CreateCompact') does something very different. It tries
to create an array of the requested length, and also allocates a storage vector
large enough to hold all properties. It does not clear the storage vector,
leaving the memory uninitialized and requiring the user to call a method
'uncheckedSetIndex' to initialize values in the vector.
This patch factors out these two behaviours, moving the 'CreateCompact' mode
into its own method, 'tryCreateUninitialized' (matching the naming for this
functionality in the string classes). 'tryCreateUninitialized' may return 0 if
memory allocation fails during construction of the object. The construction
pattern changes such that values added during initialization will be marked if
a GC is triggered during array allocation. 'CreateInitialized' no longer need
be passed to create a normal, fully constructed array with a length, and this
method is merged with the version of 'create' that does not take an initial
length (length parameter defaults to 0).
* JavaScriptCore.exp:
* runtime/ArrayConstructor.cpp:
(JSC::constructArrayWithSizeQuirk):
- removed 'CreateInitialized' argument
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncSplice):
- changed to call 'tryCreateUninitialized'
* runtime/FunctionPrototype.cpp:
(JSC::functionProtoFuncBind):
- changed to call 'tryCreateUninitialized'
* runtime/JSArray.cpp:
(JSC::JSArray::JSArray):
- initialize m_storage to null; if construction fails, make destruction safe
(JSC::JSArray::finishCreation):
- merge versions of this method, takes an initialLength parameter defaulting to zero
(JSC::JSArray::tryFinishCreationUninitialized):
- version of 'finishCreation' that tries to eagerly allocate storage; may fail & return 0
(JSC::JSArray::~JSArray):
- check for null m_storage, in case array construction failed.
(JSC::JSArray::increaseVectorPrefixLength):
* runtime/JSArray.h:
(JSC::JSArray::create):
- merge versions of this method, takes an initialLength parameter defaulting to zero
(JSC::JSArray::tryCreateUninitialized):
- version of 'create' that tries to eagerly allocate storage; may fail & return 0
(JSC::JSArray::initializeIndex):
(JSC::JSArray::completeInitialization):
- used in conjunction with 'tryCreateUninitialized' to initialize the array
* runtime/JSGlobalObject.h:
(JSC::constructEmptyArray):
- removed 'CreateInitialized' argument
* runtime/RegExpConstructor.cpp:
(JSC::RegExpMatchesArray::finishCreation):
- removed 'CreateInitialized' argument
2011-12-29 Anders Carlsson <andersca@apple.com>
Add a retainPtr function template
......@@ -280,7 +280,7 @@ __ZN3JSC6JSLockC1EPNS_9ExecStateE
__ZN3JSC6RegExp5matchERNS_12JSGlobalDataERKNS_7UStringEiPN3WTF6VectorIiLm32EEE
__ZN3JSC6RegExp6createERNS_12JSGlobalDataERKNS_7UStringENS_11RegExpFlagsE
__ZN3JSC7JSArray13visitChildrenEPNS_6JSCellERNS_11SlotVisitorE
__ZN3JSC7JSArray14finishCreationERNS_12JSGlobalDataE
__ZN3JSC7JSArray14finishCreationERNS_12JSGlobalDataEj
__ZN3JSC7JSArray14finishCreationERNS_12JSGlobalDataERKNS_7ArgListE
__ZN3JSC7JSArray15setSubclassDataEPv
__ZN3JSC7JSArray25getOwnPropertySlotByIndexEPNS_6JSCellEPNS_9ExecStateEjRNS_12PropertySlotE
......
......@@ -86,7 +86,7 @@ static inline JSObject* constructArrayWithSizeQuirk(ExecState* exec, const ArgLi
uint32_t n = args.at(0).toUInt32(exec);
if (n != args.at(0).toNumber(exec))
return throwError(exec, createRangeError(exec, "Array size is not a small enough positive integer."));
return JSArray::create(exec->globalData(), globalObject->arrayStructure(), n, CreateInitialized);
return JSArray::create(exec->globalData(), globalObject->arrayStructure(), n);
}
// otherwise the array is constructed with the arguments in it
......
......@@ -612,17 +612,19 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncSplice(ExecState* exec)
deleteCount = static_cast<unsigned>(deleteDouble);
}
JSArray* resObj = JSArray::create(exec->globalData(), exec->lexicalGlobalObject()->arrayStructure(), deleteCount, CreateCompact);
JSArray* resObj = JSArray::tryCreateUninitialized(exec->globalData(), exec->lexicalGlobalObject()->arrayStructure(), deleteCount);
if (!resObj)
return JSValue::encode(throwOutOfMemoryError(exec));
JSValue result = resObj;
JSGlobalData& globalData = exec->globalData();
for (unsigned k = 0; k < deleteCount; k++) {
JSValue v = getProperty(exec, thisObj, k + begin);
if (exec->hadException())
return JSValue::encode(jsUndefined());
resObj->uncheckedSetIndex(globalData, k, v);
resObj->initializeIndex(globalData, k, v);
}
resObj->setLength(deleteCount);
resObj->completeInitialization(deleteCount);
unsigned additionalArgs = std::max<int>(exec->argumentCount() - 2, 0);
if (additionalArgs != deleteCount) {
......
......@@ -187,10 +187,13 @@ EncodedJSValue JSC_HOST_CALL functionProtoFuncBind(ExecState* exec)
// Let A be a new (possibly empty) internal list of all of the argument values provided after thisArg (arg1, arg2 etc), in order.
size_t numBoundArgs = exec->argumentCount() > 1 ? exec->argumentCount() - 1 : 0;
JSArray* boundArgs = JSArray::create(exec->globalData(), globalObject->arrayStructure(), numBoundArgs, CreateCompact);
JSArray* boundArgs = JSArray::tryCreateUninitialized(exec->globalData(), globalObject->arrayStructure(), numBoundArgs);
if (!boundArgs)
return JSValue::encode(throwOutOfMemoryError(exec));
for (size_t i = 0; i < numBoundArgs; ++i)
boundArgs->uncheckedSetIndex(exec->globalData(), i, exec->argument(i + 1));
boundArgs->setLength(numBoundArgs);
boundArgs->initializeIndex(exec->globalData(), i, exec->argument(i + 1));
boundArgs->completeInitialization(numBoundArgs);
// If the [[Class]] internal property of Target is "Function", then ...
// Else set the length own property of F to 0.
......
......@@ -129,65 +129,69 @@ inline void JSArray::checkConsistency(ConsistencyCheckType)
JSArray::JSArray(JSGlobalData& globalData, Structure* structure)
: JSNonFinalObject(globalData, structure)
, m_storage(0)
{
}
void JSArray::finishCreation(JSGlobalData& globalData)
void JSArray::finishCreation(JSGlobalData& globalData, unsigned initialLength)
{
Base::finishCreation(globalData);
ASSERT(inherits(&s_info));
unsigned initialCapacity = 0;
unsigned initialVectorLength = BASE_VECTOR_LEN;
unsigned initialStorageSize = storageSize(initialVectorLength);
m_storage = static_cast<ArrayStorage*>(fastZeroedMalloc(storageSize(initialCapacity)));
m_storage = static_cast<ArrayStorage*>(fastMalloc(initialStorageSize));
m_storage->m_allocBase = m_storage;
m_storage->m_length = initialLength;
m_indexBias = 0;
m_vectorLength = initialCapacity;
m_vectorLength = initialVectorLength;
m_storage->m_sparseValueMap = 0;
m_storage->subclassData = 0;
m_storage->m_numValuesInVector = 0;
#if CHECK_ARRAY_CONSISTENCY
m_storage->m_inCompactInitialization = false;
#endif
checkConsistency();
WriteBarrier<Unknown>* vector = m_storage->m_vector;
for (size_t i = 0; i < initialVectorLength; ++i)
vector[i].clear();
Heap::heap(this)->reportExtraMemoryCost(storageSize(0));
checkConsistency();
Heap::heap(this)->reportExtraMemoryCost(initialStorageSize);
}
void JSArray::finishCreation(JSGlobalData& globalData, unsigned initialLength, ArrayCreationMode creationMode)
JSArray* JSArray::tryFinishCreationUninitialized(JSGlobalData& globalData, unsigned initialLength)
{
Base::finishCreation(globalData);
ASSERT(inherits(&s_info));
unsigned initialCapacity;
if (creationMode == CreateCompact)
initialCapacity = initialLength;
else
initialCapacity = min(BASE_VECTOR_LEN, MIN_SPARSE_ARRAY_INDEX);
m_storage = static_cast<ArrayStorage*>(fastMalloc(storageSize(initialCapacity)));
// Check for lengths larger than we can handle with a vector.
if (initialLength > MAX_STORAGE_VECTOR_LENGTH)
return 0;
unsigned initialVectorLength = max(initialLength, BASE_VECTOR_LEN);
unsigned initialStorageSize = storageSize(initialVectorLength);
m_storage = static_cast<ArrayStorage*>(fastMalloc(initialStorageSize));
m_storage->m_allocBase = m_storage;
m_storage->m_length = initialLength;
m_storage->m_length = 0;
m_indexBias = 0;
m_vectorLength = initialCapacity;
m_vectorLength = initialVectorLength;
m_storage->m_sparseValueMap = 0;
m_storage->subclassData = 0;
if (creationMode == CreateCompact) {
#if CHECK_ARRAY_CONSISTENCY
m_storage->m_inCompactInitialization = !!initialCapacity;
#endif
m_storage->m_length = 0;
m_storage->m_numValuesInVector = initialCapacity;
} else {
m_storage->m_numValuesInVector = initialLength;
#if CHECK_ARRAY_CONSISTENCY
storage->m_inCompactInitialization = false;
m_storage->m_inCompactInitialization = true;
#endif
m_storage->m_length = initialLength;
m_storage->m_numValuesInVector = 0;
WriteBarrier<Unknown>* vector = m_storage->m_vector;
for (size_t i = 0; i < initialCapacity; ++i)
vector[i].clear();
}
checkConsistency();
Heap::heap(this)->reportExtraMemoryCost(storageSize(initialCapacity));
WriteBarrier<Unknown>* vector = m_storage->m_vector;
for (size_t i = initialLength; i < initialVectorLength; ++i)
vector[i].clear();
Heap::heap(this)->reportExtraMemoryCost(initialStorageSize);
return this;
}
void JSArray::finishCreation(JSGlobalData& globalData, const ArgList& list)
......@@ -271,8 +275,12 @@ void JSArray::finishCreation(JSGlobalData& globalData, const JSValue* values, si
JSArray::~JSArray()
{
ASSERT(jsCast<JSArray*>(this));
checkConsistency(DestructorConsistencyCheck);
// If we are unable to allocate memory for m_storage then this may be null.
if (!m_storage)
return;
checkConsistency(DestructorConsistencyCheck);
delete m_storage->m_sparseValueMap;
fastFree(m_storage->m_allocBase);
}
......@@ -712,19 +720,13 @@ bool JSArray::increaseVectorPrefixLength(unsigned newLength)
return true;
}
void JSArray::setLength(unsigned newLength)
{
checkConsistency();
ArrayStorage* storage = m_storage;
#if CHECK_ARRAY_CONSISTENCY
if (!storage->m_inCompactInitialization)
checkConsistency();
else
storage->m_inCompactInitialization = false;
#endif
unsigned length = storage->m_length;
if (newLength < length) {
......@@ -1378,6 +1380,8 @@ void JSArray::checkConsistency(ConsistencyCheckType type)
{
ArrayStorage* storage = m_storage;
ASSERT(!storage->m_inCompactInitialization);
ASSERT(storage);
if (type == SortConsistencyCheck)
ASSERT(!storage->m_sparseValueMap);
......
......@@ -113,24 +113,14 @@ namespace JSC {
WriteBarrier<Unknown> m_vector[1];
};
// The CreateCompact creation mode is used for fast construction of arrays
// whose size and contents are known at time of creation.
//
// There are two obligations when using this mode:
//
// - uncheckedSetIndex() must be used when initializing the array.
// - setLength() must be called after initialization.
enum ArrayCreationMode { CreateCompact, CreateInitialized };
class JSArray : public JSNonFinalObject {
friend class Walker;
protected:
explicit JSArray(JSGlobalData&, Structure*);
void finishCreation(JSGlobalData&);
void finishCreation(JSGlobalData&, unsigned initialLength, ArrayCreationMode);
void finishCreation(JSGlobalData&, unsigned initialLength = 0);
JSArray* tryFinishCreationUninitialized(JSGlobalData&, unsigned initialLength);
void finishCreation(JSGlobalData&, const ArgList&);
void finishCreation(JSGlobalData&, const JSValue*, size_t length);
......@@ -140,18 +130,22 @@ namespace JSC {
~JSArray();
static void destroy(JSCell*);
static JSArray* create(JSGlobalData& globalData, Structure* structure)
static JSArray* create(JSGlobalData& globalData, Structure* structure, unsigned initialLength = 0)
{
JSArray* array = new (NotNull, allocateCell<JSArray>(globalData.heap)) JSArray(globalData, structure);
array->finishCreation(globalData);
array->finishCreation(globalData, initialLength);
return array;
}
static JSArray* create(JSGlobalData& globalData, Structure* structure, unsigned initialLength, ArrayCreationMode createMode)
// tryCreateUninitialized is used for fast construction of arrays whose size and
// contents are known at time of creation. Clients of this interface must:
// - null-check the result (indicating out of memory, or otherwise unable to allocate vector).
// - call 'initializeIndex' for all properties in sequence, for 0 <= i < initialLength.
// - called 'completeInitialization' after all properties have been initialized.
static JSArray* tryCreateUninitialized(JSGlobalData& globalData, Structure* structure, unsigned initialLength)
{
JSArray* array = new (NotNull, allocateCell<JSArray>(globalData.heap)) JSArray(globalData, structure);
array->finishCreation(globalData, initialLength, createMode);
return array;
return array->tryFinishCreationUninitialized(globalData, initialLength);
}
static JSArray* create(JSGlobalData& globalData, Structure* structure, const ArgList& initialValues)
......@@ -210,14 +204,33 @@ namespace JSC {
x.set(globalData, this, v);
}
void uncheckedSetIndex(JSGlobalData& globalData, unsigned i, JSValue v)
inline void initializeIndex(JSGlobalData& globalData, unsigned i, JSValue v)
{
ASSERT(canSetIndex(i));
ArrayStorage *storage = m_storage;
#if CHECK_ARRAY_CONSISTENCY
ASSERT(storage->m_inCompactInitialization);
#endif
storage->m_vector[i].set(globalData, this, v);
// Check that we are initializing the next index in sequence.
ASSERT_UNUSED(i, i == storage->m_length);
// tryCreateUninitialized set m_numValuesInVector to the initialLength,
// check we do not try to initialize more than this number of properties.
ASSERT(storage->m_length < storage->m_numValuesInVector);
// It is improtant that we increment length here, so that all newly added
// values in the array still get marked during the initialization phase.
storage->m_vector[storage->m_length++].set(globalData, this, v);
}
inline void completeInitialization(unsigned newLength)
{
// Check that we have initialized as meny properties as we think we have.
ASSERT_UNUSED(newLength, newLength == m_storage->m_length);
// Check that the number of propreties initialized matches the initialLength.
ASSERT(m_storage->m_length == m_storage->m_numValuesInVector);
#if CHECK_ARRAY_CONSISTENCY
ASSERT(m_storage->m_inCompactInitialization);
m_storage->m_inCompactInitialization = false;
#endif
}
bool inSparseMode()
......
......@@ -444,7 +444,7 @@ namespace JSC {
inline JSArray* constructEmptyArray(ExecState* exec, JSGlobalObject* globalObject, unsigned initialLength)
{
return JSArray::create(exec->globalData(), globalObject->arrayStructure(), initialLength, CreateInitialized);
return JSArray::create(exec->globalData(), globalObject->arrayStructure(), initialLength);
}
inline JSArray* constructEmptyArray(ExecState* exec, unsigned initialLength)
......
......@@ -127,7 +127,7 @@ RegExpMatchesArray::RegExpMatchesArray(ExecState* exec)
void RegExpMatchesArray::finishCreation(JSGlobalData& globalData, RegExpConstructorPrivate* data)
{
Base::finishCreation(globalData, data->lastNumSubPatterns + 1, CreateInitialized);
Base::finishCreation(globalData, data->lastNumSubPatterns + 1);
RegExpConstructorPrivate* d = new RegExpConstructorPrivate;
d->input = data->lastInput;
d->lastInput = data->lastInput;
......
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