Bug 15606 – make cut-off for sparse vs. dense arrays smarter for speed with large arrays

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Darin Adler]

I got 3% speedup on the SunSpider benchmark by doing as Maciej suggested and making arrays smarter about sparse vs. dense.

[Darin Adler]

Created attachment 16784 [details]
patch with change log

[Mark Rowe (bdash)]

Be sure not to introduce bug 15603 with this change :)

[Maciej Stachowiak]

Comment on attachment 16784 [details]
patch with change log

Regarding this FIXME. The extra copy is necessary because mergesort modifies the buffer that you pass it as it works, and there are times when not all values are in the buffer, some are only in auxiliary storage. But the compare function can trigger a garbage collection. If a GC happens while the array's storage pointer points to a buffer that does not contain all the values, then some objects that should be live will be trashed. This led to real crashes before I added the copy, so it's not just a problem in theory. I would suggest replacing the FIXME with a comment that explains this. My apologies for not adding such a comment at the time.

+        // FIXME: Why do we need this copy of the array storage?
+        // I know that mergesort allocates more memory internally,
+        // but I don't see any use of the original m_storage here!
+        size_t size = storageSize(m_vectorLength);
+        ArrayStorage* copy = static_cast<ArrayStorage*>(fastMalloc(size));
+        memcpy(copy, m_storage, size);
+        mergesort(copy->m_vector, lengthNotIncludingUndefined, sizeof(JSValue*), compareWithCompareFunctionForQSort);
+        fastFree(m_storage);
+        m_storage = copy;

Everything else looks good. r=me

[Darin Adler]

r26881