Commit 83c15267 authored by benjamin@webkit.org's avatar benjamin@webkit.org

Do not abuse ArrayStorage's m_length for testing array consistency

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

Patch by Benjamin Poulain <bpoulain@apple.com> on 2012-04-06
Reviewed by Geoffrey Garen.

Array creation from a list of values is a 3 steps process:
-JSArray::tryCreateUninitialized()
-JSArray::initializeIndex() for each values
-JSArray::completeInitialization()

Previously, the attribute m_length was not set to the final size
JSArray::tryCreateUninitialized() because it was used to test the array
consistency JSArray::initializeIndex().

This caused the initialization loop using JSArray::initializeIndex() maintain
two counters:
-index of the loop
-storage->m_length++

This patch fixes this by using the index of the initialization loop for the indinces of
JSArray::initializeIndex(). For testing consistency, the variable m_initializationIndex
is introduced if CHECK_ARRAY_CONSISTENCY is defined.

The patch also fixes minor unrelated build issue when CHECK_ARRAY_CONSISTENCY is defined.

This improves the performance of JSArray creation from literals by 8%.

* runtime/JSArray.cpp:
(JSC::JSArray::tryFinishCreationUninitialized):
(JSC::JSArray::checkConsistency):
* runtime/JSArray.h:
(ArrayStorage):
(JSC::JSArray::initializeIndex):
(JSC::JSArray::completeInitialization):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@113530 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 73b36797
2012-04-06 Benjamin Poulain <bpoulain@apple.com>
Do not abuse ArrayStorage's m_length for testing array consistency
https://bugs.webkit.org/show_bug.cgi?id=83403
Reviewed by Geoffrey Garen.
Array creation from a list of values is a 3 steps process:
-JSArray::tryCreateUninitialized()
-JSArray::initializeIndex() for each values
-JSArray::completeInitialization()
Previously, the attribute m_length was not set to the final size
JSArray::tryCreateUninitialized() because it was used to test the array
consistency JSArray::initializeIndex().
This caused the initialization loop using JSArray::initializeIndex() maintain
two counters:
-index of the loop
-storage->m_length++
This patch fixes this by using the index of the initialization loop for the indinces of
JSArray::initializeIndex(). For testing consistency, the variable m_initializationIndex
is introduced if CHECK_ARRAY_CONSISTENCY is defined.
The patch also fixes minor unrelated build issue when CHECK_ARRAY_CONSISTENCY is defined.
This improves the performance of JSArray creation from literals by 8%.
* runtime/JSArray.cpp:
(JSC::JSArray::tryFinishCreationUninitialized):
(JSC::JSArray::checkConsistency):
* runtime/JSArray.h:
(ArrayStorage):
(JSC::JSArray::initializeIndex):
(JSC::JSArray::completeInitialization):
2012-04-06 Jon Lee <jonlee@apple.com>
Build fix for Windows bots.
......@@ -176,11 +176,12 @@ JSArray* JSArray::tryFinishCreationUninitialized(JSGlobalData& globalData, unsig
m_storage = static_cast<ArrayStorage*>(newStorage);
m_storage->m_allocBase = m_storage;
m_storage->m_length = 0;
m_storage->m_length = initialLength;
m_vectorLength = initialVectorLength;
m_storage->m_numValuesInVector = initialLength;
#if CHECK_ARRAY_CONSISTENCY
m_storage->m_initializationIndex = 0;
m_storage->m_inCompactInitialization = true;
#endif
......@@ -1833,7 +1834,7 @@ void JSArray::checkConsistency(ConsistencyCheckType type)
unsigned numValuesInVector = 0;
for (unsigned i = 0; i < m_vectorLength; ++i) {
if (JSValue value = storage->m_vector[i]) {
if (JSValue value = storage->m_vector[i].get()) {
ASSERT(i < storage->m_length);
if (type != DestructorConsistencyCheck)
value.isUndefined(); // Likely to crash if the object was deallocated.
......@@ -1847,15 +1848,15 @@ void JSArray::checkConsistency(ConsistencyCheckType type)
ASSERT(numValuesInVector <= storage->m_length);
if (m_sparseValueMap) {
SparseArrayValueMap::iterator end = m_sparseValueMap->end();
for (SparseArrayValueMap::iterator it = m_sparseValueMap->begin(); it != end; ++it) {
SparseArrayValueMap::const_iterator end = m_sparseValueMap->end();
for (SparseArrayValueMap::const_iterator it = m_sparseValueMap->begin(); it != end; ++it) {
unsigned index = it->first;
ASSERT(index < storage->m_length);
ASSERT(index >= storage->m_vectorLength);
ASSERT(index >= m_vectorLength);
ASSERT(index <= MAX_ARRAY_INDEX);
ASSERT(it->second);
if (type != DestructorConsistencyCheck)
it->second.isUndefined(); // Likely to crash if the object was deallocated.
it->second.getNonSparseMode().isUndefined(); // Likely to crash if the object was deallocated.
}
}
}
......
......@@ -119,7 +119,9 @@ namespace JSC {
unsigned m_numValuesInVector;
void* m_allocBase; // Pointer to base address returned by malloc(). Keeping this pointer does eliminate false positives from the leak detector.
#if CHECK_ARRAY_CONSISTENCY
uintptr_t m_inCompactInitialization; // Needs to be a uintptr_t for alignment purposes.
// Needs to be a uintptr_t for alignment purposes.
uintptr_t m_initializationIndex;
uintptr_t m_inCompactInitialization;
#else
uintptr_t m_padding;
#endif
......@@ -219,24 +221,25 @@ namespace JSC {
ArrayStorage *storage = m_storage;
#if CHECK_ARRAY_CONSISTENCY
ASSERT(storage->m_inCompactInitialization);
#endif
// Check that we are initializing the next index in sequence.
ASSERT_UNUSED(i, i == storage->m_length);
ASSERT(i == storage->m_initializationIndex);
// 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);
ASSERT(storage->m_initializationIndex < storage->m_numValuesInVector);
storage->m_initializationIndex++;
#endif
ASSERT(i < storage->m_length);
ASSERT(i < storage->m_numValuesInVector);
storage->m_vector[i].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
// Check that the number of propreties initialized matches the initialLength.
ASSERT(m_storage->m_initializationIndex == m_storage->m_numValuesInVector);
ASSERT(m_storage->m_inCompactInitialization);
m_storage->m_inCompactInitialization = false;
#endif
......
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