Bug 403977 - Huge Speed Drop in Array.prototype.sort, patch by Igor Bukanov <igor@mir2.org>, r=crowder, approval1.9 by beltzner

This commit is contained in:
crowder@fiverocks.com 2007-12-13 11:55:21 -08:00
parent 65fc1513f7
commit 0fde2c75ba

View File

@ -846,12 +846,6 @@ typedef struct MSortArgs {
JSBool fastcopy;
} MSortArgs;
static JSBool
sort_compare(void *arg, const void *a, const void *b, int *result);
static int
sort_compare_strings(void *arg, const void *a, const void *b, int *result);
/* Helper function for js_MergeSort. */
static JSBool
MergeArrays(MSortArgs *msa, void *src, void *dest, size_t run1, size_t run2)
@ -924,7 +918,9 @@ js_MergeSort(void *src, size_t nel, size_t elsize,
JSBool fastcopy;
int cmp_result;
fastcopy = (cmp == sort_compare || cmp == sort_compare_strings);
/* Avoid memcpy overhead for word-sized and word-aligned elements. */
fastcopy = (elsize == sizeof(jsval) &&
(((jsuword) src | (jsuword) tmp) & JSVAL_ALIGN) == 0);
#define COPY_ONE(p,q,n) \
(fastcopy ? (void)(*(jsval*)(p) = *(jsval*)(q)) : (void)memcpy(p, q, n))
#define CALL_CMP(a, b) \
@ -995,7 +991,6 @@ js_MergeSort(void *src, size_t nel, size_t elsize,
typedef struct CompareArgs {
JSContext *context;
jsval fval;
jsval *localroot; /* need one local root */
jsval *elemroot; /* stack needed for js_Invoke */
} CompareArgs;
@ -1044,90 +1039,13 @@ sort_compare(void *arg, const void *a, const void *b, int *result)
return JS_TRUE;
}
static int
sort_compare_as_strings(void *arg, const void *a, const void *b, int *result)
{
CompareArgs *ca = (CompareArgs *) arg;
JSContext *cx = ca->context;
struct {
jsval val;
JSString *str;
char *chars;
char buf[DTOSTR_STANDARD_BUFFER_SIZE];
} conv[2], *c;
jsval v;
size_t i, n1, n2, n;
jschar *jschars;
char *chars;
if (!JS_CHECK_OPERATION_LIMIT(cx, JSOW_JUMP))
return JS_FALSE;
conv[0].val = *(const jsval *)a;
conv[1].val = *(const jsval *)b;
if (conv[0].val == conv[1].val) {
*result = 0;
return JS_TRUE;
}
for (c = conv; c != JS_ARRAY_END(conv); ++c) {
v = c->val;
c->str = NULL;
c->chars = NULL;
if (JSVAL_IS_STRING(v)) {
c->str = JSVAL_TO_STRING(v);
} else if (JSVAL_IS_INT(v)) {
c->chars = js_IntToCString(JSVAL_TO_INT(v), c->buf,
JS_ARRAY_LENGTH(c->buf));
} else if (JSVAL_IS_DOUBLE(v)) {
c->chars = js_NumberToCString(cx, *JSVAL_TO_DOUBLE(v),
c->buf, sizeof c->buf);
if (!c->chars)
return JS_FALSE;
} else {
c->str = js_ValueToString(cx, v);
if (!c->str)
return JS_FALSE;
if (c == conv) {
/* Root ToString result, as the second iteration can GC. */
*ca->localroot = STRING_TO_JSVAL(c->str);
}
}
}
if (conv[0].str && conv[1].str) {
*result = (int) js_CompareStrings(conv[0].str, conv[1].str);
} else if (conv[0].chars && conv[1].chars) {
*result = strcmp(conv[0].chars, conv[1].chars);
} else {
i = conv[0].str ? 0 : 1;
jschars = JSSTRING_CHARS(conv[i].str);
n1 = JSSTRING_LENGTH(conv[i].str);
chars = conv[1 - i].chars;
n2 = strlen(chars);
n = JS_MIN(n1, n2);
for (;;) {
if (n == 0) {
*result = (n1 < n2) ? -1 : (n1 > n2) ? 1 : 0;
break;
}
--n;
*result = (int)*jschars++ - (int)*chars++;
if (*result != 0)
break;
}
if (conv[1].str)
*result = -*result;
}
return JS_TRUE;
}
static int
sort_compare_strings(void *arg, const void *a, const void *b, int *result)
{
jsval av = *(const jsval *)a, bv = *(const jsval *)b;
JS_ASSERT(JSVAL_IS_STRING(av));
JS_ASSERT(JSVAL_IS_STRING(bv));
if (!JS_CHECK_OPERATION_LIMIT((JSContext *)arg, JSOW_JUMP))
return JS_FALSE;
@ -1145,12 +1063,14 @@ JS_STATIC_ASSERT(JSVAL_NULL == 0);
static JSBool
array_sort(JSContext *cx, uintN argc, jsval *vp)
{
jsval *argv, fval, *vec, *mergesort_tmp;
jsval *argv, fval, *vec, *mergesort_tmp, v;
JSObject *obj;
CompareArgs ca;
jsuint len, newlen, i, undefs;
JSTempValueRooter tvr;
JSBool hole, ok;
size_t elemsize;
JSString *str;
/*
* Optimize the default compare function case if all of obj's elements
@ -1165,11 +1085,9 @@ array_sort(JSContext *cx, uintN argc, jsval *vp)
JSMSG_BAD_SORT_ARG);
return JS_FALSE;
}
fval = argv[0];
all_strings = JS_FALSE; /* non-default compare function */
fval = argv[0]; /* non-default compare function */
} else {
fval = JSVAL_NULL;
all_strings = JS_TRUE; /* check for all string values */
}
obj = JS_THIS_OBJECT(cx, vp);
@ -1181,30 +1099,16 @@ array_sort(JSContext *cx, uintN argc, jsval *vp)
}
/*
* We need a temporary array of 2 * len + 1 jsvals to hold the array
* elements and a temporary root. Check that its size does not overflow
* size_t, which would allow for indexing beyond the end of the malloc'd
* vector.
*
* The overflow occurs when, mathematically,
* (2 * len + 1) * sizeof(jsval) > MAX_SIZE_T .
* But as a C expression this is always false, due to exactly the overflow
* condition we're trying to detect and avoid. So we rearrange:
* (2 * len + 1) * sizeof(jsval) > MAX_SIZE_T
* 2 * len + 1 > MAX_SIZE_T / sizeof(jsval)
* 2 * len > MAX_SIZE_T / sizeof(jsval) - 1
* len > (MAX_SIZE_T / sizeof(jsval) - 1) / 2
* We need a temporary array of 2 * len jsvals to hold the array elements
* and the scratch space for merge sort. Check that its size does not
* overflow size_t, which would allow for indexing beyond the end of the
* malloc'd vector.
*/
if (len > ((size_t)-1 / sizeof(jsval) - 1) / 2) {
if (len > (size_t) -1 / (2 * sizeof(jsval))) {
JS_ReportOutOfMemory(cx);
return JS_FALSE;
}
/*
* Allocate 2 * len instead of len, to reserve space for the mergesort
* algorithm; plus 1 for an additional temporary root.
*/
vec = (jsval *) JS_malloc(cx, (2 * (size_t)len + 1) * sizeof(jsval));
vec = (jsval *) JS_malloc(cx, 2 * (size_t) len * sizeof(jsval));
if (!vec)
return JS_FALSE;
@ -1231,6 +1135,7 @@ array_sort(JSContext *cx, uintN argc, jsval *vp)
*/
undefs = 0;
newlen = 0;
all_strings = JS_TRUE;
for (i = 0; i < len; i++) {
ok = JS_CHECK_OPERATION_LIMIT(cx, JSOW_JUMP);
if (!ok)
@ -1257,49 +1162,127 @@ array_sort(JSContext *cx, uintN argc, jsval *vp)
++newlen;
}
if (newlen == 0) {
/* The array has only holes and undefs. */
ok = JS_TRUE;
goto out;
}
/*
* The first newlen elements of vec are copied from the array
* object (above).
*
* Of the remaining 2*len-newlen positions, newlen are used as GC rooted
* temp space for mergesort and the last (2*len-2*newlen+1) positions are
* unused (except when all_strings is false, in which case one more
* element is used for ca.localroot below).
*
* Here we clear the tmp-values before GC-rooting the array.
* We assume JSVAL_NULL==0 to optimize initialization using memset.
* The first newlen elements of vec are copied from the array object
* (above). The remaining newlen positions are used as GC-rooted scratch
* space for mergesort. We must clear the space before including it to
* the root set covered by tvr.count. We assume JSVAL_NULL==0 to optimize
* initialization using memset.
*/
mergesort_tmp = vec + newlen;
memset(mergesort_tmp, 0, newlen * sizeof(jsval));
tvr.count = newlen * 2;
/* Here len == 2 * (newlen + undefs + number_of_holes). */
if (all_strings) {
ok = js_MergeSort(vec, (size_t) newlen, sizeof(jsval),
if (fval == JSVAL_NULL) {
/*
* Sort using the default comparator converting all elements to
* strings.
*/
if (all_strings) {
elemsize = sizeof(jsval);
} else {
/*
* To avoid string conversion on each compare we do it only once
* prior to sorting. But we also need the space for the original
* values to recover the sorting result. To reuse
* sort_compare_strings we move the original values to the odd
* indexes in vec, put the string conversion results in the even
* indexes and pass 2 * sizeof(jsval) as an element size to the
* sorting function. In this way sort_compare_strings will only
* see the string values when it casts the compare arguments as
* pointers to jsval.
*
* This requires doubling the temporary storage including the
* scratch space for the merge sort. Since vec already contains
* the rooted scratch space for newlen elements at the tail, we
* can use it to rearrange and convert to strings first and try
* realloc only when we know that we successfully converted all
* the elements.
*/
if (newlen > (size_t) -1 / (4 * sizeof(jsval))) {
JS_ReportOutOfMemory(cx);
ok = JS_FALSE;
goto out;
}
/*
* Rearrange and string-convert the elements of the vector from
* the tail here and, after sorting, move the results back
* starting from the start to prevent overwrite the existing
* elements.
*/
i = newlen;
do {
--i;
ok = JS_CHECK_OPERATION_LIMIT(cx, JSOW_JUMP);
if (!ok)
goto out;
v = vec[i];
str = js_ValueToString(cx, v);
if (!str) {
ok = JS_FALSE;
goto out;
}
vec[2 * i] = STRING_TO_JSVAL(str);
vec[2 * i + 1] = v;
} while (i != 0);
JS_ASSERT(tvr.u.array == vec);
vec = JS_realloc(cx, vec, 4 * (size_t) newlen * sizeof(jsval));
if (!vec) {
vec = tvr.u.array;
ok = JS_FALSE;
goto out;
}
tvr.u.array = vec;
mergesort_tmp = vec + 2 * newlen;
memset(mergesort_tmp, 0, newlen * 2 * sizeof(jsval));
tvr.count = newlen * 4;
elemsize = 2 * sizeof(jsval);
}
ok = js_MergeSort(vec, (size_t) newlen, elemsize,
sort_compare_strings, cx, mergesort_tmp);
if (!ok)
goto out;
if (!all_strings) {
/*
* Do not call the branch callback in the following moving loop
* to make it fast and unroot the cached results of toString
* invocations before the callback has a chance to run the GC.
*/
for (i = 0; i < newlen; i++)
vec[i] = vec[2 * i + 1];
}
} else {
void *mark;
ca.context = cx;
ca.fval = fval;
/* local GC root for temporary string */
ca.localroot = &vec[tvr.count++];
*ca.localroot = JSVAL_NULL;
ca.elemroot = js_AllocStack(cx, 2 + 2, &mark);
if (!ca.elemroot) {
ok = JS_FALSE;
goto out;
}
ok = js_MergeSort(vec, (size_t) newlen, sizeof(jsval),
(fval != JSVAL_NULL)
? sort_compare
: sort_compare_as_strings,
&ca, mergesort_tmp);
sort_compare, &ca, mergesort_tmp);
js_FreeStack(cx, mark);
if (!ok)
goto out;
}
if (!ok)
goto out;
/*
* We no longer need to root the scratch space for the merge sort, so
* unroot it now to make the job of a potential GC under InitArrayElements
* easier.
*/
tvr.count = newlen;
ok = InitArrayElements(cx, obj, 0, newlen, vec);
if (!ok)
goto out;