Commit 8e937f80 authored by mhahnenberg@apple.com's avatar mhahnenberg@apple.com

JSArray::shiftCountWithArrayStorage doesn't change indexBias when shifting the...

JSArray::shiftCountWithArrayStorage doesn't change indexBias when shifting the last element in m_vector
https://bugs.webkit.org/show_bug.cgi?id=120389

Reviewed by Michael Saboff.

Went through and cleaned up shiftCountWithArrayStorage. Gave meaningful variable names
and commented the confusing parts. This led to realizing how to fix this bug, which has
been done. The issue was that we were modifying the vector length unconditionally, even
when we weren't logically changing the length of the vector. Instead, we should only modify
the vector length when we modify the index bias.

* runtime/JSArray.cpp:
(JSC::JSArray::shiftCountWithArrayStorage):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@155395 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 10a331a3
2013-08-29 Mark Hahnenberg <mhahnenberg@apple.com>
JSArray::shiftCountWithArrayStorage doesn't change indexBias when shifting the last element in m_vector
https://bugs.webkit.org/show_bug.cgi?id=120389
Reviewed by Michael Saboff.
Went through and cleaned up shiftCountWithArrayStorage. Gave meaningful variable names
and commented the confusing parts. This led to realizing how to fix this bug, which has
been done. The issue was that we were modifying the vector length unconditionally, even
when we weren't logically changing the length of the vector. Instead, we should only modify
the vector length when we modify the index bias.
* runtime/JSArray.cpp:
(JSC::JSArray::shiftCountWithArrayStorage):
2013-09-08 Anders Carlsson <andersca@apple.com>
Begin moving off of TypeTraits.h
......
......@@ -55,6 +55,8 @@ namespace {
static const size_t largeHeapSize = 32 * MB; // About 1.5X the average webpage.
static const size_t smallHeapSize = 1 * MB; // Matches the FastMalloc per-thread cache.
#define ENABLE_GC_LOGGING 1
#if ENABLE(GC_LOGGING)
#if COMPILER(CLANG)
#define DEFINE_GC_LOGGING_GLOBAL(type, name, arguments) \
......
......@@ -700,29 +700,50 @@ bool JSArray::shiftCountWithArrayStorage(unsigned startIndex, unsigned count, Ar
unsigned usedVectorLength = min(vectorLength, oldLength);
vectorLength -= count;
storage->setVectorLength(vectorLength);
if (vectorLength) {
if (startIndex < usedVectorLength - (startIndex + count)) {
if (startIndex) {
memmove(
storage->m_vector + count,
storage->m_vector,
sizeof(JSValue) * startIndex);
}
m_butterfly = m_butterfly->shift(structure(), count);
storage = m_butterfly->arrayStorage();
storage->m_indexBias += count;
} else {
unsigned numElementsBeforeShiftRegion = startIndex;
unsigned firstIndexAfterShiftRegion = startIndex + count;
unsigned numElementsAfterShiftRegion = usedVectorLength - firstIndexAfterShiftRegion;
ASSERT(numElementsBeforeShiftRegion + count + numElementsAfterShiftRegion == usedVectorLength);
// The point of this comparison seems to be to minimize the amount of elements that have to
// be moved during a shift operation.
if (numElementsBeforeShiftRegion < numElementsAfterShiftRegion) {
// The number of elements before the shift region is less than the number of elements
// after the shift region, so we move the elements before to the right.
if (numElementsBeforeShiftRegion) {
RELEASE_ASSERT(count + startIndex <= vectorLength);
memmove(
storage->m_vector + startIndex,
storage->m_vector + startIndex + count,
sizeof(JSValue) * (usedVectorLength - (startIndex + count)));
for (unsigned i = usedVectorLength - count; i < usedVectorLength; ++i)
storage->m_vector[i].clear();
storage->m_vector + count,
storage->m_vector,
sizeof(JSValue) * startIndex);
}
// Adjust the Butterfly and the index bias. We only need to do this here because we're changing
// the start of the Butterfly, which needs to point at the first indexed property in the used
// portion of the vector.
m_butterfly = m_butterfly->shift(structure(), count);
storage = m_butterfly->arrayStorage();
storage->m_indexBias += count;
// Since we're consuming part of the vector by moving its beginning to the left,
// we need to modify the vector length appropriately.
storage->setVectorLength(vectorLength - count);
} else {
// The number of elements before the shift region is greater than or equal to the number
// of elements after the shift region, so we move the elements after the shift region to the left.
memmove(
storage->m_vector + startIndex,
storage->m_vector + firstIndexAfterShiftRegion,
sizeof(JSValue) * numElementsAfterShiftRegion);
// Clear the slots of the elements we just moved.
unsigned startOfEmptyVectorTail = usedVectorLength - count;
for (unsigned i = startOfEmptyVectorTail; i < usedVectorLength; ++i)
storage->m_vector[i].clear();
// We don't modify the index bias or the Butterfly pointer in this case because we're not changing
// the start of the Butterfly, which needs to point at the first indexed property in the used
// portion of the vector. We also don't modify the vector length because we're not actually changing
// its length; we're just using less of it.
}
return true;
}
......
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