From 337838393500410870ebaf00296ea4d0c67154ce Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Thu, 22 Jul 2010 18:45:21 -0700 Subject: [PATCH 01/19] Remove hole count from dense arrays (580846, r=njn). --- js/src/jsarray.cpp | 250 ++++++++++++++---------------------------- js/src/jsarray.h | 2 +- js/src/jscntxt.h | 1 + js/src/jsgc.cpp | 10 ++ js/src/jsinterp.cpp | 8 +- js/src/jsiter.cpp | 2 +- js/src/jsobj.cpp | 4 +- js/src/jsobj.h | 17 +-- js/src/jsobjinlines.h | 85 +------------- js/src/jstracer.cpp | 43 ++++---- 10 files changed, 128 insertions(+), 294 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 3ba480de6ff4..c30c2c629cad 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -48,20 +48,12 @@ * * We track these pieces of metadata for arrays in dense mode: * - The array's length property as a uint32, accessible with - * getArrayLength(), setDenseArrayLength(). - * - The number of indices that are filled (non-holes), accessible with - * {get,set}DenseArrayCount(). + * getArrayLength(), setArrayLength(). * - The number of element slots (capacity), gettable with * getDenseArrayCapacity(). - * - The minimum of length and capacity (minLenCap). There are no explicit - * setters, it's updated automatically by setDenseArrayLength() and - * setDenseArrayCapacity(). There are also no explicit getters, the only - * user is TraceRecorder which can access it directly because it's a - * friend. The function isDenseArrayMinLenCapOk() checks that it is set - * correctly; a call to it should be put in an assertion at use points. * * In dense mode, holes in the array are represented by (JS_ARRAY_HOLE) invalid - * values. The final slot in fslots is unused. + * values. The final two slot in fslots are unused. * * NB: the capacity and length of a dense array are entirely unrelated! The * length may be greater than, less than, or equal to the capacity. See @@ -131,13 +123,26 @@ INDEX_TOO_BIG(jsuint index) return index > JS_BIT(29) - 1; } -#define INDEX_TOO_SPARSE(array, index) \ - (INDEX_TOO_BIG(index) || \ - ((index) > array->getDenseArrayCapacity() && (index) >= MIN_SPARSE_INDEX && \ - (index) > ((array)->getDenseArrayCount() + 1) * 4)) +static inline bool +INDEX_TOO_SPARSE(JSObject *array, jsuint index) +{ + /* Small arrays with less than 256 elements are dense, no matter what. */ + if (index < 256) + return false; -#define ENSURE_SLOW_ARRAY(cx, obj) \ - (obj->getClass() == &js_SlowArrayClass || obj->makeDenseArraySlow(cx)) + /* + * Otherwise if the index becomes too large or is more than 256 past + * the current capacity, we have to slowify. + */ + return INDEX_TOO_BIG(index) || (index > array->getDenseArrayCapacity() + 256); +} + +static inline bool +ENSURE_SLOW_ARRAY(JSContext *cx, JSObject *obj) +{ + return obj->getClass() == &js_SlowArrayClass || + obj->makeDenseArraySlow(cx); +} /* * Determine if the id represents an array index or an XML property index. @@ -478,9 +483,7 @@ SetArrayElement(JSContext *cx, JSObject *obj, jsdouble index, const Value &v) if (!obj->ensureDenseArrayElements(cx, idx + 1)) return JS_FALSE; if (idx >= obj->getArrayLength()) - obj->setDenseArrayLength(idx + 1); - if (obj->getDenseArrayElement(idx).isMagic(JS_ARRAY_HOLE)) - obj->incDenseArrayCountBy(1); + obj->setArrayLength(idx + 1); obj->setDenseArrayElement(idx, v); return JS_TRUE; } @@ -507,9 +510,7 @@ DeleteArrayElement(JSContext *cx, JSObject *obj, jsdouble index) if (obj->isDenseArray()) { if (index <= jsuint(-1)) { jsuint idx = jsuint(index); - if (!INDEX_TOO_SPARSE(obj, idx) && idx < obj->getDenseArrayCapacity()) { - if (!obj->getDenseArrayElement(idx).isMagic(JS_ARRAY_HOLE)) - obj->decDenseArrayCountBy(1); + if (idx < obj->getDenseArrayCapacity()) { obj->setDenseArrayElement(idx, MagicValue(JS_ARRAY_HOLE)); return JS_TRUE; } @@ -623,26 +624,28 @@ array_length_setter(JSContext *cx, JSObject *obj, jsid id, Value *vp) vp->setNumber(newlen); if (oldlen < newlen) { - if (obj->isDenseArray()) - obj->setDenseArrayLength(newlen); - else - obj->setSlowArrayLength(newlen); + obj->setArrayLength(newlen); return true; } if (obj->isDenseArray()) { - /* Don't reallocate if we're not actually shrinking our slots. */ + /* + * Don't reallocate if we're not actually shrinking our slots. If we do + * shrink slots here, resizeDenseArrayElements will fill all slots to the + * right of newlen with JS_ARRAY_HOLE. This permits us to disregard + * length when reading from arrays as long we are within the capacity. + */ jsuint capacity = obj->getDenseArrayCapacity(); if (capacity > newlen && !obj->resizeDenseArrayElements(cx, capacity, newlen)) return false; - obj->setDenseArrayLength(newlen); + obj->setArrayLength(newlen); } else if (oldlen - newlen < (1 << 24)) { do { --oldlen; if (!JS_CHECK_OPERATION_LIMIT(cx) || !DeleteArrayElement(cx, obj, oldlen)) return false; } while (oldlen != newlen); - obj->setSlowArrayLength(newlen); + obj->setArrayLength(newlen); } else { /* * We are going to remove a lot of indexes in a presumably sparse @@ -669,7 +672,7 @@ array_length_setter(JSContext *cx, JSObject *obj, jsid id, Value *vp) return false; } } - obj->setSlowArrayLength(newlen); + obj->setArrayLength(newlen); } return true; @@ -786,7 +789,7 @@ slowarray_addProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) return JS_TRUE; length = obj->getArrayLength(); if (index >= length) - obj->setSlowArrayLength(index + 1); + obj->setArrayLength(index + 1); return JS_TRUE; } @@ -817,9 +820,7 @@ array_setProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) return JS_FALSE; if (i >= obj->getArrayLength()) - obj->setDenseArrayLength(i + 1); - if (obj->getDenseArrayElement(i).isMagic(JS_ARRAY_HOLE)) - obj->incDenseArrayCountBy(1); + obj->setArrayLength(i + 1); obj->setDenseArrayElement(i, *vp); return JS_TRUE; } @@ -879,8 +880,7 @@ dense_grow(JSContext* cx, JSObject* obj, jsint i, const Value &v) return JS_FALSE; if (u >= obj->getArrayLength()) - obj->setDenseArrayLength(u + 1); - obj->incDenseArrayCountBy(1); + obj->setArrayLength(u + 1); } obj->setDenseArrayElement(u, v); @@ -962,11 +962,8 @@ array_deleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval) return JS_TRUE; } - if (js_IdIsIndex(id, &i) && i < obj->getDenseArrayCapacity() && - !obj->getDenseArrayElement(i).isMagic(JS_ARRAY_HOLE)) { - obj->decDenseArrayCountBy(1); + if (js_IdIsIndex(id, &i) && i < obj->getDenseArrayCapacity()) obj->setDenseArrayElement(i, MagicValue(JS_ARRAY_HOLE)); - } if (!js_SuppressDeletedProperty(cx, obj, id)) return false; @@ -987,9 +984,23 @@ array_trace(JSTracer *trc, JSObject *obj) JS_ASSERT(obj->isDenseArray()); obj->traceProtoAndParent(trc); + size_t holes = 0; + uint32 capacity = obj->getDenseArrayCapacity(); - for (uint32 i = 0; i < capacity; i++) - MarkValue(trc, obj->getDenseArrayElement(i), "dense_array_elems"); + for (uint32 i = 0; i < capacity; i++) { + Value v = obj->getDenseArrayElement(i); + if (v.isMagic(JS_ARRAY_HOLE)) + ++holes; + else + MarkValue(trc, obj->getDenseArrayElement(i), "dense_array_elems"); + } + + if (trc == trc->context->runtime->gcMarkingTracer && + holes > MIN_SPARSE_INDEX && + holes > capacity / 4 * 3) { + /* This might fail, in which case we don't slowify it. */ + reinterpret_cast(trc)->arraysToSlowify.append(obj); + } } extern JSObjectOps js_ArrayObjectOps; @@ -1381,19 +1392,8 @@ array_toLocaleString(JSContext *cx, uintN argc, Value *vp) return array_toString_sub(cx, obj, JS_TRUE, NULL, vp); } -enum TargetElementsType { - TargetElementsAllHoles, - TargetElementsMayContainValues -}; - -enum SourceVectorType { - SourceVectorAllValues, - SourceVectorMayContainHoles -}; - static JSBool -InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint count, Value *vector, - TargetElementsType targetType, SourceVectorType vectorType) +InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint count, Value *vector) { JS_ASSERT(count < MAXINDEX); @@ -1404,54 +1404,16 @@ InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint count, Valu if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) && start <= MAXINDEX - count && !INDEX_TOO_BIG(start + count)) { -#ifdef DEBUG_jwalden - { - /* Verify that overwriteType and writeType were accurate. */ - AutoIdRooter idr(cx); - for (jsuint i = 0; i < count; i++) { - JS_ASSERT_IF(vectorType == SourceVectorAllValues, !vector[i].isMagic(JS_ARRAY_HOLE)); - - jsdouble index = jsdouble(start) + i; - if (targetType == TargetElementsAllHoles && index < jsuint(-1)) { - JS_ASSERT(ReallyBigIndexToId(cx, index, idr.addr())); - JSObject* obj2; - JSProperty* prop; - JS_ASSERT(obj->lookupProperty(cx, idr.id(), &obj2, &prop)); - JS_ASSERT(!prop); - } - } - } -#endif - jsuint newlen = start + count; JS_ASSERT(jsdouble(start) + count == jsdouble(newlen)); if (!obj->ensureDenseArrayElements(cx, newlen)) return JS_FALSE; if (newlen > obj->getArrayLength()) - obj->setDenseArrayLength(newlen); + obj->setArrayLength(newlen); JS_ASSERT(count < uint32(-1) / sizeof(Value)); - if (targetType == TargetElementsMayContainValues) { - jsuint valueCount = 0; - for (jsuint i = 0; i < count; i++) { - if (!obj->getDenseArrayElement(start + i).isMagic(JS_ARRAY_HOLE)) - valueCount++; - } - JS_ASSERT(obj->getDenseArrayCount() >= valueCount); - obj->decDenseArrayCountBy(valueCount); - } memcpy(obj->getDenseArrayElements() + start, vector, sizeof(jsval) * count); - if (vectorType == SourceVectorAllValues) { - obj->incDenseArrayCountBy(count); - } else { - jsuint valueCount = 0; - for (jsuint i = 0; i < count; i++) { - if (!obj->getDenseArrayElement(start + i).isMagic(JS_ARRAY_HOLE)) - valueCount++; - } - obj->incDenseArrayCountBy(valueCount); - } JS_ASSERT_IF(count != 0, !obj->getDenseArrayElement(newlen - 1).isMagic(JS_ARRAY_HOLE)); return JS_TRUE; } @@ -1488,34 +1450,21 @@ InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint count, Valu } static JSBool -InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector, - bool holey = false) +InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector) { JS_ASSERT(obj->isArray()); if (vector) { JS_ASSERT(obj->isDenseArray()); - obj->setDenseArrayLength(length); + obj->setArrayLength(length); if (!obj->ensureDenseArrayElements(cx, length)) return JS_FALSE; - - jsuint count = length; - if (!holey) { - memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value)); - } else { - for (jsuint i = 0; i < length; i++) { - if (vector[i].isMagic(JS_ARRAY_HOLE)) - --count; - obj->setDenseArrayElement(i, vector[i]); - } - } - obj->setDenseArrayCount(count); + memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value)); } else { if (obj->isDenseArray()) { - obj->setDenseArrayLength(length); - obj->setDenseArrayCount(0); + obj->setArrayLength(length); } else { - obj->setSlowArrayLength(length); + obj->setArrayLength(length); } } return JS_TRUE; @@ -2043,10 +1992,8 @@ array_sort(JSContext *cx, uintN argc, Value *vp) * InitArrayElements easier. */ tvr.changeLength(newlen); - if (!InitArrayElements(cx, obj, 0, newlen, vec, TargetElementsMayContainValues, - SourceVectorAllValues)) { + if (!InitArrayElements(cx, obj, 0, newlen, vec)) return false; - } } /* Set undefs that sorted after the rest of elements. */ @@ -2077,10 +2024,8 @@ array_push_slowly(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value * if (!js_GetLengthProperty(cx, obj, &length)) return JS_FALSE; - if (!InitArrayElements(cx, obj, length, argc, argv, TargetElementsMayContainValues, - SourceVectorAllValues)) { + if (!InitArrayElements(cx, obj, length, argc, argv)) return JS_FALSE; - } /* Per ECMA-262, return the new array length. */ jsdouble newlength = length + jsdouble(argc); @@ -2101,10 +2046,9 @@ array_push1_dense(JSContext* cx, JSObject* obj, const Value &v, Value *rval) if (!obj->ensureDenseArrayElements(cx, length + 1)) return JS_FALSE; - obj->setDenseArrayLength(length + 1); + obj->setArrayLength(length + 1); JS_ASSERT(obj->getDenseArrayElement(length).isMagic(JS_ARRAY_HOLE)); - obj->incDenseArrayCountBy(1); obj->setDenseArrayElement(length, v); rval->setNumber(obj->getArrayLength()); return JS_TRUE; @@ -2127,8 +2071,7 @@ ArrayCompPushImpl(JSContext *cx, JSObject *obj, const Value &v) if (!obj->ensureDenseArrayElements(cx, length + 1)) return JS_FALSE; } - obj->setDenseArrayLength(length + 1); - obj->incDenseArrayCountBy(1); + obj->setArrayLength(length + 1); obj->setDenseArrayElement(length, v); return JS_TRUE; } @@ -2198,7 +2141,7 @@ array_pop_dense(JSContext *cx, JSObject* obj, Value *vp) return JS_FALSE; if (!hole && !DeleteArrayElement(cx, obj, index)) return JS_FALSE; - obj->setDenseArrayLength(index); + obj->setArrayLength(index); return JS_TRUE; } @@ -2233,12 +2176,10 @@ array_shift(JSContext *cx, uintN argc, Value *vp) *vp = obj->getDenseArrayElement(0); if (vp->isMagic(JS_ARRAY_HOLE)) vp->setUndefined(); - else - obj->decDenseArrayCountBy(1); Value *elems = obj->getDenseArrayElements(); memmove(elems, elems + 1, length * sizeof(jsval)); obj->setDenseArrayElement(length, MagicValue(JS_ARRAY_HOLE)); - obj->setDenseArrayLength(length); + obj->setArrayLength(length); return JS_TRUE; } @@ -2304,7 +2245,7 @@ array_unshift(JSContext *cx, uintN argc, Value *vp) } /* Copy from argv to the bottom of the array. */ - if (!InitArrayElements(cx, obj, 0, argc, argv, TargetElementsAllHoles, SourceVectorAllValues)) + if (!InitArrayElements(cx, obj, 0, argc, argv)) return JS_FALSE; newlen += argc; @@ -2384,10 +2325,8 @@ array_splice(JSContext *cx, uintN argc, Value *vp) if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) && !js_PrototypeHasIndexedProperties(cx, obj2) && end <= obj->getDenseArrayCapacity()) { - if (!InitArrayObject(cx, obj2, count, obj->getDenseArrayElements() + begin, - obj->getDenseArrayCount() != obj->getArrayLength())) { + if (!InitArrayObject(cx, obj2, count, obj->getDenseArrayElements() + begin)) return JS_FALSE; - } } else { for (last = begin; last < end; last++) { if (!JS_CHECK_OPERATION_LIMIT(cx) || @@ -2419,14 +2358,10 @@ array_splice(JSContext *cx, uintN argc, Value *vp) Value *srcbeg = arraybeg + last - 1; Value *srcend = arraybeg + end - 1; Value *dstbeg = srcbeg + delta; - for (Value *src = srcbeg, *dst = dstbeg; src > srcend; --src, --dst) { - Value srcval = *src; - if (JS_UNLIKELY(!srcval.isMagic(JS_ARRAY_HOLE) && dst->isMagic(JS_ARRAY_HOLE))) - obj->incDenseArrayCountBy(1); - *dst = srcval; - } + for (Value *src = srcbeg, *dst = dstbeg; src > srcend; --src, --dst) + *dst = *src; - obj->setDenseArrayLength(obj->getArrayLength() + delta); + obj->setArrayLength(obj->getArrayLength() + delta); } else { /* (uint) end could be 0, so we can't use a vanilla >= test. */ while (last-- > end) { @@ -2447,12 +2382,8 @@ array_splice(JSContext *cx, uintN argc, Value *vp) Value *srcbeg = arraybeg + end; Value *srcend = arraybeg + length; Value *dstbeg = srcbeg - delta; - for (Value *src = srcbeg, *dst = dstbeg; src < srcend; ++src, ++dst) { - Value srcval = *src; - if (JS_UNLIKELY(!srcval.isMagic(JS_ARRAY_HOLE) && dst->isMagic(JS_ARRAY_HOLE))) - obj->incDenseArrayCountBy(1); - *dst = srcval; - } + for (Value *src = srcbeg, *dst = dstbeg; src < srcend; ++src, ++dst) + *dst = *src; } else { for (last = end; last < length; last++) { if (!JS_CHECK_OPERATION_LIMIT(cx) || @@ -2469,8 +2400,7 @@ array_splice(JSContext *cx, uintN argc, Value *vp) * Copy from argv into the hole to complete the splice, and update length in * case we deleted elements from the end. */ - return InitArrayElements(cx, obj, begin, argc, argv, TargetElementsMayContainValues, - SourceVectorAllValues) && + return InitArrayElements(cx, obj, begin, argc, argv) && js_SetLengthProperty(cx, obj, length); } @@ -2498,11 +2428,10 @@ array_concat(JSContext *cx, uintN argc, Value *vp) */ length = aobj->getArrayLength(); jsuint capacity = aobj->getDenseArrayCapacity(); - nobj = js_NewArrayObject(cx, JS_MIN(length, capacity), aobj->getDenseArrayElements(), - aobj->getDenseArrayCount() != length); + nobj = js_NewArrayObject(cx, JS_MIN(length, capacity), aobj->getDenseArrayElements()); if (!nobj) return JS_FALSE; - nobj->setDenseArrayLength(length); + nobj->setArrayLength(length); vp->setObject(*nobj); if (argc == 0) return JS_TRUE; @@ -2614,8 +2543,7 @@ array_slice(JSContext *cx, uintN argc, Value *vp) if (obj->isDenseArray() && end <= obj->getDenseArrayCapacity() && !js_PrototypeHasIndexedProperties(cx, obj)) { - nobj = js_NewArrayObject(cx, end - begin, obj->getDenseArrayElements() + begin, - obj->getDenseArrayCount() != obj->getArrayLength()); + nobj = js_NewArrayObject(cx, end - begin, obj->getDenseArrayElements() + begin); if (!nobj) return JS_FALSE; vp->setObject(*nobj); @@ -3070,8 +2998,7 @@ js_NewEmptyArray(JSContext* cx, JSObject* proto) obj->map = const_cast(&SharedArrayMap); obj->init(&js_ArrayClass, proto, proto->getParent(), NullValue()); - obj->setDenseArrayLength(0); - obj->setDenseArrayCount(0); + obj->setArrayLength(0); return obj; } #ifdef JS_TRACER @@ -3086,7 +3013,7 @@ js_NewEmptyArrayWithLength(JSContext* cx, JSObject* proto, int32 len) JSObject *obj = js_NewEmptyArray(cx, proto); if (!obj) return NULL; - obj->setDenseArrayLength(len); + obj->setArrayLength(len); return obj; } #ifdef JS_TRACER @@ -3100,7 +3027,7 @@ js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len) JSObject* obj = js_NewEmptyArray(cx, proto); if (!obj) return NULL; - obj->setDenseArrayLength(len); + obj->setArrayLength(len); if (!obj->resizeDenseArrayElements(cx, 0, JS_MAX(len, ARRAY_CAPACITY_MIN))) return NULL; return obj; @@ -3123,7 +3050,7 @@ js_InitArrayClass(JSContext *cx, JSObject *obj) } JSObject * -js_NewArrayObject(JSContext *cx, jsuint length, const Value *vector, bool holey) +js_NewArrayObject(JSContext *cx, jsuint length, const Value *vector) { JSObject *obj = NewDenseArrayObject(cx); if (!obj) @@ -3137,7 +3064,7 @@ js_NewArrayObject(JSContext *cx, jsuint length, const Value *vector, bool holey) { AutoObjectRooter tvr(cx, obj); - if (!InitArrayObject(cx, obj, length, vector, holey)) + if (!InitArrayObject(cx, obj, length, vector)) obj = NULL; } @@ -3151,7 +3078,7 @@ js_NewSlowArrayObject(JSContext *cx) { JSObject *obj = NewObject(cx, &js_SlowArrayClass, NULL, NULL); if (obj) - obj->setSlowArrayLength(0); + obj->setArrayLength(0); return obj; } @@ -3178,8 +3105,7 @@ js_ArrayInfo(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value *rval) array->isDenseArray()) ? "dense" : "sparse", array->getArrayLength()); if (array->isDenseArray()) { - fprintf(stderr, ", count %lu, capacity %lu", - array->getDenseArrayCount(), + fprintf(stderr, ", capacity %lu", array->getDenseArrayCapacity()); } fputs(")\n", stderr); @@ -3263,7 +3189,6 @@ js_NewArrayObjectWithCapacity(JSContext *cx, uint32_t capacity, jsval **vector) if (!obj) return NULL; - obj->setDenseArrayCount(capacity); *vector = Jsvalify(obj->getDenseArrayElements()); return obj; } @@ -3311,8 +3236,6 @@ js_CloneDensePrimitiveArray(JSContext *cx, JSObject *obj, JSObject **clone) if (!vector.reserve(jsvalCount)) return JS_FALSE; - jsuint holeCount = 0; - for (jsuint i = 0; i < jsvalCount; i++) { const Value &val = obj->dslots[i]; @@ -3320,8 +3243,6 @@ js_CloneDensePrimitiveArray(JSContext *cx, JSObject *obj, JSObject **clone) // Strings must be made immutable before being copied to a clone. if (!js_MakeStringImmutable(cx, val.toString())) return JS_FALSE; - } else if (val.isMagic(JS_ARRAY_HOLE)) { - holeCount++; } else if (val.isObject()) { /* * This wasn't an array of primitives. Return JS_TRUE but a null @@ -3342,8 +3263,7 @@ js_CloneDensePrimitiveArray(JSContext *cx, JSObject *obj, JSObject **clone) AutoObjectRooter cloneRoot(cx, *clone); memcpy(buffer, vector.begin(), jsvalCount * sizeof (jsval)); - (*clone)->setDenseArrayLength(length); - (*clone)->setDenseArrayCount(length - holeCount); + (*clone)->setArrayLength(length); return JS_TRUE; } diff --git a/js/src/jsarray.h b/js/src/jsarray.h index 6658b61a0692..faaa236874da 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -147,7 +147,7 @@ extern JSObject * JS_FASTCALL js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len); extern JSObject * -js_NewArrayObject(JSContext *cx, jsuint length, const js::Value *vector, bool holey = false); +js_NewArrayObject(JSContext *cx, jsuint length, const js::Value *vector); /* Create an array object that starts out already made slow/sparse. */ extern JSObject * diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 23d6084d7ce8..7c1fae48765f 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1203,6 +1203,7 @@ struct JSCompartment { struct JSGCTracer : public JSTracer { uint32 color; + js::Vector arraysToSlowify; }; struct JSRuntime { diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 4ebf6a62316c..35165878b967 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -3161,6 +3161,16 @@ GC(JSContext *cx GCTIMER_PARAM) */ js_SweepScriptFilenames(rt); + /* + * Slowify arrays we have accumulated. + */ + while (!trc.arraysToSlowify.empty()) { + JSObject *obj = trc.arraysToSlowify.back(); + trc.arraysToSlowify.popBack(); + if (IsMarkedGCThing(obj)) + obj->makeDenseArraySlow(cx); + } + /* * Destroy arenas after we finished the sweeping so finalizers can safely * use js_IsAboutToBeFinalized(). diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index a7f47e5fdcc3..2540e6662996 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -4446,8 +4446,7 @@ BEGIN_CASE(JSOP_GETELEM) if (obj->isDenseArray()) { jsuint idx = jsuint(i); - if (idx < obj->getArrayLength() && - idx < obj->getDenseArrayCapacity()) { + if (idx < obj->getDenseArrayCapacity()) { copyFrom = obj->addressOfDenseArrayElement(idx); if (!copyFrom->isMagic()) goto end_getelem; @@ -4538,8 +4537,7 @@ BEGIN_CASE(JSOP_SETELEM) if (js_PrototypeHasIndexedProperties(cx, obj)) break; if ((jsuint)i >= obj->getArrayLength()) - obj->setDenseArrayLength(i + 1); - obj->incDenseArrayCountBy(1); + obj->setArrayLength(i + 1); } obj->setDenseArrayElement(i, regs.sp[-1]); goto end_setelem; @@ -5924,7 +5922,7 @@ BEGIN_CASE(JSOP_NEWARRAY) { len = GET_UINT16(regs.pc); cx->assertValidStackDepth(len); - JSObject *obj = js_NewArrayObject(cx, len, regs.sp - len, JS_TRUE); + JSObject *obj = js_NewArrayObject(cx, len, regs.sp - len); if (!obj) goto error; regs.sp -= len - 1; diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 3cfd0c3d876b..84105c81110d 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -277,7 +277,7 @@ EnumerateDenseArrayProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uint return false; } - if (pobj->getDenseArrayCount() > 0) { + if (pobj->getArrayLength() > 0) { size_t capacity = pobj->getDenseArrayCapacity(); Value *vp = pobj->dslots; for (size_t i = 0; i < capacity; ++i, ++vp) { diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index c664d421a3f0..ebaa45d51fbb 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1863,7 +1863,7 @@ obj_keys(JSContext *cx, uintN argc, Value *vp) } JS_ASSERT(props.length() <= UINT32_MAX); - JSObject *aobj = js_NewArrayObject(cx, jsuint(vals.length()), vals.begin(), false); + JSObject *aobj = js_NewArrayObject(cx, jsuint(vals.length()), vals.begin()); if (!aobj) return JS_FALSE; vp->setObject(*aobj); @@ -2342,7 +2342,7 @@ DefinePropertyOnArray(JSContext *cx, JSObject *obj, const PropDesc &desc, if (index >= oldLen) { JS_ASSERT(index != UINT32_MAX); - obj->setSlowArrayLength(index + 1); + obj->setArrayLength(index + 1); } *rval = true; diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 31f919cfca71..e82557b2337e 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -475,35 +475,20 @@ struct JSObject { // Used by dense and slow arrays. static const uint32 JSSLOT_ARRAY_LENGTH = JSSLOT_PRIVATE; - // Used only by dense arrays. - static const uint32 JSSLOT_DENSE_ARRAY_COUNT = JSSLOT_PRIVATE + 1; - static const uint32 JSSLOT_DENSE_ARRAY_MINLENCAP = JSSLOT_PRIVATE + 2; - // This assertion must remain true; see comment in js_MakeArraySlow(). // (Nb: This method is never called, it just contains a static assertion. // The static assertion isn't inline because that doesn't work on Mac.) inline void staticAssertArrayLengthIsInPrivateSlot(); - inline uint32 uncheckedGetArrayLength() const; - inline uint32 uncheckedGetDenseArrayCapacity() const; - public: static const uint32 DENSE_ARRAY_FIXED_RESERVED_SLOTS = 3; inline uint32 getArrayLength() const; - inline void setDenseArrayLength(uint32 length); - inline void setSlowArrayLength(uint32 length); - - inline uint32 getDenseArrayCount() const; - inline void setDenseArrayCount(uint32 count); - inline void incDenseArrayCountBy(uint32 posDelta); - inline void decDenseArrayCountBy(uint32 negDelta); + inline void setArrayLength(uint32 length); inline uint32 getDenseArrayCapacity() const; inline void setDenseArrayCapacity(uint32 capacity); // XXX: bug 558263 will remove this - inline bool isDenseArrayMinLenCapOk(bool strictAboutLength = true) const; - inline const js::Value &getDenseArrayElement(uint32 i) const; inline js::Value *addressOfDenseArrayElement(uint32 i); inline void setDenseArrayElement(uint32 i, const js::Value &v); diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index d0d278566e2e..f94df13c57df 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -132,96 +132,25 @@ JSObject::staticAssertArrayLengthIsInPrivateSlot() JS_STATIC_ASSERT(JSSLOT_ARRAY_LENGTH == JSSLOT_PRIVATE); } -inline bool -JSObject::isDenseArrayMinLenCapOk(bool strictAboutLength) const -{ - JS_ASSERT(isDenseArray()); - - // This function can be called while the LENGTH and MINLENCAP slots are - // still set to JSVAL_VOID and there are no dslots (ie. the capacity is - // zero). If 'strictAboutLength' is false we allow this. - if (!strictAboutLength && - fslots[JSSLOT_ARRAY_LENGTH].isUndefined() && - uncheckedGetDenseArrayCapacity() == 0) { - return true; - } - - uint32 length = uncheckedGetArrayLength(); - uint32 capacity = uncheckedGetDenseArrayCapacity(); - uint32 minLenCap = fslots[JSSLOT_DENSE_ARRAY_MINLENCAP].toPrivateUint32(); - return minLenCap == JS_MIN(length, capacity); -} - -inline uint32 -JSObject::uncheckedGetArrayLength() const -{ - return fslots[JSSLOT_ARRAY_LENGTH].toPrivateUint32(); -} - inline uint32 JSObject::getArrayLength() const { JS_ASSERT(isArray()); - JS_ASSERT_IF(isDenseArray(), isDenseArrayMinLenCapOk()); - return uncheckedGetArrayLength(); + return fslots[JSSLOT_ARRAY_LENGTH].toPrivateUint32(); } inline void -JSObject::setDenseArrayLength(uint32 length) +JSObject::setArrayLength(uint32 length) { - JS_ASSERT(isDenseArray()); + JS_ASSERT(isArray()); fslots[JSSLOT_ARRAY_LENGTH].setPrivateUint32(length); - uint32 capacity = uncheckedGetDenseArrayCapacity(); - fslots[JSSLOT_DENSE_ARRAY_MINLENCAP].setPrivateUint32(JS_MIN(length, capacity)); -} - -inline void -JSObject::setSlowArrayLength(uint32 length) -{ - JS_ASSERT(isSlowArray()); - fslots[JSSLOT_ARRAY_LENGTH].setPrivateUint32(length); -} - -inline uint32 -JSObject::getDenseArrayCount() const -{ - JS_ASSERT(isDenseArray()); - return fslots[JSSLOT_DENSE_ARRAY_COUNT].toPrivateUint32(); -} - -inline void -JSObject::setDenseArrayCount(uint32 count) -{ - JS_ASSERT(isDenseArray()); - fslots[JSSLOT_DENSE_ARRAY_COUNT].setPrivateUint32(count); -} - -inline void -JSObject::incDenseArrayCountBy(uint32 posDelta) -{ - JS_ASSERT(isDenseArray()); - fslots[JSSLOT_DENSE_ARRAY_COUNT].getPrivateUint32Ref() += posDelta; -} - -inline void -JSObject::decDenseArrayCountBy(uint32 negDelta) -{ - JS_ASSERT(isDenseArray()); - fslots[JSSLOT_DENSE_ARRAY_COUNT].getPrivateUint32Ref() -= negDelta; -} - -inline uint32 -JSObject::uncheckedGetDenseArrayCapacity() const -{ - return dslots ? dslots[-1].toPrivateUint32() : 0; } inline uint32 JSObject::getDenseArrayCapacity() const { JS_ASSERT(isDenseArray()); - JS_ASSERT(isDenseArrayMinLenCapOk(/* strictAboutLength = */false)); - return uncheckedGetDenseArrayCapacity(); + return dslots ? dslots[-1].toPrivateUint32() : 0; } inline void @@ -230,8 +159,6 @@ JSObject::setDenseArrayCapacity(uint32 capacity) JS_ASSERT(isDenseArray()); JS_ASSERT(dslots); dslots[-1].setPrivateUint32(capacity); - uint32 length = uncheckedGetArrayLength(); - fslots[JSSLOT_DENSE_ARRAY_MINLENCAP].setPrivateUint32(JS_MIN(length, capacity)); } inline const js::Value & @@ -273,16 +200,12 @@ JSObject::freeDenseArrayElements(JSContext *cx) cx->free(dslots - 1); dslots = NULL; } - fslots[JSSLOT_DENSE_ARRAY_MINLENCAP].setPrivateUint32(0); - JS_ASSERT(isDenseArrayMinLenCapOk(/* strictAboutLength = */false)); } inline void JSObject::voidDenseOnlyArraySlots() { JS_ASSERT(isDenseArray()); - fslots[JSSLOT_DENSE_ARRAY_COUNT].setUndefined(); - fslots[JSSLOT_DENSE_ARRAY_MINLENCAP].setUndefined(); } inline void diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 3d717debdd24..3c4f9424f558 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -11037,9 +11037,6 @@ TraceRecorder::newArray(JSObject* ctor, uint32 argc, Value* argv, Value* rval) for (uint32 i = 0; i < argc && !outOfMemory(); i++) { stobj_set_dslot(arr_ins, i, dslots_ins, argv[i], get(&argv[i])); } - - if (argc > 0) - set_array_fslot(arr_ins, JSObject::JSSLOT_DENSE_ARRAY_COUNT, argc); } set(rval, arr_ins); @@ -13654,23 +13651,27 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ LIns* idx_ins = makeNumberInt32(get(&ival)); VMSideExit* exit = snapshot(BRANCH_EXIT); - /* check that the index is within bounds */ + + /* + * Arrays have both a length and a capacity, but we only need to check + * |index < capacity|; in the case where |length < index < capacity| + * the entries [length..capacity-1] will have already been marked as + * holes by resizeDenseArrayElements() so we can read them and get + * the correct value. + */ LIns* dslots_ins = addName(lir->insLoad(LIR_ldp, obj_ins, offsetof(JSObject, dslots), ACC_OTHER), "dslots"); jsuint capacity = obj->getDenseArrayCapacity(); - bool within = (jsuint(idx) < obj->getArrayLength() && jsuint(idx) < capacity); + bool within = (jsuint(idx) < capacity); if (!within) { - /* If not idx < min(length, capacity), stay on trace (and read value as undefined). */ - JS_ASSERT(obj->isDenseArrayMinLenCapOk()); - LIns* minLenCap = - addName(stobj_get_fslot_uint32(obj_ins, JSObject::JSSLOT_DENSE_ARRAY_MINLENCAP), "minLenCap"); - LIns* br = lir->insBranch(LIR_jf, - lir->ins2(LIR_ltui, idx_ins, minLenCap), - NULL); + /* Also stay on trace if dslots is NULL. */ + LIns *br = lir->insBranch(LIR_jt, lir->insEqP_0(dslots_ins), NULL); - lir->insGuard(LIR_x, NULL, createGuardRecord(exit)); - LIns* label = lir->ins0(LIR_label); - br->setTarget(label); + /* If not idx < capacity, stay on trace (and read value as undefined). */ + LIns* capacity_ins = addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity"); + guard(true, lir->ins2(LIR_geui, idx_ins, capacity_ins), exit); + + br->setTarget(lir->ins0(LIR_label)); CHECK_STATUS(guardPrototypeHasNoIndexedProperties(obj, obj_ins, MISMATCH_EXIT)); @@ -13680,11 +13681,10 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ return RECORD_CONTINUE; } - /* Guard array min(length, capacity). */ - JS_ASSERT(obj->isDenseArrayMinLenCapOk()); - LIns* minLenCap = - addName(stobj_get_fslot_uint32(obj_ins, JSObject::JSSLOT_DENSE_ARRAY_MINLENCAP), "minLenCap"); - guard(true, lir->ins2(LIR_ltui, idx_ins, minLenCap), exit); + /* Guard that index is within capacity. */ + guard(false, lir->insEqP_0(dslots_ins), exit); + LIns* capacity_ins = addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity"); + guard(true, lir->ins2(LIR_ltui, idx_ins, capacity_ins), exit); /* Load the value and guard on its type to unbox it. */ vp = &obj->dslots[jsuint(idx)]; @@ -15810,9 +15810,6 @@ TraceRecorder::record_JSOP_NEWARRAY() stobj_set_dslot(v_ins, i, dslots_ins, v, get(&v)); } - if (count > 0) - set_array_fslot(v_ins, JSObject::JSSLOT_DENSE_ARRAY_COUNT, count); - stack(-int(len), v_ins); return ARECORD_CONTINUE; } From 9ae6a6411c7475b59b11e5eb392797e55c3230ab Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Thu, 22 Jul 2010 22:59:59 +0200 Subject: [PATCH 02/19] bug 580458 - trigger operation callback once per thread, not once per cx. r=gal,mrbkap --- js/src/jsapi.cpp | 19 ++++++++++++++----- js/src/jscntxt.cpp | 10 +++++----- js/src/jscntxt.h | 27 +++++++++++++++++++-------- js/src/jsgc.cpp | 14 +++++++++----- js/src/jslock.cpp | 39 +-------------------------------------- js/src/jslock.h | 3 --- js/src/jstracer.cpp | 9 +++++---- 7 files changed, 53 insertions(+), 68 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 218e0843f68e..1259e60eeb00 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4854,12 +4854,21 @@ JS_PUBLIC_API(void) JS_TriggerOperationCallback(JSContext *cx) { /* - * Use JS_ATOMIC_SET in the hope that it will make sure the write - * will become immediately visible to other processors polling - * cx->operationCallbackFlag. Note that we only care about - * visibility here, not read/write ordering. + * We allow for cx to come from another thread. Thus we must deal with + * possible JS_ClearContextThread calls when accessing cx->thread. But we + * assume that the calling thread is in a request so JSThread cannot be + * GC-ed. */ - JS_ATOMIC_SET(&cx->operationCallbackFlag, 1); + JSThreadData *td; +#ifdef JS_THREADSAFE + JSThread *thread = cx->thread; + if (!thread) + return; + td = &thread->data; +#else + td = JS_THREAD_DATA(cx); +#endif + td->triggerOperationCallback(); } JS_PUBLIC_API(void) diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 460c770ce3bb..5e8e28ae0a9f 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -1879,14 +1879,15 @@ js_GetErrorMessage(void *userRef, const char *locale, const uintN errorNumber) JSBool js_InvokeOperationCallback(JSContext *cx) { - JS_ASSERT(cx->operationCallbackFlag); + JS_ASSERT(cx->requestDepth >= 1); + JS_ASSERT(JS_THREAD_DATA(cx)->operationCallbackFlag); /* * Reset the callback flag first, then yield. If another thread is racing * us here we will accumulate another callback request which will be * serviced at the next opportunity. */ - cx->operationCallbackFlag = 0; + JS_THREAD_DATA(cx)->operationCallbackFlag = 0; /* * Unless we are going to run the GC, we automatically yield the current @@ -1935,9 +1936,8 @@ js_TriggerAllOperationCallbacks(JSRuntime *rt, JSBool gcLocked) #ifdef JS_THREADSAFE Conditionally lockIf(!gcLocked, rt); #endif - JSContext *iter = NULL; - while (JSContext *acx = js_ContextIterator(rt, JS_FALSE, &iter)) - JS_TriggerOperationCallback(acx); + for (ThreadDataIter i(rt); !i.empty(); i.popFront()) + i.threadData()->triggerOperationCallback(); } JSStackFrame * diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 7c1fae48765f..49c271e4979f 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -965,6 +965,12 @@ struct JSPendingProxyOperation { }; struct JSThreadData { + /* + * If this flag is set, we were asked to call back the operation callback + * as soon as possible. + */ + volatile int32 operationCallbackFlag; + JSGCFreeLists gcFreeLists; /* Keeper of the contiguous stack used by all contexts in this thread. */ @@ -1027,6 +1033,16 @@ struct JSThreadData { void finish(); void mark(JSTracer *trc); void purge(JSContext *cx); + + void triggerOperationCallback() { + /* + * Use JS_ATOMIC_SET in the hope that it will make sure the write will + * become immediately visible to other processors polling the flag. + * Note that we only care about visibility here, not read/write + * ordering. + */ + JS_ATOMIC_SET(&operationCallbackFlag, 1); + } }; #ifdef JS_THREADSAFE @@ -1678,12 +1694,6 @@ struct JSContext { explicit JSContext(JSRuntime *rt); - /* - * If this flag is set, we were asked to call back the operation callback - * as soon as possible. - */ - volatile jsint operationCallbackFlag; - /* JSRuntime contextList linkage. */ JSCList link; @@ -2942,8 +2952,9 @@ extern JSErrorFormatString js_ErrorFormatString[JSErr_Limit]; * This macro can run the full GC. Return true if it is OK to continue and * false otherwise. */ -#define JS_CHECK_OPERATION_LIMIT(cx) \ - (!(cx)->operationCallbackFlag || js_InvokeOperationCallback(cx)) +#define JS_CHECK_OPERATION_LIMIT(cx) \ + (JS_ASSERT((cx)->requestDepth >= 1), \ + (!JS_THREAD_DATA(cx)->operationCallbackFlag || js_InvokeOperationCallback(cx))) /* * Invoke the operation callback and return false if the current execution diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 35165878b967..fcc6325d178a 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -3327,12 +3327,16 @@ BeginGCSession(JSContext *cx) rt->gcThread = cx->thread; /* - * Notify all operation callbacks, which will give them a chance to yield - * their current request. Contexts that are not currently executing will - * perform their callback at some later point, which then will be - * unnecessary, but harmless. + * Notify operation callbacks on other threads, which will give them a + * chance to yield their requests. Threads without requests perform their + * callback at some later point, which then will be unnecessary, but + * harmless. */ - js_NudgeOtherContexts(cx); + for (JSThread::Map::Range r = rt->threads.all(); !r.empty(); r.popFront()) { + JSThread *thread = r.front().value; + if (thread != cx->thread) + thread->data.triggerOperationCallback(); + } /* * Discount the request on the current thread from contributing to diff --git a/js/src/jslock.cpp b/js/src/jslock.cpp index 0c291e951b01..2b349e330287 100644 --- a/js/src/jslock.cpp +++ b/js/src/jslock.cpp @@ -518,43 +518,6 @@ FinishSharingTitle(JSContext *cx, JSTitle *title) JS_RUNTIME_METER(cx->runtime, sharedTitles); } -/* - * Notify all contexts that are currently in a request, which will give them a - * chance to yield their current request. - */ -void -js_NudgeOtherContexts(JSContext *cx) -{ - JSRuntime *rt = cx->runtime; - JSContext *acx = NULL; - - while ((acx = js_NextActiveContext(rt, acx)) != NULL) { - if (cx != acx) - JS_TriggerOperationCallback(acx); - } -} - -/* - * Notify all contexts that are currently in a request and execute on this - * specific thread. - */ -static void -NudgeThread(JSRuntime *rt, JSThread *thread) -{ - JS_ASSERT(thread); - - /* - * We cannot walk here over thread->contextList as that is manipulated - * outside the GC lock and must be accessed only from the the thread that - * owns JSThread. - */ - JSContext *acx = NULL; - while ((acx = js_NextActiveContext(rt, acx)) != NULL) { - if (acx->thread == thread) - JS_TriggerOperationCallback(acx); - } -} - /* * Given a title with apparently non-null ownercx different from cx, try to * set ownercx to cx, claiming exclusive (single-threaded) ownership of title. @@ -647,7 +610,7 @@ ClaimTitle(JSTitle *title, JSContext *cx) * But before waiting, we force the operation callback for that other * thread so it can quickly suspend. */ - NudgeThread(rt, ownercx->thread); + JS_THREAD_DATA(ownercx)->triggerOperationCallback(); JS_ASSERT(!cx->thread->titleToShare); cx->thread->titleToShare = title; diff --git a/js/src/jslock.h b/js/src/jslock.h index 3be65498cdb1..4a9ca6573b80 100644 --- a/js/src/jslock.h +++ b/js/src/jslock.h @@ -215,9 +215,6 @@ extern void js_FinishLock(JSThinLock *); extern void js_ShareWaitingTitles(JSContext *cx); -extern void -js_NudgeOtherContexts(JSContext *cx); - #ifdef DEBUG #define JS_IS_RUNTIME_LOCKED(rt) js_IsRuntimeLocked(rt) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 3c4f9424f558..3ea3e661ee54 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2343,12 +2343,13 @@ TraceRecorder::TraceRecorder(JSContext* cx, VMSideExit* anchor, VMFragment* frag if (fragment == fragment->root) { /* * We poll the operation callback request flag. It is updated asynchronously whenever - * the callback is to be invoked. + * the callback is to be invoked. We can use INS_CONSTPTR here as JIT-ed code is per + * thread and cannot outlive the corresponding JSThreaData. */ // XXX: this load is volatile. If bug 545406 (loop-invariant code // hoisting) is implemented this fact will need to be made explicit. - LIns* x = - lir->insLoad(LIR_ldi, cx_ins, offsetof(JSContext, operationCallbackFlag), ACC_LOAD_ANY); + LIns* flagptr = INS_CONSTPTR((void *) &JS_THREAD_DATA(cx)->operationCallbackFlag); + LIns* x = lir->insLoad(LIR_ldi, flagptr, 0, ACC_LOAD_ANY); guard(true, lir->insEqI_0(x), snapshot(TIMEOUT_EXIT)); } @@ -7124,7 +7125,7 @@ MonitorLoopEdge(JSContext* cx, uintN& inlineCallCount, RecordReason reason) } /* Do not enter the JIT code with a pending operation callback. */ - if (cx->operationCallbackFlag) { + if (JS_THREAD_DATA(cx)->operationCallbackFlag) { #ifdef MOZ_TRACEVIS tvso.r = R_CALLBACK_PENDING; #endif From d7a21deef38b2ee5ae91403c2a752d426517fdb6 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Fri, 23 Jul 2010 13:33:15 +0200 Subject: [PATCH 03/19] bug 552266 - - asserting that autorooters are used only under a request. r=mrbkap --- dom/base/nsJSEnvironment.cpp | 4 ++-- js/src/jscntxt.h | 6 ++++++ modules/plugin/base/src/nsNPAPIPlugin.cpp | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index 2f975cd9542c..9e411f20009f 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -2172,6 +2172,8 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, void *aScope, void *aHandler NS_TIME_FUNCTION_FMT(1.0, "%s (line %d) (function: %s)", MOZ_FUNCTION_NAME, __LINE__, JS_GetFunctionName(static_cast(JS_GetPrivate(mContext, static_cast(aHandler))))); + + JSAutoRequest ar(mContext); JSObject* target = nsnull; nsresult rv = JSObjectFromInterface(aTarget, aScope, &target); NS_ENSURE_SUCCESS(rv, rv); @@ -2215,7 +2217,6 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, void *aScope, void *aHandler } jsval funval = OBJECT_TO_JSVAL(static_cast(aHandler)); - JSAutoRequest ar(mContext); JSAutoCrossCompartmentCall accc; if (!accc.enter(mContext, target)) { stack->Pop(nsnull); @@ -2247,7 +2248,6 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, void *aScope, void *aHandler // Convert to variant before calling ScriptEvaluated, as it may GC, meaning // we would need to root rval. - JSAutoRequest ar(mContext); if (NS_SUCCEEDED(rv)) { if (rval == JSVAL_NULL) *arv = nsnull; diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 49c271e4979f..5dbd97dc4acf 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -2224,11 +2224,17 @@ class AutoGCRooter { : down(cx->autoGCRooters), tag(tag), context(cx) { JS_ASSERT(this != cx->autoGCRooters); +#ifdef JS_THREADSAFE + JS_ASSERT(cx->requestDepth != 0); +#endif cx->autoGCRooters = this; } ~AutoGCRooter() { JS_ASSERT(this == context->autoGCRooters); +#ifdef JS_THREADSAFE + JS_ASSERT(context->requestDepth != 0); +#endif context->autoGCRooters = down; } diff --git a/modules/plugin/base/src/nsNPAPIPlugin.cpp b/modules/plugin/base/src/nsNPAPIPlugin.cpp index 7edfefaae4f7..bc0c24dc3c4c 100644 --- a/modules/plugin/base/src/nsNPAPIPlugin.cpp +++ b/modules/plugin/base/src/nsNPAPIPlugin.cpp @@ -1560,6 +1560,8 @@ _evaluate(NPP npp, NPObject* npobj, NPString *script, NPVariant *result) JSContext *cx = GetJSContextFromDoc(doc); NS_ENSURE_TRUE(cx, false); + JSAutoRequest req(cx); + nsCOMPtr scx = GetScriptContextFromJSContext(cx); NS_ENSURE_TRUE(scx, false); From 745062e740fdc71bae8ca42085ebbe23ac4f6a17 Mon Sep 17 00:00:00 2001 From: Jacek Caban Date: Fri, 23 Jul 2010 17:03:43 +0200 Subject: [PATCH 04/19] Bug 578340 - Sync jschar and PRUnichar on Windows r=jorendorff --- js/src/jsapi.cpp | 4 ++++ js/src/jspubtd.h | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 1259e60eeb00..9d101e496b2b 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -121,6 +121,10 @@ JS_PUBLIC_DATA(jsval) JSVAL_TRUE = { BUILD_JSVAL(JSVAL_TAG_BOOLEAN, JS_TRUE) JS_PUBLIC_DATA(jsval) JSVAL_VOID = { BUILD_JSVAL(JSVAL_TAG_UNDEFINED, 0) }; #endif +/* Make sure that jschar is two bytes unsigned integer */ +JS_STATIC_ASSERT((jschar)-1 > 0); +JS_STATIC_ASSERT(sizeof(jschar) == 2); + JS_PUBLIC_API(int64) JS_Now() { diff --git a/js/src/jspubtd.h b/js/src/jspubtd.h index 5b44c866ae85..e3df1494199d 100644 --- a/js/src/jspubtd.h +++ b/js/src/jspubtd.h @@ -49,12 +49,18 @@ JS_BEGIN_EXTERN_C /* Scalar typedefs. */ -typedef uint16 jschar; typedef int32 jsint; typedef uint32 jsuint; typedef float64 jsdouble; typedef int32 jsrefcount; /* PRInt32 if JS_THREADSAFE, see jslock.h */ +#ifdef WIN32 +typedef wchar_t jschar; +#else +typedef uint16 jschar; +#endif + + /* * Run-time version enumeration. See jsversion.h for compile-time counterparts * to these values that may be selected by the JS_VERSION macro, and tested by From cba1ad53db0d96b83ff73a9e0336404d2805fb9f Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Fri, 23 Jul 2010 11:32:50 -0500 Subject: [PATCH 05/19] Fix debug non-threadsafe builds. rs=brendan. --- js/src/jscntxt.cpp | 2 +- js/src/jscntxt.h | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 5e8e28ae0a9f..505464736a80 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -1879,7 +1879,7 @@ js_GetErrorMessage(void *userRef, const char *locale, const uintN errorNumber) JSBool js_InvokeOperationCallback(JSContext *cx) { - JS_ASSERT(cx->requestDepth >= 1); + JS_ASSERT_REQUEST_DEPTH(cx); JS_ASSERT(JS_THREAD_DATA(cx)->operationCallbackFlag); /* diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 5dbd97dc4acf..914b44f860a1 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -2953,13 +2953,19 @@ extern JSErrorFormatString js_ErrorFormatString[JSErr_Limit]; # define JS_CHECK_STACK_SIZE(cx, lval) ((jsuword)&(lval) > (cx)->stackLimit) #endif +#ifdef JS_THREADSAFE +# define JS_ASSERT_REQUEST_DEPTH(cx) JS_ASSERT((cx)->requestDepth >= 1) +#else +# define JS_ASSERT_REQUEST_DEPTH(cx) ((void) 0) +#endif + /* * If the operation callback flag was set, call the operation callback. * This macro can run the full GC. Return true if it is OK to continue and * false otherwise. */ #define JS_CHECK_OPERATION_LIMIT(cx) \ - (JS_ASSERT((cx)->requestDepth >= 1), \ + (JS_ASSERT_REQUEST_DEPTH(cx), \ (!JS_THREAD_DATA(cx)->operationCallbackFlag || js_InvokeOperationCallback(cx))) /* From 401ce49c59bac820b822629643ada5f853f4b6be Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Sat, 10 Jul 2010 11:48:00 -0700 Subject: [PATCH 06/19] arguments.callee.caller does not work in FF 4 under certain circumstances (577648, r=jwalden). --- js/src/jsapi.cpp | 9 +- js/src/jsarray.cpp | 4 +- js/src/jsarray.h | 9 ++ js/src/jscntxt.cpp | 94 +++++++----- js/src/jscntxt.h | 23 ++- js/src/jsdbgapi.cpp | 11 ++ js/src/jsdbgapi.h | 33 ++++- js/src/jsemit.cpp | 3 +- js/src/jsfun.cpp | 140 ++++++++++++++---- js/src/jsfun.h | 44 ++++++ js/src/jsgc.cpp | 11 ++ js/src/jsinterp.cpp | 103 ++++++++++--- js/src/jsinterp.h | 24 ++- js/src/jsobj.cpp | 43 +++++- js/src/jsobj.h | 29 +++- js/src/jsobjinlines.h | 19 +++ js/src/jsopcode.cpp | 4 - js/src/jsparse.cpp | 9 +- js/src/jsscope.cpp | 2 +- js/src/jsscope.h | 4 +- js/src/jsscopeinlines.h | 32 +++- js/src/jsstr.cpp | 4 +- js/src/jsstr.h | 9 ++ js/src/jstracer.cpp | 49 +++++- js/src/tests/js1_8_5/regress/jstests.list | 2 + .../tests/js1_8_5/regress/regress-577648-1.js | 87 +++++++++++ .../tests/js1_8_5/regress/regress-577648-2.js | 12 ++ 27 files changed, 680 insertions(+), 133 deletions(-) create mode 100644 js/src/tests/js1_8_5/regress/regress-577648-1.js create mode 100644 js/src/tests/js1_8_5/regress/regress-577648-2.js diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 9d101e496b2b..a42968c9599c 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -546,7 +546,7 @@ static JSBool js_NewRuntimeWasCalled = JS_FALSE; #endif JSRuntime::JSRuntime() - : gcChunkAllocator(&defaultGCChunkAllocator) + : gcChunkAllocator(&defaultGCChunkAllocator) { /* Initialize infallibly first, so we can goto bad and JS_DestroyRuntime. */ JS_INIT_CLIST(&contextList); @@ -557,6 +557,13 @@ JSRuntime::JSRuntime() bool JSRuntime::init(uint32 maxbytes) { +#ifdef JS_FUNCTION_METERING + if (!methodReadBarrierCountMap.init()) + return false; + if (!unjoinedFunctionCountMap.init()) + return false; +#endif + if (!(defaultCompartment = new JSCompartment(this)) || !defaultCompartment->init()) { return false; diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index c30c2c629cad..967e527ebffd 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -1774,8 +1774,8 @@ sort_compare_strings(void *arg, const void *a, const void *b, int *result) return JS_TRUE; } -static JSBool -array_sort(JSContext *cx, uintN argc, Value *vp) +JSBool +js::array_sort(JSContext *cx, uintN argc, Value *vp) { jsuint len, newlen, i, undefs; size_t elemsize; diff --git a/js/src/jsarray.h b/js/src/jsarray.h index faaa236874da..7839f407731a 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -200,6 +200,15 @@ extern bool js_MergeSort(void *vec, size_t nel, size_t elsize, JSComparator cmp, void *arg, void *tmp, JSMergeSortElemType elemType); +/* + * The Array.prototype.sort fast-native entry point is exported for joined + * function optimization in js{interp,tracer}.cpp. + */ +namespace js { +extern JSBool +array_sort(JSContext *cx, uintN argc, js::Value *vp); +} + #ifdef DEBUG_ARRAYS extern JSBool js_ArrayInfo(JSContext *cx, JSObject *obj, uintN argc, js::Value *argv, js::Value *rval); diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 505464736a80..5a2a4515cdaa 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -959,69 +959,83 @@ private: static void DumpEvalCacheMeter(JSContext *cx) { - struct { - const char *name; - ptrdiff_t offset; - } table[] = { + if (const char *filename = getenv("JS_EVALCACHE_STATFILE")) { + struct { + const char *name; + ptrdiff_t offset; + } table[] = { #define frob(x) { #x, offsetof(JSEvalCacheMeter, x) } - EVAL_CACHE_METER_LIST(frob) + EVAL_CACHE_METER_LIST(frob) #undef frob - }; - JSEvalCacheMeter *ecm = &JS_THREAD_DATA(cx)->evalCacheMeter; + }; + JSEvalCacheMeter *ecm = &JS_THREAD_DATA(cx)->evalCacheMeter; - static JSAutoFile fp; - if (!fp) { - fp.open("/tmp/evalcache.stats", "w"); - if (!fp) + static JSAutoFile fp; + if (!fp && !fp.open(filename, "w")) return; - } - fprintf(fp, "eval cache meter (%p):\n", + fprintf(fp, "eval cache meter (%p):\n", #ifdef JS_THREADSAFE - (void *) cx->thread + (void *) cx->thread #else - (void *) cx->runtime + (void *) cx->runtime #endif - ); - for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { - fprintf(fp, "%-8.8s %llu\n", - table[i].name, - (unsigned long long int) *(uint64 *)((uint8 *)ecm + table[i].offset)); + ); + for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { + fprintf(fp, "%-8.8s %llu\n", + table[i].name, + (unsigned long long int) *(uint64 *)((uint8 *)ecm + table[i].offset)); + } + fprintf(fp, "hit ratio %g%%\n", ecm->hit * 100. / ecm->probe); + fprintf(fp, "avg steps %g\n", double(ecm->step) / ecm->probe); + fflush(fp); } - fprintf(fp, "hit ratio %g%%\n", ecm->hit * 100. / ecm->probe); - fprintf(fp, "avg steps %g\n", double(ecm->step) / ecm->probe); - fflush(fp); } # define DUMP_EVAL_CACHE_METER(cx) DumpEvalCacheMeter(cx) #endif #ifdef JS_FUNCTION_METERING +static void +DumpFunctionCountMap(const char *title, JSRuntime::FunctionCountMap &map, FILE *fp) +{ + fprintf(fp, "\n%s count map:\n", title); + + for (JSRuntime::FunctionCountMap::Range r = map.all(); !r.empty(); r.popFront()) { + JSFunction *fun = r.front().key; + int32 count = r.front().value; + + fprintf(fp, "%10d %s:%u\n", count, fun->u.i.script->filename, fun->u.i.script->lineno); + } +} + static void DumpFunctionMeter(JSContext *cx) { - struct { - const char *name; - ptrdiff_t offset; - } table[] = { + if (const char *filename = getenv("JS_FUNCTION_STATFILE")) { + struct { + const char *name; + ptrdiff_t offset; + } table[] = { #define frob(x) { #x, offsetof(JSFunctionMeter, x) } - FUNCTION_KIND_METER_LIST(frob) + FUNCTION_KIND_METER_LIST(frob) #undef frob - }; - JSFunctionMeter *fm = &cx->runtime->functionMeter; + }; + JSFunctionMeter *fm = &cx->runtime->functionMeter; - static JSAutoFile fp; - if (!fp) { - fp.open("/tmp/function.stats", "a"); - if (!fp) + static JSAutoFile fp; + if (!fp && !fp.open(filename, "w")) return; - } - fprintf(fp, "function meter (%s):\n", cx->runtime->lastScriptFilename); - for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { - fprintf(fp, "%-11.11s %d\n", - table[i].name, *(int32 *)((uint8 *)fm + table[i].offset)); + fprintf(fp, "function meter (%s):\n", cx->runtime->lastScriptFilename); + for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) + fprintf(fp, "%-19.19s %d\n", table[i].name, *(int32 *)((uint8 *)fm + table[i].offset)); + + DumpFunctionCountMap("method read barrier", cx->runtime->methodReadBarrierCountMap, fp); + DumpFunctionCountMap("unjoined function", cx->runtime->unjoinedFunctionCountMap, fp); + + putc('\n', fp); + fflush(fp); } - fflush(fp); } # define DUMP_FUNCTION_METER(cx) DumpFunctionMeter(cx) #endif diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 914b44f860a1..bed92a71ebee 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -920,7 +920,7 @@ struct TraceMonitor { # define JS_ON_TRACE(cx) JS_FALSE #endif -#ifdef DEBUG_brendan +#ifdef DEBUG # define JS_EVAL_CACHE_METERING 1 # define JS_FUNCTION_METERING 1 #endif @@ -945,7 +945,11 @@ struct JSEvalCacheMeter { #ifdef JS_FUNCTION_METERING # define FUNCTION_KIND_METER_LIST(_) \ _(allfun), _(heavy), _(nofreeupvar), _(onlyfreevar), \ - _(display), _(flat), _(setupvar), _(badfunarg) + _(display), _(flat), _(setupvar), _(badfunarg), \ + _(joinedsetmethod), _(joinedinitmethod), \ + _(joinedreplace), _(joinedsort), _(joinedmodulepat), \ + _(mreadbarrier), _(mwritebarrier), _(mwslotbarrier), \ + _(unjoined) # define identity(x) x struct JSFunctionMeter { @@ -953,8 +957,13 @@ struct JSFunctionMeter { }; # undef identity + +# define JS_FUNCTION_METER(cx,x) JS_RUNTIME_METER((cx)->runtime, functionMeter.x) +#else +# define JS_FUNCTION_METER(cx,x) ((void)0) #endif + #define NATIVE_ITER_CACHE_LOG2 8 #define NATIVE_ITER_CACHE_MASK JS_BITMASK(NATIVE_ITER_CACHE_LOG2) #define NATIVE_ITER_CACHE_SIZE JS_BIT(NATIVE_ITER_CACHE_LOG2) @@ -1575,6 +1584,14 @@ struct JSRuntime { #ifdef JS_FUNCTION_METERING JSFunctionMeter functionMeter; char lastScriptFilename[1024]; + + typedef js::HashMap, + js::SystemAllocPolicy> FunctionCountMap; + + FunctionCountMap methodReadBarrierCountMap; + FunctionCountMap unjoinedFunctionCountMap; #endif JSWrapObjectCallback wrapObjectCallback; @@ -2136,7 +2153,7 @@ JS_ALWAYS_INLINE jsbytecode * JSStackFrame::pc(JSContext *cx) const { JS_ASSERT(cx->containingSegment(this) != NULL); - return cx->fp == this ? cx->regs->pc : savedPC; + return (cx->fp == this) ? cx->regs->pc : savedPC; } /* diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 6540f5f8750d..c18bb1019adb 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -1270,6 +1270,17 @@ JS_GetFrameCalleeObject(JSContext *cx, JSStackFrame *fp) return fp->callee(); } +JS_PUBLIC_API(JSBool) +JS_GetValidFrameCalleeObject(JSContext *cx, JSStackFrame *fp, jsval *vp) +{ + Value v; + + if (!fp->getValidCalleeObject(cx, &v)) + return false; + *vp = Jsvalify(v); + return true; +} + JS_PUBLIC_API(JSBool) JS_IsDebuggerFrame(JSContext *cx, JSStackFrame *fp) { diff --git a/js/src/jsdbgapi.h b/js/src/jsdbgapi.h index 03420b4cfa94..c3d45460f8e2 100644 --- a/js/src/jsdbgapi.h +++ b/js/src/jsdbgapi.h @@ -272,11 +272,42 @@ extern JS_PUBLIC_API(void) JS_SetFrameReturnValue(JSContext *cx, JSStackFrame *fp, jsval rval); /** - * Return fp's callee function object (fp->callee) if it has one. + * Return fp's callee function object (fp->callee) if it has one. Note that + * this API cannot fail. A null return means "no callee": fp is a global or + * eval-from-global frame, not a call frame. + * + * This API began life as an infallible getter, but now it can return either: + * + * 1. An optimized closure that was compiled assuming the function could not + * escape and be called from sites the compiler could not see. + * + * 2. A "joined function object", an optimization whereby SpiderMonkey avoids + * creating fresh function objects for every evaluation of a function + * expression that is used only once by a consumer that either promises to + * clone later when asked for the value or that cannot leak the value. + * + * Because Mozilla's Gecko embedding of SpiderMonkey (and no doubt other + * embeddings) calls this API in potentially performance-sensitive ways (e.g. + * in nsContentUtils::GetDocumentFromCaller), we are leaving this API alone. It + * may now return an unwrapped non-escaping optimized closure, or a joined + * function object. Such optimized objects may work well if called from the + * correct context, never mutated or compared for identity, etc. + * + * However, if you really need to get the same callee object that JS code would + * see, which means undoing the optimizations, where an undo attempt can fail, + * then use JS_GetValidFrameCalleeObject. */ extern JS_PUBLIC_API(JSObject *) JS_GetFrameCalleeObject(JSContext *cx, JSStackFrame *fp); +/** + * Return fp's callee function object after running the deferred closure + * cloning "method read barrier". This API can fail! If the frame has no + * callee, this API returns true with JSVAL_IS_NULL(*vp). + */ +extern JS_PUBLIC_API(JSBool) +JS_GetValidFrameCalleeObject(JSContext *cx, JSStackFrame *fp, jsval *vp); + /************************************************************************/ extern JS_PUBLIC_API(const char *) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 968d0cf03709..4728b565abdc 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -6634,8 +6634,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) bool lambda = PN_OP(init) == JSOP_LAMBDA; if (lambda) ++methodInits; - if (op == JSOP_INITPROP && lambda && init->pn_funbox->joinable()) - { + if (op == JSOP_INITPROP && lambda && init->pn_funbox->joinable()) { op = JSOP_INITMETHOD; pn2->pn_op = uint8(op); } else { diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 3cc1a4b8a791..5cd4c8c34a1b 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1272,6 +1272,102 @@ JS_PUBLIC_DATA(Class) js_CallClass = { JS_CLASS_TRACE(args_or_call_trace), NULL }; +bool +JSStackFrame::getValidCalleeObject(JSContext *cx, Value *vp) +{ + if (!fun) { + vp->setObjectOrNull(callee()); + return true; + } + + /* + * See the equivalent condition in args_getProperty for ARGS_CALLEE, but + * note that here we do not want to throw, since this escape can happen via + * a foo.caller reference alone, without any debugger or indirect eval. And + * alas, it seems foo.caller is still used on the Web. + */ + if (fun->needsWrapper()) { + JSObject *wrapper = WrapEscapingClosure(cx, this, fun, fun); + if (!wrapper) + return false; + vp->setObject(*wrapper); + return true; + } + + JSObject *funobj = &calleeObject(); + vp->setObject(*funobj); + + /* + * Check for an escape attempt by a joined function object, which must go + * through the frame's |this| object's method read barrier for the method + * atom by which it was uniquely associated with a property. + */ + if (thisv.isObject()) { + JS_ASSERT(GET_FUNCTION_PRIVATE(cx, funobj) == fun); + + if (fun == funobj && fun->methodAtom()) { + JSObject *thisp = &thisv.toObject(); + JS_ASSERT(thisp->canHaveMethodBarrier()); + + JSScope *scope = thisp->scope(); + if (scope->hasMethodBarrier()) { + JSScopeProperty *sprop = scope->lookup(ATOM_TO_JSID(fun->methodAtom())); + + /* + * The method property might have been deleted while the method + * barrier scope flag stuck, so we must lookup and test here. + * + * Two cases follow: the method barrier was not crossed yet, so + * we cross it here; the method barrier *was* crossed, in which + * case we must fetch and validate the cloned (unjoined) funobj + * in the method property's slot. + * + * In either case we must allow for the method property to have + * been replaced, or its value to have been overwritten. + */ + if (sprop) { + if (sprop->isMethod() && &sprop->methodObject() == funobj) { + if (!scope->methodReadBarrier(cx, sprop, vp)) + return false; + setCalleeObject(vp->toObject()); + return true; + } + if (sprop->hasSlot()) { + Value v = thisp->getSlot(sprop->slot); + JSObject *clone; + + if (IsFunctionObject(v, &clone) && + GET_FUNCTION_PRIVATE(cx, clone) == fun && + clone->hasMethodObj(*thisp)) { + JS_ASSERT(clone != funobj); + *vp = v; + setCalleeObject(*clone); + return true; + } + } + } + + /* + * If control flows here, we can't find an already-existing + * clone (or force to exist a fresh clone) created via thisp's + * method read barrier, so we must clone fun and store it in + * fp's callee to avoid re-cloning upon repeated foo.caller + * access. It seems that there are no longer any properties + * referring to fun. + */ + funobj = CloneFunctionObject(cx, fun, fun->getParent()); + if (!funobj) + return false; + funobj->setMethodObj(*thisp); + setCalleeObject(*funobj); + return true; + } + } + } + + return true; +} + /* Generic function tinyids. */ enum { FUN_ARGUMENTS = -1, /* predefined arguments local variable */ @@ -1285,7 +1381,7 @@ static JSBool fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) { if (!JSID_IS_INT(id)) - return JS_TRUE; + return true; jsint slot = JSID_TO_INT(id); @@ -1313,10 +1409,10 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) while (!(fun = (JSFunction *) GetInstancePrivate(cx, obj, &js_FunctionClass, NULL))) { if (slot != FUN_LENGTH) - return JS_TRUE; + return true; obj = obj->getProto(); if (!obj) - return JS_TRUE; + return true; } /* Find fun's top-most activation record. */ @@ -1335,11 +1431,11 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) js_GetErrorMessage, NULL, JSMSG_DEPRECATED_USAGE, js_arguments_str)) { - return JS_FALSE; + return false; } if (fp) { if (!js_GetArgsValue(cx, fp, vp)) - return JS_FALSE; + return false; } else { vp->setNull(); } @@ -1356,30 +1452,12 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) break; case FUN_CALLER: - if (fp && fp->down && fp->down->fun) { - JSFunction *caller = fp->down->fun; - /* - * See equivalent condition in args_getProperty for ARGS_CALLEE, - * but here we do not want to throw, since this escape can happen - * via foo.caller alone, without any debugger or indirect eval. And - * it seems foo.caller is still used on the Web. - */ - if (caller->needsWrapper()) { - JSObject *wrapper = WrapEscapingClosure(cx, fp->down, FUN_OBJECT(caller), caller); - if (!wrapper) - return JS_FALSE; - vp->setObject(*wrapper); - return JS_TRUE; - } + vp->setNull(); + if (fp && fp->down && !fp->down->getValidCalleeObject(cx, vp)) + return false; - JS_ASSERT(fp->down->argv); - *vp = fp->down->calleeValue(); - } else { - vp->setNull(); - } - - /* Censor the caller if it is from another compartment. */ if (vp->isObject()) { + /* Censor the caller if it is from another compartment. */ if (vp->toObject().getCompartment(cx) != cx->compartment) vp->setNull(); } @@ -1392,7 +1470,7 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) break; } - return JS_TRUE; + return true; } struct LazyFunctionProp { @@ -1824,7 +1902,8 @@ JSFunction::countInterpretedReservedSlots() const */ JS_PUBLIC_DATA(Class) js_FunctionClass = { js_Function_str, - JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_HAS_RESERVED_SLOTS(2) | + JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | + JSCLASS_HAS_RESERVED_SLOTS(JSObject::FUN_FIXED_RESERVED_SLOTS) | JSCLASS_MARK_IS_TRACE | JSCLASS_HAS_CACHED_PROTO(JSProto_Function), PropertyStub, PropertyStub, PropertyStub, PropertyStub, @@ -2149,8 +2228,7 @@ Function(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value *rval) * js_CheckContentSecurityPolicy is defined in jsobj.cpp */ if (!js_CheckContentSecurityPolicy(cx)) { - JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, - JSMSG_CSP_BLOCKED_FUNCTION); + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CSP_BLOCKED_FUNCTION); return JS_FALSE; } diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 5ca4f91f8010..e24b25651c20 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -45,6 +45,8 @@ #include "jsprvtd.h" #include "jspubtd.h" #include "jsobj.h" +#include "jsatom.h" +#include "jsstr.h" typedef struct JSLocalNameMap JSLocalNameMap; @@ -95,6 +97,10 @@ typedef union JSLocalNames { * can move to u.i.script->flags. For now we use function flag bits to minimize * pointer-chasing. */ +#define JSFUN_JOINABLE 0x0001 /* function is null closure that does not + appear to call itself via its own name + or arguments.callee */ + #define JSFUN_EXPR_CLOSURE 0x1000 /* expression closure: function(x) x*x */ #define JSFUN_TRCINFO 0x2000 /* when set, u.n.trcinfo is non-null, JSFunctionSpec::call points to a @@ -202,6 +208,44 @@ struct JSFunction : public JSObject bool mightEscape() const { return FUN_INTERPRETED(this) && (FUN_FLAT_CLOSURE(this) || u.i.nupvars == 0); } + + bool joinable() const { + return flags & JSFUN_JOINABLE; + } + + private: + /* + * js_FunctionClass reserves two slots, which are free in JSObject::fslots + * without requiring dslots allocation. Null closures that can be joined to + * a compiler-created function object use the first one to hold a mutable + * methodAtom() state variable, needed for correct foo.caller handling. + */ + enum { + METHOD_ATOM_SLOT = JSSLOT_FUN_METHOD_ATOM + }; + + public: + void setJoinable() { + JS_ASSERT(FUN_INTERPRETED(this)); + fslots[METHOD_ATOM_SLOT].setNull(); + flags |= JSFUN_JOINABLE; + } + + /* + * Method name imputed from property uniquely assigned to or initialized, + * where the function does not need to be cloned to carry a scope chain or + * flattened upvars. + */ + JSAtom *methodAtom() const { + return (joinable() && fslots[METHOD_ATOM_SLOT].isString()) + ? STRING_TO_ATOM(fslots[METHOD_ATOM_SLOT].toString()) + : NULL; + } + + void setMethodAtom(JSAtom *atom) { + JS_ASSERT(joinable()); + fslots[METHOD_ATOM_SLOT].setString(ATOM_TO_STRING(atom)); + } }; JS_STATIC_ASSERT(sizeof(JSFunction) % JS_GCTHING_ALIGN == 0); diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index fcc6325d178a..16b375204083 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2525,6 +2525,17 @@ js_TraceRuntime(JSTracer *trc) if (rt->gcExtraRootsTraceOp) rt->gcExtraRootsTraceOp(trc, rt->gcExtraRootsData); + +#ifdef JS_FUNCTION_METERING + for (int k = 0; k < 2; k++) { + typedef JSRuntime::FunctionCountMap HM; + HM &h = (k == 0) ? rt->methodReadBarrierCountMap : rt->unjoinedFunctionCountMap; + for (HM::Range r = h.all(); !r.empty(); r.popFront()) { + JSFunction *fun = r.front().key; + JS_CALL_OBJECT_TRACER(trc, fun, "FunctionCountMap key"); + } + } +#endif } void diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 2540e6662996..95c5e87ebbfe 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -2696,15 +2696,15 @@ BEGIN_CASE(JSOP_STOP) == JSOP_CALL_LENGTH); TRACE_0(LeaveFrame); if (!TRACE_RECORDER(cx) && recursive) { - if (*(regs.pc + JSOP_CALL_LENGTH) == JSOP_TRACE) { + if (regs.pc[JSOP_CALL_LENGTH] == JSOP_TRACE) { regs.pc += JSOP_CALL_LENGTH; MONITOR_BRANCH(Record_LeaveFrame); op = (JSOp)*regs.pc; DO_OP(); } } - if (*(regs.pc + JSOP_CALL_LENGTH) == JSOP_TRACE || - *(regs.pc + JSOP_CALL_LENGTH) == JSOP_NOP) { + if (regs.pc[JSOP_CALL_LENGTH] == JSOP_TRACE || + regs.pc[JSOP_CALL_LENGTH] == JSOP_NOP) { JS_STATIC_ASSERT(JSOP_TRACE_LENGTH == JSOP_NOP_LENGTH); regs.pc += JSOP_CALL_LENGTH; len = JSOP_TRACE_LENGTH; @@ -4064,7 +4064,8 @@ BEGIN_CASE(JSOP_GETXPROP) jsid id = ATOM_TO_JSID(atom); if (JS_LIKELY(aobj->map->ops->getProperty == js_GetProperty) ? !js_GetPropertyHelper(cx, obj, id, - fp->imacpc + (fp->imacpc || + regs.pc[JSOP_GETPROP_LENGTH + i] == JSOP_IFEQ) ? JSGET_CACHE_RESULT | JSGET_NO_METHOD_BARRIER : JSGET_CACHE_RESULT | JSGET_METHOD_BARRIER, &rval) @@ -5727,24 +5728,19 @@ BEGIN_CASE(JSOP_LAMBDA) parent = fp->scopeChain; if (obj->getParent() == parent) { - op = JSOp(regs.pc[JSOP_LAMBDA_LENGTH]); + jsbytecode *pc2 = regs.pc + JSOP_LAMBDA_LENGTH; + JSOp op2 = JSOp(*pc2); /* - * Optimize ({method: function () { ... }, ...}) and - * this.method = function () { ... }; bytecode sequences. + * Optimize var obj = {method: function () { ... }, ...}, + * this.method = function () { ... }; and other significant + * single-use-of-null-closure bytecode sequences. + * + * WARNING: code in TraceRecorder::record_JSOP_LAMBDA must + * match the optimization cases in the following code that + * break from the outer do-while(0). */ - if (op == JSOP_SETMETHOD) { -#ifdef DEBUG - JSOp op2 = JSOp(regs.pc[JSOP_LAMBDA_LENGTH + JSOP_SETMETHOD_LENGTH]); - JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV); -#endif - - const Value &lref = regs.sp[-1]; - if (lref.isObject() && - lref.toObject().getClass() == &js_ObjectClass) { - break; - } - } else if (op == JSOP_INITMETHOD) { + if (op2 == JSOP_INITMETHOD) { #ifdef DEBUG const Value &lref = regs.sp[-1]; JS_ASSERT(lref.isObject()); @@ -5752,9 +5748,78 @@ BEGIN_CASE(JSOP_LAMBDA) JS_ASSERT(obj2->getClass() == &js_ObjectClass); JS_ASSERT(obj2->scope()->object == obj2); #endif + + fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(JSOP_LAMBDA_LENGTH))); + JS_FUNCTION_METER(cx, joinedinitmethod); break; } + + if (op2 == JSOP_SETMETHOD) { +#ifdef DEBUG + op2 = JSOp(pc2[JSOP_SETMETHOD_LENGTH]); + JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV); +#endif + + const Value &lref = regs.sp[-1]; + if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) { + fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(JSOP_LAMBDA_LENGTH))); + JS_FUNCTION_METER(cx, joinedsetmethod); + break; + } + } else if (fun->joinable()) { + if (op2 == JSOP_CALL) { + /* + * Array.prototype.sort and String.prototype.replace are + * optimized as if they are special form. We know that they + * won't leak the joined function object in obj, therefore + * we don't need to clone that compiler- created function + * object for identity/mutation reasons. + */ + int iargc = GET_ARGC(pc2); + + /* + * Note that we have not yet pushed obj as the final argument, + * so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)], + * is the callee for this JSOP_CALL. + */ + JSFunction *calleeFun = + GET_FUNCTION_PRIVATE(cx, ®s.sp[1 - (iargc + 2)].toObject()); + FastNative fastNative = FUN_FAST_NATIVE(calleeFun); + + if (iargc == 1 && fastNative == array_sort) { + JS_FUNCTION_METER(cx, joinedsort); + break; + } + if (iargc == 2 && fastNative == str_replace) { + JS_FUNCTION_METER(cx, joinedreplace); + break; + } + } else if (op2 == JSOP_NULL) { + pc2 += JSOP_NULL_LENGTH; + op2 = JSOp(*pc2); + + if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) { + JS_FUNCTION_METER(cx, joinedmodulepat); + break; + } + } + } } + +#ifdef JS_FUNCTION_METERING + // No locking, this is mainly for js shell testing. + ++rt->functionMeter.unjoined; + + typedef JSRuntime::FunctionCountMap HM; + HM &h = rt->unjoinedFunctionCountMap; + HM::AddPtr p = h.lookupForAdd(fun); + if (!p) { + h.add(p, fun, 1); + } else { + JS_ASSERT(p->key == fun); + ++p->value; + } +#endif } else { parent = js_GetScopeChain(cx, fp); if (!parent) diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 7ed3f9fb1f15..9a2f287968e5 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -181,14 +181,32 @@ struct JSStackFrame return argv[-2]; } + /* Infallible getter to return the callee object from this frame. */ + JSObject &calleeObject() const { + JS_ASSERT(argv); + return argv[-2].toObject(); + } + + /* + * Fallible getter to compute the correct callee function object, which may + * require deferred cloning due to JSScope::methodReadBarrier. For a frame + * with null fun member, return true with *vp set from this->callee(). + */ + bool getValidCalleeObject(JSContext *cx, js::Value *vp); + + void setCalleeObject(JSObject &callable) { + JS_ASSERT(argv); + argv[-2].setObject(callable); + } + JSObject *callee() { return argv ? &argv[-2].toObject() : NULL; } /* - * Get the object associated with the Execution Context's - * VariableEnvironment (ES5 10.3). The given CallStackSegment must contain - * this stack frame. + * Get the "variable object" (ES3 term) associated with the Execution + * Context's VariableEnvironment (ES5 10.3). The given CallStackSegment + * must contain this stack frame. */ JSObject *varobj(js::CallStackSegment *css) const; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index ebaa45d51fbb..ff3962652e6f 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -5141,13 +5141,13 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, * Check for Object class here to avoid defining a method on a class * with magic resolve, addProperty, getProperty, etc. hooks. */ - if ((defineHow & JSDNP_SET_METHOD) && - obj->getClass() == &js_ObjectClass) { + if ((defineHow & JSDNP_SET_METHOD) && obj->canHaveMethodBarrier()) { JS_ASSERT(IsFunctionObject(*vp)); JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER))); JSObject *funobj = &vp->toObject(); - if (FUN_OBJECT(GET_FUNCTION_PRIVATE(cx, funobj)) == funobj) { + JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); + if (fun == funobj) { flags |= JSScopeProperty::METHOD; getter = CastAsPropertyOp(funobj); } @@ -5298,8 +5298,39 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval) } scope = obj->scope(); - if (SPROP_HAS_VALID_SLOT(sprop, scope)) - GC_POKE(cx, obj->lockedGetSlot(sprop->slot)); + if (SPROP_HAS_VALID_SLOT(sprop, scope)) { + const Value &v = obj->lockedGetSlot(sprop->slot); + GC_POKE(cx, v); + + /* + * Delete is rare enough that we can take the hit of checking for an + * active cloned method function object that must be homed to a callee + * slot on the active stack frame before this delete completes, in case + * someone saved the clone and checks it against foo.caller for a foo + * called from the active method. + * + * We do not check suspended frames. They can't be reached via caller, + * so the only way they could have the method's joined function object + * as callee is through an API abusage. We break any such edge case. + */ + if (scope->hasMethodBarrier()) { + JSObject *funobj; + + if (IsFunctionObject(v, &funobj)) { + JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); + + if (fun != funobj) { + for (JSStackFrame *fp = cx->fp; fp; fp = fp->down) { + if (fp->callee() == fun && + fp->thisv.isObject() && + &fp->thisv.toObject() == obj) { + fp->setCalleeObject(*funobj); + } + } + } + } + } + } ok = scope->removeProperty(cx, id); JS_UNLOCK_OBJ(cx, obj); @@ -6223,7 +6254,7 @@ dumpValue(const Value &v) Class *clasp = obj->getClass(); fprintf(stderr, "<%s%s at %p>", clasp->name, - clasp == &js_ObjectClass ? "" : " object", + (clasp == &js_ObjectClass) ? "" : " object", (void *) obj); } else if (v.isBoolean()) { if (v.toBoolean()) diff --git a/js/src/jsobj.h b/js/src/jsobj.h index e82557b2337e..97bfc493b199 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -252,6 +252,8 @@ const uint32 JSSLOT_PARENT = 0; */ const uint32 JSSLOT_PRIVATE = 1; +class JSFunction; + /* * JSObject struct, with members sized to fit in 32 bytes on 32-bit targets, * 64 bytes on 64-bit systems. The JSFunction struct is an extension of this @@ -293,7 +295,7 @@ struct JSObject { js::Value *dslots; /* dynamically allocated slots */ #if JS_BITS_PER_WORD == 32 // TODO: this is needed to pad out fslots. alternatively, clasp could be - // merged by with flags and the padding removed, but I think the upcoming + // merged with flags and the padding removed, but I think the upcoming // removal of JSScope will change this all anyway so I will leave this // here for now. uint32 padding; @@ -559,6 +561,22 @@ struct JSObject { inline const js::Value &getDateUTCTime() const; inline void setDateUTCTime(const js::Value &pthis); + /* + * Function-specific getters and setters. + */ + + private: + friend class JSFunction; + + static const uint32 JSSLOT_FUN_METHOD_ATOM = JSSLOT_PRIVATE + 1; + static const uint32 JSSLOT_FUN_METHOD_OBJ = JSSLOT_PRIVATE + 2; + + public: + static const uint32 FUN_FIXED_RESERVED_SLOTS = 2; + + inline bool hasMethodObj(const JSObject& obj) const; + inline void setMethodObj(JSObject& obj); + /* * RegExp-specific getters and setters. */ @@ -722,6 +740,8 @@ struct JSObject { void swap(JSObject *obj); + inline bool canHaveMethodBarrier() const; + inline bool isArguments() const; inline bool isArray() const; inline bool isDenseArray() const; @@ -732,6 +752,9 @@ struct JSObject { inline bool isPrimitive() const; inline bool isDate() const; inline bool isFunction() const; + inline bool isObject() const; + inline bool isWith() const; + inline bool isBlock() const; inline bool isRegExp() const; inline bool isXML() const; inline bool isNamespace() const; @@ -831,6 +854,10 @@ extern js::Class js_ObjectClass; extern js::Class js_WithClass; extern js::Class js_BlockClass; +inline bool JSObject::isObject() const { return getClass() == &js_ObjectClass; } +inline bool JSObject::isWith() const { return getClass() == &js_WithClass; } +inline bool JSObject::isBlock() const { return getClass() == &js_BlockClass; } + /* * Block scope object macros. The slots reserved by js_BlockClass are: * diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index f94df13c57df..7d4ff4263602 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -106,6 +106,12 @@ JSObject::getReservedSlot(uintN index) const return (slot < numSlots()) ? getSlot(slot) : js::UndefinedValue(); } +inline bool +JSObject::canHaveMethodBarrier() const +{ + return isObject() || isFunction() || isPrimitive() || isDate(); +} + inline bool JSObject::isPrimitive() const { @@ -307,6 +313,19 @@ JSObject::setDateUTCTime(const js::Value &time) fslots[JSSLOT_DATE_UTC_TIME] = time; } +inline bool +JSObject::hasMethodObj(const JSObject& obj) const +{ + return fslots[JSSLOT_FUN_METHOD_OBJ].isObject() && + &fslots[JSSLOT_FUN_METHOD_OBJ].toObject() == &obj; +} + +inline void +JSObject::setMethodObj(JSObject& obj) +{ + fslots[JSSLOT_FUN_METHOD_OBJ].setObject(obj); +} + inline const js::Value & JSObject::getRegExpLastIndex() const { diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index eb430d3bbabe..af60f81d779e 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -5499,10 +5499,6 @@ SimulateImacroCFG(JSContext *cx, JSScript *script, #undef LOCAL_ASSERT #define LOCAL_ASSERT(expr) LOCAL_ASSERT_RV(expr, -1); -static intN -ReconstructPCStack(JSContext *cx, JSScript *script, jsbytecode *target, - jsbytecode **pcstack); - static intN ReconstructImacroPCStack(JSContext *cx, JSScript *script, jsbytecode *imacstart, jsbytecode *target, diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index bafa70f72183..f255950d8cd2 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -2130,11 +2130,7 @@ DeoptimizeUsesWithin(JSDefinition *dn, JSFunctionBox *funbox, uint32& tcflags) void Parser::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags) { -#ifdef JS_FUNCTION_METERING -# define FUN_METER(x) JS_RUNTIME_METER(context->runtime, functionMeter.x) -#else -# define FUN_METER(x) ((void)0) -#endif +#define FUN_METER(x) JS_FUNCTION_METER(context, x) for (;;) { JSParseNode *fn = funbox->node; @@ -2344,6 +2340,9 @@ Parser::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags) } } + if (funbox->joinable()) + fun->setJoinable(); + funbox = funbox->siblings; if (!funbox) break; diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 39c9c06e3436..433b10c4afa0 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -1214,7 +1214,7 @@ JSScope::methodShapeChange(JSContext *cx, JSScopeProperty *sprop) const Value &prev = object->lockedGetSlot(sprop->slot); JS_ASSERT(&sprop->methodObject() == &prev.toObject()); JS_ASSERT(hasMethodBarrier()); - JS_ASSERT(object->getClass() == &js_ObjectClass); + JS_ASSERT(object->canHaveMethodBarrier()); JS_ASSERT(!sprop->rawSetter || sprop->rawSetter == js_watch_set); #endif diff --git a/js/src/jsscope.h b/js/src/jsscope.h index a0efd4b7857f..bc3e72889f0c 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -612,7 +612,9 @@ struct JSScopeProperty { union { js::PropertyOp rawGetter; /* getter and setter hooks or objects */ JSObject *getterObj; /* user-defined callable "get" object or - null if sprop->hasGetterValue() */ + null if sprop->hasGetterValue(); or + joined function object if METHOD flag + is set. */ JSScopeProperty *next; /* next node in freelist */ }; diff --git a/js/src/jsscopeinlines.h b/js/src/jsscopeinlines.h index 6249473c182a..8182a40b0c19 100644 --- a/js/src/jsscopeinlines.h +++ b/js/src/jsscopeinlines.h @@ -122,17 +122,35 @@ JSScope::methodReadBarrier(JSContext *cx, JSScopeProperty *sprop, js::Value *vp) JS_ASSERT(hasProperty(sprop)); JS_ASSERT(sprop->isMethod()); JS_ASSERT(&vp->toObject() == &sprop->methodObject()); - JS_ASSERT(object->getClass() == &js_ObjectClass); + JS_ASSERT(object->canHaveMethodBarrier()); JSObject *funobj = &vp->toObject(); JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); - JS_ASSERT(FUN_OBJECT(fun) == funobj && FUN_NULL_CLOSURE(fun)); + JS_ASSERT(fun == funobj && FUN_NULL_CLOSURE(fun)); funobj = CloneFunctionObject(cx, fun, funobj->getParent()); if (!funobj) return false; + funobj->setMethodObj(*object); + vp->setObject(*funobj); - return !!js_SetPropertyHelper(cx, object, sprop->id, 0, vp); + if (!js_SetPropertyHelper(cx, object, sprop->id, 0, vp)) + return false; + +#ifdef JS_FUNCTION_METERING + JS_FUNCTION_METER(cx, mreadbarrier); + + typedef JSRuntime::FunctionCountMap HM; + HM &h = cx->runtime->methodReadBarrierCountMap; + HM::AddPtr p = h.lookupForAdd(fun); + if (!p) { + h.add(p, fun, 1); + } else { + JS_ASSERT(p->key == fun); + ++p->value; + } +#endif + return true; } static JS_ALWAYS_INLINE bool @@ -149,8 +167,10 @@ JSScope::methodWriteBarrier(JSContext *cx, JSScopeProperty *sprop, { if (flags & (BRANDED | METHOD_BARRIER)) { const js::Value &prev = object->lockedGetSlot(sprop->slot); - if (ChangesMethodValue(prev, v)) + if (ChangesMethodValue(prev, v)) { + JS_FUNCTION_METER(cx, mwritebarrier); return methodShapeChange(cx, sprop); + } } return true; } @@ -160,8 +180,10 @@ JSScope::methodWriteBarrier(JSContext *cx, uint32 slot, const js::Value &v) { if (flags & (BRANDED | METHOD_BARRIER)) { const js::Value &prev = object->lockedGetSlot(slot); - if (ChangesMethodValue(prev, v)) + if (ChangesMethodValue(prev, v)) { + JS_FUNCTION_METER(cx, mwslotbarrier); return methodShapeChange(cx, slot); + } } return true; } diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index 8d37aa94f31f..fc2d184af179 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -2301,8 +2301,8 @@ BuildFlatReplacement(JSContext *cx, JSString *textstr, JSString *repstr, return true; } -static JSBool -str_replace(JSContext *cx, uintN argc, Value *vp) +JSBool +js::str_replace(JSContext *cx, uintN argc, Value *vp) { ReplaceData rdata(cx); NORMALIZE_THIS(cx, vp, rdata.str); diff --git a/js/src/jsstr.h b/js/src/jsstr.h index 254b98dda926..a316e4be6d05 100644 --- a/js/src/jsstr.h +++ b/js/src/jsstr.h @@ -995,6 +995,15 @@ extern JSBool js_str_escape(JSContext *cx, JSObject *obj, uintN argc, js::Value *argv, js::Value *rval); +/* + * The String.prototype.replace fast-native entry point is exported for joined + * function optimization in js{interp,tracer}.cpp. + */ +namespace js { +extern JSBool +str_replace(JSContext *cx, uintN argc, js::Value *vp); +} + extern JSBool js_str_toString(JSContext *cx, uintN argc, js::Value *vp); diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 3ea3e661ee54..ccb5f2525266 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -14816,19 +14816,56 @@ TraceRecorder::record_JSOP_LAMBDA() if (FUN_NULL_CLOSURE(fun)) { if (FUN_OBJECT(fun)->getParent() != globalObj) RETURN_STOP_A("Null closure function object parent must be global object"); - JSOp op2 = JSOp(cx->regs->pc[JSOP_LAMBDA_LENGTH]); + + jsbytecode *pc2 = cx->regs->pc + JSOP_LAMBDA_LENGTH; + JSOp op2 = JSOp(*pc2); + + if (op2 == JSOP_INITMETHOD) { + stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); + return ARECORD_CONTINUE; + } if (op2 == JSOP_SETMETHOD) { Value lval = stackval(-1); - if (!lval.isPrimitive() && - lval.toObject().getClass() == &js_ObjectClass) { + if (!lval.isPrimitive() && lval.toObject().canHaveMethodBarrier()) { stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); return ARECORD_CONTINUE; } - } else if (op2 == JSOP_INITMETHOD) { - stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); - return ARECORD_CONTINUE; + } else if (fun->joinable()) { + if (op2 == JSOP_CALL) { + /* + * Array.prototype.sort and String.prototype.replace are + * optimized as if they are special form. We know that they + * won't leak the joined function object in obj, therefore + * we don't need to clone that compiler- created function + * object for identity/mutation reasons. + */ + int iargc = GET_ARGC(pc2); + + /* + * Note that we have not yet pushed obj as the final argument, + * so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)], + * is the callee for this JSOP_CALL. + */ + JSFunction *calleeFun = + GET_FUNCTION_PRIVATE(cx, &cx->regs->sp[1 - (iargc + 2)].toObject()); + FastNative fastNative = FUN_FAST_NATIVE(calleeFun); + + if ((iargc == 1 && fastNative == array_sort) || + (iargc == 2 && fastNative == str_replace)) { + stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); + return ARECORD_CONTINUE; + } + } else if (op2 == JSOP_NULL) { + pc2 += JSOP_NULL_LENGTH; + op2 = JSOp(*pc2); + + if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) { + stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); + return ARECORD_CONTINUE; + } + } } LIns *proto_ins; diff --git a/js/src/tests/js1_8_5/regress/jstests.list b/js/src/tests/js1_8_5/regress/jstests.list index 4c12e6a4adf2..3ecde0d61546 100644 --- a/js/src/tests/js1_8_5/regress/jstests.list +++ b/js/src/tests/js1_8_5/regress/jstests.list @@ -24,3 +24,5 @@ script regress-566914.js script regress-567152.js script regress-569306.js script regress-571014.js +script regress-577648-1.js +script regress-577648-2.js diff --git a/js/src/tests/js1_8_5/regress/regress-577648-1.js b/js/src/tests/js1_8_5/regress/regress-577648-1.js new file mode 100644 index 000000000000..d40c7b0e4237 --- /dev/null +++ b/js/src/tests/js1_8_5/regress/regress-577648-1.js @@ -0,0 +1,87 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var count = 0; + +function testCaller(obj) { + switch (++count) { + case 1: + case 2: + /* + * The first two times, obj is objA. The first time, we reference + * arguments.callee.caller before obj.go, so the caller getter must + * force the joined function object in the stack frame to cross the + * method read barrier. The second time, obj.go has been cloned and + * it should match the new frame's callee from the get-go. + */ + assertEq(obj, objA); + break; + + case 3: { + assertEq(obj, objB); + + /* + * Store another clone of the joined function object before obj.go has + * been read, but after it has been invoked via objB.go(objB). + * + * In this case, arguments.callee.caller must not lie and return what + * is currently stored in objB.go, since that function object (objA.go) + * was cloned earlier, when count was 1, and it is not the function + * object that was truly invoked. + * + * But since the invocation of objB.go(objB) did not clone go, and the + * following assignment overwrote the invoked value, leaving the only + * reference to the joined function object for go in the stack frame's + * callee (argv[-2]) member, the arguments.callee.caller reference must + * clone a function object for the callee, store it as the callee, and + * return it here. + * + * It won't equal obj.go, but (implementation detail) it should have + * the same proto as obj.go + */ + obj.go = objA.go; + + let caller = arguments.callee.caller; + let obj_go = obj.go; + return caller != obj_go && caller.__proto__ == obj_go.__proto__; + } + + case 4: { + assertEq(obj, objC); + + let save = obj.go; + delete obj.go; + return arguments.callee.caller == save; + } + + case 5: { + assertEq(obj, objD); + + let read = obj.go; + break; + } + } + + return arguments.callee.caller == obj.go; +} + +function make() { + return { + go: function(obj) { + return testCaller(obj); + } + }; +} + +var objA = make(), + objB = make(), + objC = make(), + objD = make(); + +reportCompare(true, objA.go(objA), "1"); +reportCompare(true, objA.go(objA), "2"); +reportCompare(true, objB.go(objB), "3"); +reportCompare(true, objC.go(objC), "4"); +reportCompare(true, objD.go(objD), "5"); diff --git a/js/src/tests/js1_8_5/regress/regress-577648-2.js b/js/src/tests/js1_8_5/regress/regress-577648-2.js new file mode 100644 index 000000000000..d9ba9131630e --- /dev/null +++ b/js/src/tests/js1_8_5/regress/regress-577648-2.js @@ -0,0 +1,12 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + * Contributor: Jason Orendorff + */ + +var o = { f: function() { return o.g(); }, g: function() { return arguments.callee.caller; } }; +var c = o.f(); +var i = 'f'; +var d = o[i](); + +reportCompare(true, c === o.f && d === o.f(), ""); From 8fdb12a38b7a11255bcab3c08fc71bd606e3290f Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Fri, 23 Jul 2010 14:21:50 -0700 Subject: [PATCH 07/19] Back out patch for bug 577648, need to fix a leak. --- js/src/jsapi.cpp | 9 +- js/src/jsarray.cpp | 4 +- js/src/jsarray.h | 9 -- js/src/jscntxt.cpp | 104 ++++++------- js/src/jscntxt.h | 23 +-- js/src/jsdbgapi.cpp | 11 -- js/src/jsdbgapi.h | 33 +---- js/src/jsemit.cpp | 3 +- js/src/jsfun.cpp | 140 ++++-------------- js/src/jsfun.h | 44 ------ js/src/jsgc.cpp | 11 -- js/src/jsinterp.cpp | 103 +++---------- js/src/jsinterp.h | 24 +-- js/src/jsobj.cpp | 43 +----- js/src/jsobj.h | 29 +--- js/src/jsobjinlines.h | 19 --- js/src/jsopcode.cpp | 4 + js/src/jsparse.cpp | 9 +- js/src/jsscope.cpp | 2 +- js/src/jsscope.h | 4 +- js/src/jsscopeinlines.h | 32 +--- js/src/jsstr.cpp | 4 +- js/src/jsstr.h | 9 -- js/src/jstracer.cpp | 49 +----- js/src/tests/js1_8_5/regress/jstests.list | 2 - .../tests/js1_8_5/regress/regress-577648-1.js | 87 ----------- .../tests/js1_8_5/regress/regress-577648-2.js | 12 -- 27 files changed, 138 insertions(+), 685 deletions(-) delete mode 100644 js/src/tests/js1_8_5/regress/regress-577648-1.js delete mode 100644 js/src/tests/js1_8_5/regress/regress-577648-2.js diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index a42968c9599c..9d101e496b2b 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -546,7 +546,7 @@ static JSBool js_NewRuntimeWasCalled = JS_FALSE; #endif JSRuntime::JSRuntime() - : gcChunkAllocator(&defaultGCChunkAllocator) + : gcChunkAllocator(&defaultGCChunkAllocator) { /* Initialize infallibly first, so we can goto bad and JS_DestroyRuntime. */ JS_INIT_CLIST(&contextList); @@ -557,13 +557,6 @@ JSRuntime::JSRuntime() bool JSRuntime::init(uint32 maxbytes) { -#ifdef JS_FUNCTION_METERING - if (!methodReadBarrierCountMap.init()) - return false; - if (!unjoinedFunctionCountMap.init()) - return false; -#endif - if (!(defaultCompartment = new JSCompartment(this)) || !defaultCompartment->init()) { return false; diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 967e527ebffd..c30c2c629cad 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -1774,8 +1774,8 @@ sort_compare_strings(void *arg, const void *a, const void *b, int *result) return JS_TRUE; } -JSBool -js::array_sort(JSContext *cx, uintN argc, Value *vp) +static JSBool +array_sort(JSContext *cx, uintN argc, Value *vp) { jsuint len, newlen, i, undefs; size_t elemsize; diff --git a/js/src/jsarray.h b/js/src/jsarray.h index 7839f407731a..faaa236874da 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -200,15 +200,6 @@ extern bool js_MergeSort(void *vec, size_t nel, size_t elsize, JSComparator cmp, void *arg, void *tmp, JSMergeSortElemType elemType); -/* - * The Array.prototype.sort fast-native entry point is exported for joined - * function optimization in js{interp,tracer}.cpp. - */ -namespace js { -extern JSBool -array_sort(JSContext *cx, uintN argc, js::Value *vp); -} - #ifdef DEBUG_ARRAYS extern JSBool js_ArrayInfo(JSContext *cx, JSObject *obj, uintN argc, js::Value *argv, js::Value *rval); diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 5a2a4515cdaa..505464736a80 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -959,83 +959,69 @@ private: static void DumpEvalCacheMeter(JSContext *cx) { - if (const char *filename = getenv("JS_EVALCACHE_STATFILE")) { - struct { - const char *name; - ptrdiff_t offset; - } table[] = { + struct { + const char *name; + ptrdiff_t offset; + } table[] = { #define frob(x) { #x, offsetof(JSEvalCacheMeter, x) } - EVAL_CACHE_METER_LIST(frob) + EVAL_CACHE_METER_LIST(frob) #undef frob - }; - JSEvalCacheMeter *ecm = &JS_THREAD_DATA(cx)->evalCacheMeter; + }; + JSEvalCacheMeter *ecm = &JS_THREAD_DATA(cx)->evalCacheMeter; - static JSAutoFile fp; - if (!fp && !fp.open(filename, "w")) + static JSAutoFile fp; + if (!fp) { + fp.open("/tmp/evalcache.stats", "w"); + if (!fp) return; - - fprintf(fp, "eval cache meter (%p):\n", -#ifdef JS_THREADSAFE - (void *) cx->thread -#else - (void *) cx->runtime -#endif - ); - for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { - fprintf(fp, "%-8.8s %llu\n", - table[i].name, - (unsigned long long int) *(uint64 *)((uint8 *)ecm + table[i].offset)); - } - fprintf(fp, "hit ratio %g%%\n", ecm->hit * 100. / ecm->probe); - fprintf(fp, "avg steps %g\n", double(ecm->step) / ecm->probe); - fflush(fp); } + + fprintf(fp, "eval cache meter (%p):\n", +#ifdef JS_THREADSAFE + (void *) cx->thread +#else + (void *) cx->runtime +#endif + ); + for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { + fprintf(fp, "%-8.8s %llu\n", + table[i].name, + (unsigned long long int) *(uint64 *)((uint8 *)ecm + table[i].offset)); + } + fprintf(fp, "hit ratio %g%%\n", ecm->hit * 100. / ecm->probe); + fprintf(fp, "avg steps %g\n", double(ecm->step) / ecm->probe); + fflush(fp); } # define DUMP_EVAL_CACHE_METER(cx) DumpEvalCacheMeter(cx) #endif #ifdef JS_FUNCTION_METERING -static void -DumpFunctionCountMap(const char *title, JSRuntime::FunctionCountMap &map, FILE *fp) -{ - fprintf(fp, "\n%s count map:\n", title); - - for (JSRuntime::FunctionCountMap::Range r = map.all(); !r.empty(); r.popFront()) { - JSFunction *fun = r.front().key; - int32 count = r.front().value; - - fprintf(fp, "%10d %s:%u\n", count, fun->u.i.script->filename, fun->u.i.script->lineno); - } -} - static void DumpFunctionMeter(JSContext *cx) { - if (const char *filename = getenv("JS_FUNCTION_STATFILE")) { - struct { - const char *name; - ptrdiff_t offset; - } table[] = { + struct { + const char *name; + ptrdiff_t offset; + } table[] = { #define frob(x) { #x, offsetof(JSFunctionMeter, x) } - FUNCTION_KIND_METER_LIST(frob) + FUNCTION_KIND_METER_LIST(frob) #undef frob - }; - JSFunctionMeter *fm = &cx->runtime->functionMeter; + }; + JSFunctionMeter *fm = &cx->runtime->functionMeter; - static JSAutoFile fp; - if (!fp && !fp.open(filename, "w")) + static JSAutoFile fp; + if (!fp) { + fp.open("/tmp/function.stats", "a"); + if (!fp) return; - - fprintf(fp, "function meter (%s):\n", cx->runtime->lastScriptFilename); - for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) - fprintf(fp, "%-19.19s %d\n", table[i].name, *(int32 *)((uint8 *)fm + table[i].offset)); - - DumpFunctionCountMap("method read barrier", cx->runtime->methodReadBarrierCountMap, fp); - DumpFunctionCountMap("unjoined function", cx->runtime->unjoinedFunctionCountMap, fp); - - putc('\n', fp); - fflush(fp); } + + fprintf(fp, "function meter (%s):\n", cx->runtime->lastScriptFilename); + for (uintN i = 0; i < JS_ARRAY_LENGTH(table); ++i) { + fprintf(fp, "%-11.11s %d\n", + table[i].name, *(int32 *)((uint8 *)fm + table[i].offset)); + } + fflush(fp); } # define DUMP_FUNCTION_METER(cx) DumpFunctionMeter(cx) #endif diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index bed92a71ebee..914b44f860a1 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -920,7 +920,7 @@ struct TraceMonitor { # define JS_ON_TRACE(cx) JS_FALSE #endif -#ifdef DEBUG +#ifdef DEBUG_brendan # define JS_EVAL_CACHE_METERING 1 # define JS_FUNCTION_METERING 1 #endif @@ -945,11 +945,7 @@ struct JSEvalCacheMeter { #ifdef JS_FUNCTION_METERING # define FUNCTION_KIND_METER_LIST(_) \ _(allfun), _(heavy), _(nofreeupvar), _(onlyfreevar), \ - _(display), _(flat), _(setupvar), _(badfunarg), \ - _(joinedsetmethod), _(joinedinitmethod), \ - _(joinedreplace), _(joinedsort), _(joinedmodulepat), \ - _(mreadbarrier), _(mwritebarrier), _(mwslotbarrier), \ - _(unjoined) + _(display), _(flat), _(setupvar), _(badfunarg) # define identity(x) x struct JSFunctionMeter { @@ -957,13 +953,8 @@ struct JSFunctionMeter { }; # undef identity - -# define JS_FUNCTION_METER(cx,x) JS_RUNTIME_METER((cx)->runtime, functionMeter.x) -#else -# define JS_FUNCTION_METER(cx,x) ((void)0) #endif - #define NATIVE_ITER_CACHE_LOG2 8 #define NATIVE_ITER_CACHE_MASK JS_BITMASK(NATIVE_ITER_CACHE_LOG2) #define NATIVE_ITER_CACHE_SIZE JS_BIT(NATIVE_ITER_CACHE_LOG2) @@ -1584,14 +1575,6 @@ struct JSRuntime { #ifdef JS_FUNCTION_METERING JSFunctionMeter functionMeter; char lastScriptFilename[1024]; - - typedef js::HashMap, - js::SystemAllocPolicy> FunctionCountMap; - - FunctionCountMap methodReadBarrierCountMap; - FunctionCountMap unjoinedFunctionCountMap; #endif JSWrapObjectCallback wrapObjectCallback; @@ -2153,7 +2136,7 @@ JS_ALWAYS_INLINE jsbytecode * JSStackFrame::pc(JSContext *cx) const { JS_ASSERT(cx->containingSegment(this) != NULL); - return (cx->fp == this) ? cx->regs->pc : savedPC; + return cx->fp == this ? cx->regs->pc : savedPC; } /* diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index c18bb1019adb..6540f5f8750d 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -1270,17 +1270,6 @@ JS_GetFrameCalleeObject(JSContext *cx, JSStackFrame *fp) return fp->callee(); } -JS_PUBLIC_API(JSBool) -JS_GetValidFrameCalleeObject(JSContext *cx, JSStackFrame *fp, jsval *vp) -{ - Value v; - - if (!fp->getValidCalleeObject(cx, &v)) - return false; - *vp = Jsvalify(v); - return true; -} - JS_PUBLIC_API(JSBool) JS_IsDebuggerFrame(JSContext *cx, JSStackFrame *fp) { diff --git a/js/src/jsdbgapi.h b/js/src/jsdbgapi.h index c3d45460f8e2..03420b4cfa94 100644 --- a/js/src/jsdbgapi.h +++ b/js/src/jsdbgapi.h @@ -272,42 +272,11 @@ extern JS_PUBLIC_API(void) JS_SetFrameReturnValue(JSContext *cx, JSStackFrame *fp, jsval rval); /** - * Return fp's callee function object (fp->callee) if it has one. Note that - * this API cannot fail. A null return means "no callee": fp is a global or - * eval-from-global frame, not a call frame. - * - * This API began life as an infallible getter, but now it can return either: - * - * 1. An optimized closure that was compiled assuming the function could not - * escape and be called from sites the compiler could not see. - * - * 2. A "joined function object", an optimization whereby SpiderMonkey avoids - * creating fresh function objects for every evaluation of a function - * expression that is used only once by a consumer that either promises to - * clone later when asked for the value or that cannot leak the value. - * - * Because Mozilla's Gecko embedding of SpiderMonkey (and no doubt other - * embeddings) calls this API in potentially performance-sensitive ways (e.g. - * in nsContentUtils::GetDocumentFromCaller), we are leaving this API alone. It - * may now return an unwrapped non-escaping optimized closure, or a joined - * function object. Such optimized objects may work well if called from the - * correct context, never mutated or compared for identity, etc. - * - * However, if you really need to get the same callee object that JS code would - * see, which means undoing the optimizations, where an undo attempt can fail, - * then use JS_GetValidFrameCalleeObject. + * Return fp's callee function object (fp->callee) if it has one. */ extern JS_PUBLIC_API(JSObject *) JS_GetFrameCalleeObject(JSContext *cx, JSStackFrame *fp); -/** - * Return fp's callee function object after running the deferred closure - * cloning "method read barrier". This API can fail! If the frame has no - * callee, this API returns true with JSVAL_IS_NULL(*vp). - */ -extern JS_PUBLIC_API(JSBool) -JS_GetValidFrameCalleeObject(JSContext *cx, JSStackFrame *fp, jsval *vp); - /************************************************************************/ extern JS_PUBLIC_API(const char *) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 4728b565abdc..968d0cf03709 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -6634,7 +6634,8 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) bool lambda = PN_OP(init) == JSOP_LAMBDA; if (lambda) ++methodInits; - if (op == JSOP_INITPROP && lambda && init->pn_funbox->joinable()) { + if (op == JSOP_INITPROP && lambda && init->pn_funbox->joinable()) + { op = JSOP_INITMETHOD; pn2->pn_op = uint8(op); } else { diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 5cd4c8c34a1b..3cc1a4b8a791 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1272,102 +1272,6 @@ JS_PUBLIC_DATA(Class) js_CallClass = { JS_CLASS_TRACE(args_or_call_trace), NULL }; -bool -JSStackFrame::getValidCalleeObject(JSContext *cx, Value *vp) -{ - if (!fun) { - vp->setObjectOrNull(callee()); - return true; - } - - /* - * See the equivalent condition in args_getProperty for ARGS_CALLEE, but - * note that here we do not want to throw, since this escape can happen via - * a foo.caller reference alone, without any debugger or indirect eval. And - * alas, it seems foo.caller is still used on the Web. - */ - if (fun->needsWrapper()) { - JSObject *wrapper = WrapEscapingClosure(cx, this, fun, fun); - if (!wrapper) - return false; - vp->setObject(*wrapper); - return true; - } - - JSObject *funobj = &calleeObject(); - vp->setObject(*funobj); - - /* - * Check for an escape attempt by a joined function object, which must go - * through the frame's |this| object's method read barrier for the method - * atom by which it was uniquely associated with a property. - */ - if (thisv.isObject()) { - JS_ASSERT(GET_FUNCTION_PRIVATE(cx, funobj) == fun); - - if (fun == funobj && fun->methodAtom()) { - JSObject *thisp = &thisv.toObject(); - JS_ASSERT(thisp->canHaveMethodBarrier()); - - JSScope *scope = thisp->scope(); - if (scope->hasMethodBarrier()) { - JSScopeProperty *sprop = scope->lookup(ATOM_TO_JSID(fun->methodAtom())); - - /* - * The method property might have been deleted while the method - * barrier scope flag stuck, so we must lookup and test here. - * - * Two cases follow: the method barrier was not crossed yet, so - * we cross it here; the method barrier *was* crossed, in which - * case we must fetch and validate the cloned (unjoined) funobj - * in the method property's slot. - * - * In either case we must allow for the method property to have - * been replaced, or its value to have been overwritten. - */ - if (sprop) { - if (sprop->isMethod() && &sprop->methodObject() == funobj) { - if (!scope->methodReadBarrier(cx, sprop, vp)) - return false; - setCalleeObject(vp->toObject()); - return true; - } - if (sprop->hasSlot()) { - Value v = thisp->getSlot(sprop->slot); - JSObject *clone; - - if (IsFunctionObject(v, &clone) && - GET_FUNCTION_PRIVATE(cx, clone) == fun && - clone->hasMethodObj(*thisp)) { - JS_ASSERT(clone != funobj); - *vp = v; - setCalleeObject(*clone); - return true; - } - } - } - - /* - * If control flows here, we can't find an already-existing - * clone (or force to exist a fresh clone) created via thisp's - * method read barrier, so we must clone fun and store it in - * fp's callee to avoid re-cloning upon repeated foo.caller - * access. It seems that there are no longer any properties - * referring to fun. - */ - funobj = CloneFunctionObject(cx, fun, fun->getParent()); - if (!funobj) - return false; - funobj->setMethodObj(*thisp); - setCalleeObject(*funobj); - return true; - } - } - } - - return true; -} - /* Generic function tinyids. */ enum { FUN_ARGUMENTS = -1, /* predefined arguments local variable */ @@ -1381,7 +1285,7 @@ static JSBool fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) { if (!JSID_IS_INT(id)) - return true; + return JS_TRUE; jsint slot = JSID_TO_INT(id); @@ -1409,10 +1313,10 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) while (!(fun = (JSFunction *) GetInstancePrivate(cx, obj, &js_FunctionClass, NULL))) { if (slot != FUN_LENGTH) - return true; + return JS_TRUE; obj = obj->getProto(); if (!obj) - return true; + return JS_TRUE; } /* Find fun's top-most activation record. */ @@ -1431,11 +1335,11 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) js_GetErrorMessage, NULL, JSMSG_DEPRECATED_USAGE, js_arguments_str)) { - return false; + return JS_FALSE; } if (fp) { if (!js_GetArgsValue(cx, fp, vp)) - return false; + return JS_FALSE; } else { vp->setNull(); } @@ -1452,12 +1356,30 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) break; case FUN_CALLER: - vp->setNull(); - if (fp && fp->down && !fp->down->getValidCalleeObject(cx, vp)) - return false; + if (fp && fp->down && fp->down->fun) { + JSFunction *caller = fp->down->fun; + /* + * See equivalent condition in args_getProperty for ARGS_CALLEE, + * but here we do not want to throw, since this escape can happen + * via foo.caller alone, without any debugger or indirect eval. And + * it seems foo.caller is still used on the Web. + */ + if (caller->needsWrapper()) { + JSObject *wrapper = WrapEscapingClosure(cx, fp->down, FUN_OBJECT(caller), caller); + if (!wrapper) + return JS_FALSE; + vp->setObject(*wrapper); + return JS_TRUE; + } + JS_ASSERT(fp->down->argv); + *vp = fp->down->calleeValue(); + } else { + vp->setNull(); + } + + /* Censor the caller if it is from another compartment. */ if (vp->isObject()) { - /* Censor the caller if it is from another compartment. */ if (vp->toObject().getCompartment(cx) != cx->compartment) vp->setNull(); } @@ -1470,7 +1392,7 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp) break; } - return true; + return JS_TRUE; } struct LazyFunctionProp { @@ -1902,8 +1824,7 @@ JSFunction::countInterpretedReservedSlots() const */ JS_PUBLIC_DATA(Class) js_FunctionClass = { js_Function_str, - JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | - JSCLASS_HAS_RESERVED_SLOTS(JSObject::FUN_FIXED_RESERVED_SLOTS) | + JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_HAS_RESERVED_SLOTS(2) | JSCLASS_MARK_IS_TRACE | JSCLASS_HAS_CACHED_PROTO(JSProto_Function), PropertyStub, PropertyStub, PropertyStub, PropertyStub, @@ -2228,7 +2149,8 @@ Function(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value *rval) * js_CheckContentSecurityPolicy is defined in jsobj.cpp */ if (!js_CheckContentSecurityPolicy(cx)) { - JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CSP_BLOCKED_FUNCTION); + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, + JSMSG_CSP_BLOCKED_FUNCTION); return JS_FALSE; } diff --git a/js/src/jsfun.h b/js/src/jsfun.h index e24b25651c20..5ca4f91f8010 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -45,8 +45,6 @@ #include "jsprvtd.h" #include "jspubtd.h" #include "jsobj.h" -#include "jsatom.h" -#include "jsstr.h" typedef struct JSLocalNameMap JSLocalNameMap; @@ -97,10 +95,6 @@ typedef union JSLocalNames { * can move to u.i.script->flags. For now we use function flag bits to minimize * pointer-chasing. */ -#define JSFUN_JOINABLE 0x0001 /* function is null closure that does not - appear to call itself via its own name - or arguments.callee */ - #define JSFUN_EXPR_CLOSURE 0x1000 /* expression closure: function(x) x*x */ #define JSFUN_TRCINFO 0x2000 /* when set, u.n.trcinfo is non-null, JSFunctionSpec::call points to a @@ -208,44 +202,6 @@ struct JSFunction : public JSObject bool mightEscape() const { return FUN_INTERPRETED(this) && (FUN_FLAT_CLOSURE(this) || u.i.nupvars == 0); } - - bool joinable() const { - return flags & JSFUN_JOINABLE; - } - - private: - /* - * js_FunctionClass reserves two slots, which are free in JSObject::fslots - * without requiring dslots allocation. Null closures that can be joined to - * a compiler-created function object use the first one to hold a mutable - * methodAtom() state variable, needed for correct foo.caller handling. - */ - enum { - METHOD_ATOM_SLOT = JSSLOT_FUN_METHOD_ATOM - }; - - public: - void setJoinable() { - JS_ASSERT(FUN_INTERPRETED(this)); - fslots[METHOD_ATOM_SLOT].setNull(); - flags |= JSFUN_JOINABLE; - } - - /* - * Method name imputed from property uniquely assigned to or initialized, - * where the function does not need to be cloned to carry a scope chain or - * flattened upvars. - */ - JSAtom *methodAtom() const { - return (joinable() && fslots[METHOD_ATOM_SLOT].isString()) - ? STRING_TO_ATOM(fslots[METHOD_ATOM_SLOT].toString()) - : NULL; - } - - void setMethodAtom(JSAtom *atom) { - JS_ASSERT(joinable()); - fslots[METHOD_ATOM_SLOT].setString(ATOM_TO_STRING(atom)); - } }; JS_STATIC_ASSERT(sizeof(JSFunction) % JS_GCTHING_ALIGN == 0); diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 16b375204083..fcc6325d178a 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2525,17 +2525,6 @@ js_TraceRuntime(JSTracer *trc) if (rt->gcExtraRootsTraceOp) rt->gcExtraRootsTraceOp(trc, rt->gcExtraRootsData); - -#ifdef JS_FUNCTION_METERING - for (int k = 0; k < 2; k++) { - typedef JSRuntime::FunctionCountMap HM; - HM &h = (k == 0) ? rt->methodReadBarrierCountMap : rt->unjoinedFunctionCountMap; - for (HM::Range r = h.all(); !r.empty(); r.popFront()) { - JSFunction *fun = r.front().key; - JS_CALL_OBJECT_TRACER(trc, fun, "FunctionCountMap key"); - } - } -#endif } void diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 95c5e87ebbfe..2540e6662996 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -2696,15 +2696,15 @@ BEGIN_CASE(JSOP_STOP) == JSOP_CALL_LENGTH); TRACE_0(LeaveFrame); if (!TRACE_RECORDER(cx) && recursive) { - if (regs.pc[JSOP_CALL_LENGTH] == JSOP_TRACE) { + if (*(regs.pc + JSOP_CALL_LENGTH) == JSOP_TRACE) { regs.pc += JSOP_CALL_LENGTH; MONITOR_BRANCH(Record_LeaveFrame); op = (JSOp)*regs.pc; DO_OP(); } } - if (regs.pc[JSOP_CALL_LENGTH] == JSOP_TRACE || - regs.pc[JSOP_CALL_LENGTH] == JSOP_NOP) { + if (*(regs.pc + JSOP_CALL_LENGTH) == JSOP_TRACE || + *(regs.pc + JSOP_CALL_LENGTH) == JSOP_NOP) { JS_STATIC_ASSERT(JSOP_TRACE_LENGTH == JSOP_NOP_LENGTH); regs.pc += JSOP_CALL_LENGTH; len = JSOP_TRACE_LENGTH; @@ -4064,8 +4064,7 @@ BEGIN_CASE(JSOP_GETXPROP) jsid id = ATOM_TO_JSID(atom); if (JS_LIKELY(aobj->map->ops->getProperty == js_GetProperty) ? !js_GetPropertyHelper(cx, obj, id, - (fp->imacpc || - regs.pc[JSOP_GETPROP_LENGTH + i] == JSOP_IFEQ) + fp->imacpc ? JSGET_CACHE_RESULT | JSGET_NO_METHOD_BARRIER : JSGET_CACHE_RESULT | JSGET_METHOD_BARRIER, &rval) @@ -5728,19 +5727,24 @@ BEGIN_CASE(JSOP_LAMBDA) parent = fp->scopeChain; if (obj->getParent() == parent) { - jsbytecode *pc2 = regs.pc + JSOP_LAMBDA_LENGTH; - JSOp op2 = JSOp(*pc2); + op = JSOp(regs.pc[JSOP_LAMBDA_LENGTH]); /* - * Optimize var obj = {method: function () { ... }, ...}, - * this.method = function () { ... }; and other significant - * single-use-of-null-closure bytecode sequences. - * - * WARNING: code in TraceRecorder::record_JSOP_LAMBDA must - * match the optimization cases in the following code that - * break from the outer do-while(0). + * Optimize ({method: function () { ... }, ...}) and + * this.method = function () { ... }; bytecode sequences. */ - if (op2 == JSOP_INITMETHOD) { + if (op == JSOP_SETMETHOD) { +#ifdef DEBUG + JSOp op2 = JSOp(regs.pc[JSOP_LAMBDA_LENGTH + JSOP_SETMETHOD_LENGTH]); + JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV); +#endif + + const Value &lref = regs.sp[-1]; + if (lref.isObject() && + lref.toObject().getClass() == &js_ObjectClass) { + break; + } + } else if (op == JSOP_INITMETHOD) { #ifdef DEBUG const Value &lref = regs.sp[-1]; JS_ASSERT(lref.isObject()); @@ -5748,78 +5752,9 @@ BEGIN_CASE(JSOP_LAMBDA) JS_ASSERT(obj2->getClass() == &js_ObjectClass); JS_ASSERT(obj2->scope()->object == obj2); #endif - - fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(JSOP_LAMBDA_LENGTH))); - JS_FUNCTION_METER(cx, joinedinitmethod); break; } - - if (op2 == JSOP_SETMETHOD) { -#ifdef DEBUG - op2 = JSOp(pc2[JSOP_SETMETHOD_LENGTH]); - JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV); -#endif - - const Value &lref = regs.sp[-1]; - if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) { - fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(JSOP_LAMBDA_LENGTH))); - JS_FUNCTION_METER(cx, joinedsetmethod); - break; - } - } else if (fun->joinable()) { - if (op2 == JSOP_CALL) { - /* - * Array.prototype.sort and String.prototype.replace are - * optimized as if they are special form. We know that they - * won't leak the joined function object in obj, therefore - * we don't need to clone that compiler- created function - * object for identity/mutation reasons. - */ - int iargc = GET_ARGC(pc2); - - /* - * Note that we have not yet pushed obj as the final argument, - * so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)], - * is the callee for this JSOP_CALL. - */ - JSFunction *calleeFun = - GET_FUNCTION_PRIVATE(cx, ®s.sp[1 - (iargc + 2)].toObject()); - FastNative fastNative = FUN_FAST_NATIVE(calleeFun); - - if (iargc == 1 && fastNative == array_sort) { - JS_FUNCTION_METER(cx, joinedsort); - break; - } - if (iargc == 2 && fastNative == str_replace) { - JS_FUNCTION_METER(cx, joinedreplace); - break; - } - } else if (op2 == JSOP_NULL) { - pc2 += JSOP_NULL_LENGTH; - op2 = JSOp(*pc2); - - if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) { - JS_FUNCTION_METER(cx, joinedmodulepat); - break; - } - } - } } - -#ifdef JS_FUNCTION_METERING - // No locking, this is mainly for js shell testing. - ++rt->functionMeter.unjoined; - - typedef JSRuntime::FunctionCountMap HM; - HM &h = rt->unjoinedFunctionCountMap; - HM::AddPtr p = h.lookupForAdd(fun); - if (!p) { - h.add(p, fun, 1); - } else { - JS_ASSERT(p->key == fun); - ++p->value; - } -#endif } else { parent = js_GetScopeChain(cx, fp); if (!parent) diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 9a2f287968e5..7ed3f9fb1f15 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -181,32 +181,14 @@ struct JSStackFrame return argv[-2]; } - /* Infallible getter to return the callee object from this frame. */ - JSObject &calleeObject() const { - JS_ASSERT(argv); - return argv[-2].toObject(); - } - - /* - * Fallible getter to compute the correct callee function object, which may - * require deferred cloning due to JSScope::methodReadBarrier. For a frame - * with null fun member, return true with *vp set from this->callee(). - */ - bool getValidCalleeObject(JSContext *cx, js::Value *vp); - - void setCalleeObject(JSObject &callable) { - JS_ASSERT(argv); - argv[-2].setObject(callable); - } - JSObject *callee() { return argv ? &argv[-2].toObject() : NULL; } /* - * Get the "variable object" (ES3 term) associated with the Execution - * Context's VariableEnvironment (ES5 10.3). The given CallStackSegment - * must contain this stack frame. + * Get the object associated with the Execution Context's + * VariableEnvironment (ES5 10.3). The given CallStackSegment must contain + * this stack frame. */ JSObject *varobj(js::CallStackSegment *css) const; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index ff3962652e6f..ebaa45d51fbb 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -5141,13 +5141,13 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, * Check for Object class here to avoid defining a method on a class * with magic resolve, addProperty, getProperty, etc. hooks. */ - if ((defineHow & JSDNP_SET_METHOD) && obj->canHaveMethodBarrier()) { + if ((defineHow & JSDNP_SET_METHOD) && + obj->getClass() == &js_ObjectClass) { JS_ASSERT(IsFunctionObject(*vp)); JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER))); JSObject *funobj = &vp->toObject(); - JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); - if (fun == funobj) { + if (FUN_OBJECT(GET_FUNCTION_PRIVATE(cx, funobj)) == funobj) { flags |= JSScopeProperty::METHOD; getter = CastAsPropertyOp(funobj); } @@ -5298,39 +5298,8 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval) } scope = obj->scope(); - if (SPROP_HAS_VALID_SLOT(sprop, scope)) { - const Value &v = obj->lockedGetSlot(sprop->slot); - GC_POKE(cx, v); - - /* - * Delete is rare enough that we can take the hit of checking for an - * active cloned method function object that must be homed to a callee - * slot on the active stack frame before this delete completes, in case - * someone saved the clone and checks it against foo.caller for a foo - * called from the active method. - * - * We do not check suspended frames. They can't be reached via caller, - * so the only way they could have the method's joined function object - * as callee is through an API abusage. We break any such edge case. - */ - if (scope->hasMethodBarrier()) { - JSObject *funobj; - - if (IsFunctionObject(v, &funobj)) { - JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); - - if (fun != funobj) { - for (JSStackFrame *fp = cx->fp; fp; fp = fp->down) { - if (fp->callee() == fun && - fp->thisv.isObject() && - &fp->thisv.toObject() == obj) { - fp->setCalleeObject(*funobj); - } - } - } - } - } - } + if (SPROP_HAS_VALID_SLOT(sprop, scope)) + GC_POKE(cx, obj->lockedGetSlot(sprop->slot)); ok = scope->removeProperty(cx, id); JS_UNLOCK_OBJ(cx, obj); @@ -6254,7 +6223,7 @@ dumpValue(const Value &v) Class *clasp = obj->getClass(); fprintf(stderr, "<%s%s at %p>", clasp->name, - (clasp == &js_ObjectClass) ? "" : " object", + clasp == &js_ObjectClass ? "" : " object", (void *) obj); } else if (v.isBoolean()) { if (v.toBoolean()) diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 97bfc493b199..e82557b2337e 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -252,8 +252,6 @@ const uint32 JSSLOT_PARENT = 0; */ const uint32 JSSLOT_PRIVATE = 1; -class JSFunction; - /* * JSObject struct, with members sized to fit in 32 bytes on 32-bit targets, * 64 bytes on 64-bit systems. The JSFunction struct is an extension of this @@ -295,7 +293,7 @@ struct JSObject { js::Value *dslots; /* dynamically allocated slots */ #if JS_BITS_PER_WORD == 32 // TODO: this is needed to pad out fslots. alternatively, clasp could be - // merged with flags and the padding removed, but I think the upcoming + // merged by with flags and the padding removed, but I think the upcoming // removal of JSScope will change this all anyway so I will leave this // here for now. uint32 padding; @@ -561,22 +559,6 @@ struct JSObject { inline const js::Value &getDateUTCTime() const; inline void setDateUTCTime(const js::Value &pthis); - /* - * Function-specific getters and setters. - */ - - private: - friend class JSFunction; - - static const uint32 JSSLOT_FUN_METHOD_ATOM = JSSLOT_PRIVATE + 1; - static const uint32 JSSLOT_FUN_METHOD_OBJ = JSSLOT_PRIVATE + 2; - - public: - static const uint32 FUN_FIXED_RESERVED_SLOTS = 2; - - inline bool hasMethodObj(const JSObject& obj) const; - inline void setMethodObj(JSObject& obj); - /* * RegExp-specific getters and setters. */ @@ -740,8 +722,6 @@ struct JSObject { void swap(JSObject *obj); - inline bool canHaveMethodBarrier() const; - inline bool isArguments() const; inline bool isArray() const; inline bool isDenseArray() const; @@ -752,9 +732,6 @@ struct JSObject { inline bool isPrimitive() const; inline bool isDate() const; inline bool isFunction() const; - inline bool isObject() const; - inline bool isWith() const; - inline bool isBlock() const; inline bool isRegExp() const; inline bool isXML() const; inline bool isNamespace() const; @@ -854,10 +831,6 @@ extern js::Class js_ObjectClass; extern js::Class js_WithClass; extern js::Class js_BlockClass; -inline bool JSObject::isObject() const { return getClass() == &js_ObjectClass; } -inline bool JSObject::isWith() const { return getClass() == &js_WithClass; } -inline bool JSObject::isBlock() const { return getClass() == &js_BlockClass; } - /* * Block scope object macros. The slots reserved by js_BlockClass are: * diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 7d4ff4263602..f94df13c57df 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -106,12 +106,6 @@ JSObject::getReservedSlot(uintN index) const return (slot < numSlots()) ? getSlot(slot) : js::UndefinedValue(); } -inline bool -JSObject::canHaveMethodBarrier() const -{ - return isObject() || isFunction() || isPrimitive() || isDate(); -} - inline bool JSObject::isPrimitive() const { @@ -313,19 +307,6 @@ JSObject::setDateUTCTime(const js::Value &time) fslots[JSSLOT_DATE_UTC_TIME] = time; } -inline bool -JSObject::hasMethodObj(const JSObject& obj) const -{ - return fslots[JSSLOT_FUN_METHOD_OBJ].isObject() && - &fslots[JSSLOT_FUN_METHOD_OBJ].toObject() == &obj; -} - -inline void -JSObject::setMethodObj(JSObject& obj) -{ - fslots[JSSLOT_FUN_METHOD_OBJ].setObject(obj); -} - inline const js::Value & JSObject::getRegExpLastIndex() const { diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index af60f81d779e..eb430d3bbabe 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -5499,6 +5499,10 @@ SimulateImacroCFG(JSContext *cx, JSScript *script, #undef LOCAL_ASSERT #define LOCAL_ASSERT(expr) LOCAL_ASSERT_RV(expr, -1); +static intN +ReconstructPCStack(JSContext *cx, JSScript *script, jsbytecode *target, + jsbytecode **pcstack); + static intN ReconstructImacroPCStack(JSContext *cx, JSScript *script, jsbytecode *imacstart, jsbytecode *target, diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index f255950d8cd2..bafa70f72183 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -2130,7 +2130,11 @@ DeoptimizeUsesWithin(JSDefinition *dn, JSFunctionBox *funbox, uint32& tcflags) void Parser::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags) { -#define FUN_METER(x) JS_FUNCTION_METER(context, x) +#ifdef JS_FUNCTION_METERING +# define FUN_METER(x) JS_RUNTIME_METER(context->runtime, functionMeter.x) +#else +# define FUN_METER(x) ((void)0) +#endif for (;;) { JSParseNode *fn = funbox->node; @@ -2340,9 +2344,6 @@ Parser::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags) } } - if (funbox->joinable()) - fun->setJoinable(); - funbox = funbox->siblings; if (!funbox) break; diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 433b10c4afa0..39c9c06e3436 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -1214,7 +1214,7 @@ JSScope::methodShapeChange(JSContext *cx, JSScopeProperty *sprop) const Value &prev = object->lockedGetSlot(sprop->slot); JS_ASSERT(&sprop->methodObject() == &prev.toObject()); JS_ASSERT(hasMethodBarrier()); - JS_ASSERT(object->canHaveMethodBarrier()); + JS_ASSERT(object->getClass() == &js_ObjectClass); JS_ASSERT(!sprop->rawSetter || sprop->rawSetter == js_watch_set); #endif diff --git a/js/src/jsscope.h b/js/src/jsscope.h index bc3e72889f0c..a0efd4b7857f 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -612,9 +612,7 @@ struct JSScopeProperty { union { js::PropertyOp rawGetter; /* getter and setter hooks or objects */ JSObject *getterObj; /* user-defined callable "get" object or - null if sprop->hasGetterValue(); or - joined function object if METHOD flag - is set. */ + null if sprop->hasGetterValue() */ JSScopeProperty *next; /* next node in freelist */ }; diff --git a/js/src/jsscopeinlines.h b/js/src/jsscopeinlines.h index 8182a40b0c19..6249473c182a 100644 --- a/js/src/jsscopeinlines.h +++ b/js/src/jsscopeinlines.h @@ -122,35 +122,17 @@ JSScope::methodReadBarrier(JSContext *cx, JSScopeProperty *sprop, js::Value *vp) JS_ASSERT(hasProperty(sprop)); JS_ASSERT(sprop->isMethod()); JS_ASSERT(&vp->toObject() == &sprop->methodObject()); - JS_ASSERT(object->canHaveMethodBarrier()); + JS_ASSERT(object->getClass() == &js_ObjectClass); JSObject *funobj = &vp->toObject(); JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj); - JS_ASSERT(fun == funobj && FUN_NULL_CLOSURE(fun)); + JS_ASSERT(FUN_OBJECT(fun) == funobj && FUN_NULL_CLOSURE(fun)); funobj = CloneFunctionObject(cx, fun, funobj->getParent()); if (!funobj) return false; - funobj->setMethodObj(*object); - vp->setObject(*funobj); - if (!js_SetPropertyHelper(cx, object, sprop->id, 0, vp)) - return false; - -#ifdef JS_FUNCTION_METERING - JS_FUNCTION_METER(cx, mreadbarrier); - - typedef JSRuntime::FunctionCountMap HM; - HM &h = cx->runtime->methodReadBarrierCountMap; - HM::AddPtr p = h.lookupForAdd(fun); - if (!p) { - h.add(p, fun, 1); - } else { - JS_ASSERT(p->key == fun); - ++p->value; - } -#endif - return true; + return !!js_SetPropertyHelper(cx, object, sprop->id, 0, vp); } static JS_ALWAYS_INLINE bool @@ -167,10 +149,8 @@ JSScope::methodWriteBarrier(JSContext *cx, JSScopeProperty *sprop, { if (flags & (BRANDED | METHOD_BARRIER)) { const js::Value &prev = object->lockedGetSlot(sprop->slot); - if (ChangesMethodValue(prev, v)) { - JS_FUNCTION_METER(cx, mwritebarrier); + if (ChangesMethodValue(prev, v)) return methodShapeChange(cx, sprop); - } } return true; } @@ -180,10 +160,8 @@ JSScope::methodWriteBarrier(JSContext *cx, uint32 slot, const js::Value &v) { if (flags & (BRANDED | METHOD_BARRIER)) { const js::Value &prev = object->lockedGetSlot(slot); - if (ChangesMethodValue(prev, v)) { - JS_FUNCTION_METER(cx, mwslotbarrier); + if (ChangesMethodValue(prev, v)) return methodShapeChange(cx, slot); - } } return true; } diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index fc2d184af179..8d37aa94f31f 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -2301,8 +2301,8 @@ BuildFlatReplacement(JSContext *cx, JSString *textstr, JSString *repstr, return true; } -JSBool -js::str_replace(JSContext *cx, uintN argc, Value *vp) +static JSBool +str_replace(JSContext *cx, uintN argc, Value *vp) { ReplaceData rdata(cx); NORMALIZE_THIS(cx, vp, rdata.str); diff --git a/js/src/jsstr.h b/js/src/jsstr.h index a316e4be6d05..254b98dda926 100644 --- a/js/src/jsstr.h +++ b/js/src/jsstr.h @@ -995,15 +995,6 @@ extern JSBool js_str_escape(JSContext *cx, JSObject *obj, uintN argc, js::Value *argv, js::Value *rval); -/* - * The String.prototype.replace fast-native entry point is exported for joined - * function optimization in js{interp,tracer}.cpp. - */ -namespace js { -extern JSBool -str_replace(JSContext *cx, uintN argc, js::Value *vp); -} - extern JSBool js_str_toString(JSContext *cx, uintN argc, js::Value *vp); diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index ccb5f2525266..3ea3e661ee54 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -14816,56 +14816,19 @@ TraceRecorder::record_JSOP_LAMBDA() if (FUN_NULL_CLOSURE(fun)) { if (FUN_OBJECT(fun)->getParent() != globalObj) RETURN_STOP_A("Null closure function object parent must be global object"); - - jsbytecode *pc2 = cx->regs->pc + JSOP_LAMBDA_LENGTH; - JSOp op2 = JSOp(*pc2); - - if (op2 == JSOP_INITMETHOD) { - stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); - return ARECORD_CONTINUE; - } + JSOp op2 = JSOp(cx->regs->pc[JSOP_LAMBDA_LENGTH]); if (op2 == JSOP_SETMETHOD) { Value lval = stackval(-1); - if (!lval.isPrimitive() && lval.toObject().canHaveMethodBarrier()) { + if (!lval.isPrimitive() && + lval.toObject().getClass() == &js_ObjectClass) { stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); return ARECORD_CONTINUE; } - } else if (fun->joinable()) { - if (op2 == JSOP_CALL) { - /* - * Array.prototype.sort and String.prototype.replace are - * optimized as if they are special form. We know that they - * won't leak the joined function object in obj, therefore - * we don't need to clone that compiler- created function - * object for identity/mutation reasons. - */ - int iargc = GET_ARGC(pc2); - - /* - * Note that we have not yet pushed obj as the final argument, - * so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)], - * is the callee for this JSOP_CALL. - */ - JSFunction *calleeFun = - GET_FUNCTION_PRIVATE(cx, &cx->regs->sp[1 - (iargc + 2)].toObject()); - FastNative fastNative = FUN_FAST_NATIVE(calleeFun); - - if ((iargc == 1 && fastNative == array_sort) || - (iargc == 2 && fastNative == str_replace)) { - stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); - return ARECORD_CONTINUE; - } - } else if (op2 == JSOP_NULL) { - pc2 += JSOP_NULL_LENGTH; - op2 = JSOp(*pc2); - - if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) { - stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); - return ARECORD_CONTINUE; - } - } + } else if (op2 == JSOP_INITMETHOD) { + stack(0, INS_CONSTOBJ(FUN_OBJECT(fun))); + return ARECORD_CONTINUE; } LIns *proto_ins; diff --git a/js/src/tests/js1_8_5/regress/jstests.list b/js/src/tests/js1_8_5/regress/jstests.list index 3ecde0d61546..4c12e6a4adf2 100644 --- a/js/src/tests/js1_8_5/regress/jstests.list +++ b/js/src/tests/js1_8_5/regress/jstests.list @@ -24,5 +24,3 @@ script regress-566914.js script regress-567152.js script regress-569306.js script regress-571014.js -script regress-577648-1.js -script regress-577648-2.js diff --git a/js/src/tests/js1_8_5/regress/regress-577648-1.js b/js/src/tests/js1_8_5/regress/regress-577648-1.js deleted file mode 100644 index d40c7b0e4237..000000000000 --- a/js/src/tests/js1_8_5/regress/regress-577648-1.js +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/licenses/publicdomain/ - */ - -var count = 0; - -function testCaller(obj) { - switch (++count) { - case 1: - case 2: - /* - * The first two times, obj is objA. The first time, we reference - * arguments.callee.caller before obj.go, so the caller getter must - * force the joined function object in the stack frame to cross the - * method read barrier. The second time, obj.go has been cloned and - * it should match the new frame's callee from the get-go. - */ - assertEq(obj, objA); - break; - - case 3: { - assertEq(obj, objB); - - /* - * Store another clone of the joined function object before obj.go has - * been read, but after it has been invoked via objB.go(objB). - * - * In this case, arguments.callee.caller must not lie and return what - * is currently stored in objB.go, since that function object (objA.go) - * was cloned earlier, when count was 1, and it is not the function - * object that was truly invoked. - * - * But since the invocation of objB.go(objB) did not clone go, and the - * following assignment overwrote the invoked value, leaving the only - * reference to the joined function object for go in the stack frame's - * callee (argv[-2]) member, the arguments.callee.caller reference must - * clone a function object for the callee, store it as the callee, and - * return it here. - * - * It won't equal obj.go, but (implementation detail) it should have - * the same proto as obj.go - */ - obj.go = objA.go; - - let caller = arguments.callee.caller; - let obj_go = obj.go; - return caller != obj_go && caller.__proto__ == obj_go.__proto__; - } - - case 4: { - assertEq(obj, objC); - - let save = obj.go; - delete obj.go; - return arguments.callee.caller == save; - } - - case 5: { - assertEq(obj, objD); - - let read = obj.go; - break; - } - } - - return arguments.callee.caller == obj.go; -} - -function make() { - return { - go: function(obj) { - return testCaller(obj); - } - }; -} - -var objA = make(), - objB = make(), - objC = make(), - objD = make(); - -reportCompare(true, objA.go(objA), "1"); -reportCompare(true, objA.go(objA), "2"); -reportCompare(true, objB.go(objB), "3"); -reportCompare(true, objC.go(objC), "4"); -reportCompare(true, objD.go(objD), "5"); diff --git a/js/src/tests/js1_8_5/regress/regress-577648-2.js b/js/src/tests/js1_8_5/regress/regress-577648-2.js deleted file mode 100644 index d9ba9131630e..000000000000 --- a/js/src/tests/js1_8_5/regress/regress-577648-2.js +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/licenses/publicdomain/ - * Contributor: Jason Orendorff - */ - -var o = { f: function() { return o.g(); }, g: function() { return arguments.callee.caller; } }; -var c = o.f(); -var i = 'f'; -var d = o[i](); - -reportCompare(true, c === o.f && d === o.f(), ""); From 49ded6d0dfc7aefa01fb0b5f8e1c514f6ae2798c Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Mon, 12 Jul 2010 15:40:44 -0500 Subject: [PATCH 08/19] Bug 465199 - RegExp.lastIndex setting shouldn't coerce to integer (should happen during internal use of the property instead). r=cdleary --HG-- extra : rebase_source : d2f32831a11a4e88dbed927a9a17e96f2a9561b3 --- js/src/jsobj.h | 1 + js/src/jsobjinlines.h | 21 ------- js/src/jsregexp.cpp | 23 +++---- js/src/jsregexp.h | 28 +++++++++ js/src/tests/ecma_5/RegExp/15.10.7.5-01.js | 72 ++++++++++++++++++++++ js/src/tests/ecma_5/RegExp/browser.js | 1 + js/src/tests/ecma_5/RegExp/jstests.list | 2 + js/src/tests/ecma_5/RegExp/shell.js | 1 + js/src/tests/ecma_5/jstests.list | 1 + 9 files changed, 115 insertions(+), 35 deletions(-) create mode 100644 js/src/tests/ecma_5/RegExp/15.10.7.5-01.js create mode 100644 js/src/tests/ecma_5/RegExp/browser.js create mode 100644 js/src/tests/ecma_5/RegExp/jstests.list create mode 100644 js/src/tests/ecma_5/RegExp/shell.js diff --git a/js/src/jsobj.h b/js/src/jsobj.h index e82557b2337e..4c0bcd5cd26b 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -571,6 +571,7 @@ struct JSObject { inline const js::Value &getRegExpLastIndex() const; inline void setRegExpLastIndex(const js::Value &v); + inline void setRegExpLastIndex(jsdouble d); inline void zeroRegExpLastIndex(); /* diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index f94df13c57df..da3e2e0c484d 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -307,27 +307,6 @@ JSObject::setDateUTCTime(const js::Value &time) fslots[JSSLOT_DATE_UTC_TIME] = time; } -inline const js::Value & -JSObject::getRegExpLastIndex() const -{ - JS_ASSERT(isRegExp()); - return fslots[JSSLOT_REGEXP_LAST_INDEX]; -} - -inline void -JSObject::setRegExpLastIndex(const js::Value &v) -{ - JS_ASSERT(isRegExp()); - fslots[JSSLOT_REGEXP_LAST_INDEX] = v; -} - -inline void -JSObject::zeroRegExpLastIndex() -{ - JS_ASSERT(isRegExp()); - fslots[JSSLOT_REGEXP_LAST_INDEX].setInt32(0); -} - inline NativeIterator * JSObject::getNativeIterator() const { diff --git a/js/src/jsregexp.cpp b/js/src/jsregexp.cpp index 7444c70a024a..cd688fe69a29 100644 --- a/js/src/jsregexp.cpp +++ b/js/src/jsregexp.cpp @@ -5086,13 +5086,6 @@ out: /************************************************************************/ -static void -SetRegExpLastIndex(JSContext *cx, JSObject *obj, jsdouble lastIndex) -{ - JS_ASSERT(obj->isRegExp()); - obj->setRegExpLastIndex(NumberValue(lastIndex)); -} - #define DEFINE_GETTER(name, code) \ static JSBool \ name(JSContext *cx, JSObject *obj, jsid id, Value *vp) \ @@ -5125,11 +5118,7 @@ lastIndex_setter(JSContext *cx, JSObject *obj, jsid id, Value *vp) if (!obj) return true; } - jsdouble lastIndex; - if (!ValueToNumber(cx, *vp, &lastIndex)) - return false; - lastIndex = js_DoubleToInteger(lastIndex); - SetRegExpLastIndex(cx, obj, lastIndex); + obj->setRegExpLastIndex(*vp); return true; } @@ -5653,7 +5642,13 @@ regexp_exec_sub(JSContext *cx, JSObject *obj, uintN argc, Value *argv, HOLD_REGEXP(cx, re); sticky = (re->flags & JSREG_STICKY) != 0; if (re->flags & (JSREG_GLOB | JSREG_STICKY)) { - lastIndex = obj->getRegExpLastIndex().toNumber(); + const Value &v = obj->getRegExpLastIndex(); + if (v.isNumber()) { + lastIndex = v.toNumber(); + } else { + if (!ValueToNumber(cx, v, &lastIndex)) + return JS_FALSE; + } } else { lastIndex = 0; } @@ -5697,7 +5692,7 @@ regexp_exec_sub(JSContext *cx, JSObject *obj, uintN argc, Value *argv, if (rval->isNull()) obj->zeroRegExpLastIndex(); else - SetRegExpLastIndex(cx, obj, i); + obj->setRegExpLastIndex(i); } } diff --git a/js/src/jsregexp.h b/js/src/jsregexp.h index b299c336f8cc..c43dc0228760 100644 --- a/js/src/jsregexp.h +++ b/js/src/jsregexp.h @@ -50,6 +50,34 @@ #include "jsdhash.h" #endif +inline const js::Value & +JSObject::getRegExpLastIndex() const +{ + JS_ASSERT(isRegExp()); + return fslots[JSSLOT_REGEXP_LAST_INDEX]; +} + +inline void +JSObject::setRegExpLastIndex(const js::Value &v) +{ + JS_ASSERT(isRegExp()); + fslots[JSSLOT_REGEXP_LAST_INDEX] = v; +} + +inline void +JSObject::setRegExpLastIndex(jsdouble d) +{ + JS_ASSERT(isRegExp()); + fslots[JSSLOT_REGEXP_LAST_INDEX] = js::NumberValue(d); +} + +inline void +JSObject::zeroRegExpLastIndex() +{ + JS_ASSERT(isRegExp()); + fslots[JSSLOT_REGEXP_LAST_INDEX].setInt32(0); +} + namespace js { class AutoStringRooter; } extern JS_FRIEND_API(void) diff --git a/js/src/tests/ecma_5/RegExp/15.10.7.5-01.js b/js/src/tests/ecma_5/RegExp/15.10.7.5-01.js new file mode 100644 index 000000000000..8adfcdf591d8 --- /dev/null +++ b/js/src/tests/ecma_5/RegExp/15.10.7.5-01.js @@ -0,0 +1,72 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var gTestfile = '15.10.7.5-01.js'; +var BUGNUMBER = 465199; +var summary = "RegExp lastIndex property set should not coerce type to number"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +var called = false; +var o = { valueOf: function() { called = true; return 1; } }; +var r = /a/g; +var desc, m; + +assertEq(r.lastIndex, 0); + +desc = Object.getOwnPropertyDescriptor(r, "lastIndex"); +assertEq(desc.enumerable, false); +assertEq(desc.configurable, false); +assertEq(desc.value, 0); +assertEq(desc.writable, true); + +r.lastIndex = o; + +assertEq(r.lastIndex, o); + +desc = Object.getOwnPropertyDescriptor(r, "lastIndex"); +assertEq(desc.enumerable, false); +assertEq(desc.configurable, false); +assertEq(desc.value, o); +assertEq(desc.writable, true); + +assertEq(called, false); +assertEq(r.exec("aaaa").length, 1); +assertEq(called, true); +assertEq(r.lastIndex, 2); + +desc = Object.getOwnPropertyDescriptor(r, "lastIndex"); +assertEq(desc.enumerable, false); +assertEq(desc.configurable, false); +assertEq(desc.value, 2); +assertEq(desc.writable, true); + + +r.lastIndex = -0.2; +assertEq(r.lastIndex, -0.2); + +m = r.exec("aaaa"); +assertEq(Array.isArray(m), true); +assertEq(m.length, 1); +assertEq(m[0], "a"); +assertEq(r.lastIndex, 1); + +m = r.exec("aaaa"); +assertEq(Array.isArray(m), true); +assertEq(m.length, 1); +assertEq(m[0], "a"); +assertEq(r.lastIndex, 2); + + +/******************************************************************************/ + +if (typeof reportCompare === "function") + reportCompare(true, true); + +print("All tests passed!"); diff --git a/js/src/tests/ecma_5/RegExp/browser.js b/js/src/tests/ecma_5/RegExp/browser.js new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/js/src/tests/ecma_5/RegExp/browser.js @@ -0,0 +1 @@ + diff --git a/js/src/tests/ecma_5/RegExp/jstests.list b/js/src/tests/ecma_5/RegExp/jstests.list new file mode 100644 index 000000000000..ed3044da2c8a --- /dev/null +++ b/js/src/tests/ecma_5/RegExp/jstests.list @@ -0,0 +1,2 @@ +url-prefix ../../jsreftest.html?test=ecma_5/RegExp/ +script 15.10.7.5-01.js diff --git a/js/src/tests/ecma_5/RegExp/shell.js b/js/src/tests/ecma_5/RegExp/shell.js new file mode 100644 index 000000000000..f5fba0512240 --- /dev/null +++ b/js/src/tests/ecma_5/RegExp/shell.js @@ -0,0 +1 @@ +gTestsubsuite='RegExp'; diff --git a/js/src/tests/ecma_5/jstests.list b/js/src/tests/ecma_5/jstests.list index edd8063010da..e700343e0d8b 100644 --- a/js/src/tests/ecma_5/jstests.list +++ b/js/src/tests/ecma_5/jstests.list @@ -3,6 +3,7 @@ include Date/jstests.list include Expressions/jstests.list include JSON/jstests.list include Object/jstests.list +include RegExp/jstests.list include Types/jstests.list include extensions/jstests.list include misc/jstests.list From fe367b4bb15642f64749febea62c06d23ac18ef5 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Thu, 22 Jul 2010 22:28:33 -0500 Subject: [PATCH 09/19] Bug 581067 - U+FEFF should be a WhiteSpace character (change in ES5 from ES3). r=cdleary --HG-- extra : rebase_source : 38947caa8d4f21d0867137d0933495061a88d052 --- js/src/jsstr.h | 7 +++++-- js/src/tests/ecma/GlobalObject/15.1.2.2-1.js | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/js/src/jsstr.h b/js/src/jsstr.h index 254b98dda926..8981341cd95c 100644 --- a/js/src/jsstr.h +++ b/js/src/jsstr.h @@ -752,15 +752,18 @@ extern const bool js_alnum[]; #define JS_ISDIGIT(c) (JS_CTYPE(c) == JSCT_DECIMAL_DIGIT_NUMBER) +const jschar BYTE_ORDER_MARK = 0xFEFF; +const jschar NO_BREAK_SPACE = 0x00A0; + static inline bool JS_ISSPACE(jschar c) { unsigned w = c; if (w < 256) - return (w <= ' ' && (w == ' ' || (9 <= w && w <= 0xD))) || w == 0xA0; + return (w <= ' ' && (w == ' ' || (9 <= w && w <= 0xD))) || w == NO_BREAK_SPACE; - return (JS_CCODE(w) & 0x00070000) == 0x00040000; + return w == BYTE_ORDER_MARK || (JS_CCODE(w) & 0x00070000) == 0x00040000; } #define JS_ISPRINT(c) ((c) < 128 && isprint(c)) diff --git a/js/src/tests/ecma/GlobalObject/15.1.2.2-1.js b/js/src/tests/ecma/GlobalObject/15.1.2.2-1.js index e7cfdf14a7c4..daf013ca2511 100644 --- a/js/src/tests/ecma/GlobalObject/15.1.2.2-1.js +++ b/js/src/tests/ecma/GlobalObject/15.1.2.2-1.js @@ -218,6 +218,8 @@ for ( var space = " ", HEX_STRING = "0x0", HEX_VALUE = 0, POWER = 0; HEX_VALUE += Math.pow(16,POWER)*15; } +new TestCase(SECTION, "parseInt(BOM + '123', 10)", 123, parseInt("\uFEFF" + "123", 10)); + // a few tests with negative numbers for ( HEX_STRING = "-0x0", HEX_VALUE = 0, POWER = 0; POWER < 15; POWER++, HEX_STRING = HEX_STRING +"f" ) { new TestCase( SECTION, "parseInt("+HEX_STRING+")", HEX_VALUE, parseInt(HEX_STRING) ); From f3866c0a679ba2df39f8033ceff8dfa4ddcde5d1 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 20 Jul 2010 12:46:58 -0500 Subject: [PATCH 10/19] Bug 580200 - Assertion failure, or duplicated enumeration, enumerating own properties of proxy returning duplicated property names. r=jorendorff --HG-- extra : rebase_source : 19d4bf112e0dfdb5ed619f9d6e1f72b72a3e0801 --- js/src/jsiter.cpp | 7 ++-- js/src/tests/js1_8_5/extensions/jstests.list | 1 + .../proxy-enumerateOwn-duplicates.js | 32 +++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 js/src/tests/js1_8_5/extensions/proxy-enumerateOwn-duplicates.js diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 84105c81110d..62f81dc948d6 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -206,7 +206,7 @@ Enumerate(JSContext *cx, JSObject *obj, JSObject *pobj, jsid id, JS_ASSERT(JSID_IS_INT(id) || JSID_IS_ATOM(id)); IdSet::AddPtr p = ht.lookupForAdd(id); - JS_ASSERT_IF(obj == pobj, !p); + JS_ASSERT_IF(obj == pobj && !obj->isProxy(), !p); /* If we've already seen this, we definitely won't add it. */ if (JS_UNLIKELY(!!p)) @@ -214,9 +214,10 @@ Enumerate(JSContext *cx, JSObject *obj, JSObject *pobj, jsid id, /* * It's not necessary to add properties to the hash table at the end of the - * prototype chain. + * prototype chain -- but a proxy might return duplicated properties, so + * always add for them. */ - if (pobj->getProto() && !ht.add(p, id)) + if ((pobj->getProto() || pobj->isProxy()) && !ht.add(p, id)) return false; if (JS_UNLIKELY(flags & JSITER_OWNONLY)) { diff --git a/js/src/tests/js1_8_5/extensions/jstests.list b/js/src/tests/js1_8_5/extensions/jstests.list index 39a75c128162..cdaa03c7874f 100644 --- a/js/src/tests/js1_8_5/extensions/jstests.list +++ b/js/src/tests/js1_8_5/extensions/jstests.list @@ -10,3 +10,4 @@ skip-if(!xulRuntime.shell) script worker-terminate.js skip-if(!xulRuntime.shell) script worker-timeout.js script scripted-proxies.js script array-length-protochange.js +script proxy-enumerateOwn-duplicates.js diff --git a/js/src/tests/js1_8_5/extensions/proxy-enumerateOwn-duplicates.js b/js/src/tests/js1_8_5/extensions/proxy-enumerateOwn-duplicates.js new file mode 100644 index 000000000000..c426929573a0 --- /dev/null +++ b/js/src/tests/js1_8_5/extensions/proxy-enumerateOwn-duplicates.js @@ -0,0 +1,32 @@ +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/licenses/publicdomain/ + +var gTestfile = 'proxy-enumerateOwn-duplicates.js'; +//----------------------------------------------------------------------------- +var BUGNUMBER = 580200; +var summary = + 'Assertion failure enumerating own properties of proxy returning ' + + 'duplicated own property name'; +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +var x = Proxy.create({ enumerateOwn: function() { return ["0","0"]; } }, [1,2]); +var ax = Object.getOwnPropertyNames(x); +assertEq(ax.length, 1, "array: " + ax); +assertEq(ax[0], "0"); + +var p = Proxy.create({ enumerateOwn: function() { return ["1","1"]; } }, null); +var ap = Object.getOwnPropertyNames(p); +assertEq(ap.length, 1, "array: " + ap); +assertEq(ap[0], "1"); + + +/******************************************************************************/ + +if (typeof reportCompare === "function") + reportCompare(true, true); + +print("All tests passed!"); From 356128292d35a76ee5d1b1cd237d7fbe6be00511 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Fri, 23 Jul 2010 23:56:16 +0200 Subject: [PATCH 11/19] bug 576596 - follow up to rename js_HasInstance into HasInstance --HG-- extra : rebase_source : 2f4d4db7a849d3683a85f971e88855b1e72d8398 --- js/src/jsapi.cpp | 2 +- js/src/jsinterp.cpp | 4 ++-- js/src/jsinterp.h | 2 +- js/src/jstracer.cpp | 9 +++++---- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 9d101e496b2b..d7b2aa2cbe34 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2777,7 +2777,7 @@ JS_PUBLIC_API(JSBool) JS_HasInstance(JSContext *cx, JSObject *obj, jsval v, JSBool *bp) { assertSameCompartment(cx, obj, v); - return js_HasInstance(cx, obj, Valueify(&v), bp); + return HasInstance(cx, obj, Valueify(&v), bp); } JS_PUBLIC_API(void *) diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 2540e6662996..e7eb4902cbd2 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -1031,7 +1031,7 @@ CheckRedeclaration(JSContext *cx, JSObject *obj, jsid id, uintN attrs, } JSBool -js_HasInstance(JSContext *cx, JSObject *obj, const Value *v, JSBool *bp) +HasInstance(JSContext *cx, JSObject *obj, const Value *v, JSBool *bp) { Class *clasp = obj->getClass(); if (clasp->hasInstance) @@ -6319,7 +6319,7 @@ BEGIN_CASE(JSOP_INSTANCEOF) JSObject *obj = &rref.toObject(); const Value &lref = regs.sp[-2]; JSBool cond = JS_FALSE; - if (!js_HasInstance(cx, obj, &lref, &cond)) + if (!HasInstance(cx, obj, &lref, &cond)) goto error; regs.sp--; regs.sp[-1].setBoolean(cond); diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 7ed3f9fb1f15..06929c2fa039 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -369,7 +369,7 @@ InstanceOf(JSContext *cx, JSObject *obj, Class *clasp, Value *argv) } extern JSBool -js_HasInstance(JSContext *cx, JSObject *obj, const js::Value *v, JSBool *bp); +HasInstance(JSContext *cx, JSObject *obj, const js::Value *v, JSBool *bp); inline void * GetInstancePrivate(JSContext *cx, JSObject *obj, Class *clasp, Value *argv) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 3ea3e661ee54..33848b44d850 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -14654,15 +14654,16 @@ TraceRecorder::record_JSOP_IN() } static JSBool FASTCALL -HasInstance(JSContext* cx, JSObject* ctor, ValueArgType arg) +HasInstanceOnTrace(JSContext* cx, JSObject* ctor, ValueArgType arg) { const Value &argref = ValueArgToConstRef(arg); JSBool result = JS_FALSE; - if (!js_HasInstance(cx, ctor, &argref, &result)) + if (!HasInstance(cx, ctor, &argref, &result)) SetBuiltinError(cx); return result; } -JS_DEFINE_CALLINFO_3(static, BOOL_FAIL, HasInstance, CONTEXT, OBJECT, VALUE, 0, ACC_STORE_ANY) +JS_DEFINE_CALLINFO_3(static, BOOL_FAIL, HasInstanceOnTrace, CONTEXT, OBJECT, VALUE, 0, + ACC_STORE_ANY) JS_REQUIRES_STACK AbortableRecordingStatus TraceRecorder::record_JSOP_INSTANCEOF() @@ -14677,7 +14678,7 @@ TraceRecorder::record_JSOP_INSTANCEOF() enterDeepBailCall(); LIns* args[] = {val_ins, get(&ctor), cx_ins}; - stack(-2, lir->insCall(&HasInstance_ci, args)); + stack(-2, lir->insCall(&HasInstanceOnTrace_ci, args)); LIns* status_ins = lir->insLoad(LIR_ldi, lirbuf->state, offsetof(TracerState, builtinStatus), ACC_OTHER); From 7bc0fea77af5bcd4f3f918d6a1a70dbd87040798 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 15:17:42 -0700 Subject: [PATCH 12/19] Consolidate GC heuristics (580803, r=igor). --- dom/base/nsJSEnvironment.cpp | 20 +---- js/src/jsapi.cpp | 67 +++-------------- js/src/jscntxt.cpp | 83 +++++---------------- js/src/jscntxt.h | 139 ++++++++++++----------------------- js/src/jsgc.cpp | 120 +++++------------------------- js/src/jsgc.h | 10 --- js/src/jsscope.cpp | 6 +- js/src/jstracer.cpp | 21 ------ 8 files changed, 97 insertions(+), 369 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index 9e411f20009f..e65ba6be034e 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -903,20 +903,6 @@ DumpString(const nsAString &str) } #endif -static void -MaybeGC(JSContext *cx) -{ - size_t bytes = cx->runtime->gcBytes; - size_t lastBytes = cx->runtime->gcLastBytes; - if ((bytes > 8192 && bytes > lastBytes * 16) -#ifdef DEBUG - || cx->runtime->gcZeal > 0 -#endif - ) { - JS_GC(cx); - } -} - static already_AddRefed GetPromptFromContext(nsJSContext* ctx) { @@ -955,7 +941,7 @@ nsJSContext::DOMOperationCallback(JSContext *cx) PRTime callbackTime = ctx->mOperationCallbackTime; PRTime modalStateTime = ctx->mModalStateTime; - MaybeGC(cx); + JS_MaybeGC(cx); // Now restore the callback time and count, in case they got reset. ctx->mOperationCallbackTime = callbackTime; @@ -3511,12 +3497,12 @@ nsJSContext::ScriptEvaluated(PRBool aTerminated) #ifdef JS_GC_ZEAL if (mContext->runtime->gcZeal >= 2) { - MaybeGC(mContext); + JS_MaybeGC(mContext); } else #endif if (mNumEvaluations > 20) { mNumEvaluations = 0; - MaybeGC(mContext); + JS_MaybeGC(mContext); } if (aTerminated) { diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index d7b2aa2cbe34..28cc561b2bc6 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1849,7 +1849,7 @@ JS_free(JSContext *cx, void *p) JS_PUBLIC_API(void) JS_updateMallocCounter(JSContext *cx, size_t nbytes) { - return cx->updateMallocCounter(nbytes); + cx->runtime->updateMallocCounter(nbytes); } JS_PUBLIC_API(char *) @@ -2445,65 +2445,16 @@ JS_MaybeGC(JSContext *cx) rt = cx->runtime; -#ifdef JS_GC_ZEAL - if (rt->gcZeal > 0) { - JS_GC(cx); - return; - } -#endif - bytes = rt->gcBytes; lastBytes = rt->gcLastBytes; - /* - * We run the GC if we used all available free GC cells and had to - * allocate extra 1/3 of GC arenas since the last run of GC, or if - * we have malloc'd more bytes through JS_malloc than we were told - * to allocate by JS_NewRuntime. - * - * The reason for - * bytes > 4/3 lastBytes - * condition is the following. Bug 312238 changed bytes and lastBytes - * to mean the total amount of memory that the GC uses now and right - * after the last GC. - * - * Before the bug the variables meant the size of allocated GC things - * now and right after the last GC. That size did not include the - * memory taken by free GC cells and the condition was - * bytes > 3/2 lastBytes. - * That is, we run the GC if we have half again as many bytes of - * GC-things as the last time we GC'd. To be compatible we need to - * express that condition through the new meaning of bytes and - * lastBytes. - * - * We write the original condition as - * B*(1-F) > 3/2 Bl*(1-Fl) - * where B is the total memory size allocated by GC and F is the free - * cell density currently and Sl and Fl are the size and the density - * right after GC. The density by definition is memory taken by free - * cells divided by total amount of memory. In other words, B and Bl - * are bytes and lastBytes with the new meaning and B*(1-F) and - * Bl*(1-Fl) are bytes and lastBytes with the original meaning. - * - * Our task is to exclude F and Fl from the last statement. According - * to the stats from bug 331966 comment 23, Fl is about 10-25% for a - * typical run of the browser. It means that the original condition - * implied that we did not run GC unless we exhausted the pool of - * free cells. Indeed if we still have free cells, then B == Bl since - * we did not yet allocated any new arenas and the condition means - * 1 - F > 3/2 (1-Fl) or 3/2Fl > 1/2 + F - * That implies 3/2 Fl > 1/2 or Fl > 1/3. That cannot be fulfilled - * for the state described by the stats. So we can write the original - * condition as: - * F == 0 && B > 3/2 Bl(1-Fl) - * Again using the stats we see that Fl is about 11% when the browser - * starts up and when we are far from hitting rt->gcMaxBytes. With - * this F we have - * F == 0 && B > 3/2 Bl(1-0.11) - * or approximately F == 0 && B > 4/3 Bl. - */ - if ((bytes > 8192 && bytes > lastBytes + lastBytes / 3) || - rt->isGCMallocLimitReached()) { + if (rt->gcIsNeeded || +#ifdef JS_GC_ZEAL + rt->gcZeal > 0 || +#endif + (bytes > 8192 && bytes > lastBytes * 16) || + bytes >= rt->gcTriggerBytes || + rt->overQuota()) { JS_GC(cx); } } @@ -2624,7 +2575,7 @@ JS_NewExternalString(JSContext *cx, jschar *chars, size_t length, intN type) if (!str) return NULL; str->initFlat(chars, length); - cx->updateMallocCounter((length + 1) * sizeof(jschar)); + cx->runtime->updateMallocCounter((length + 1) * sizeof(jschar)); return str; } diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 505464736a80..b2c7a3826bc6 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -738,7 +738,6 @@ js_PurgeThreads(JSContext *cx) e.removeFront(); } else { thread->data.purge(cx); - thread->gcThreadMallocBytes = JS_GC_THREAD_MALLOC_LIMIT; } } #else @@ -1890,43 +1889,36 @@ js_InvokeOperationCallback(JSContext *cx) JS_THREAD_DATA(cx)->operationCallbackFlag = 0; /* - * Unless we are going to run the GC, we automatically yield the current - * context every time the operation callback is hit since we might be - * called as a result of an impending GC, which would deadlock if we do - * not yield. Operation callbacks are supposed to happen rarely (seconds, - * not milliseconds) so it is acceptable to yield at every callback. + * Ideally this should never be hit. The embedding should call JS_MaybeGC() + * to schedule preemptive GCs. */ JSRuntime *rt = cx->runtime; - if (rt->gcIsNeeded) { - js_GC(cx, GC_NORMAL); - - /* - * On trace we can exceed the GC quota, see comments in NewGCArena. So - * we check the quota and report OOM here when we are off trace. - */ - bool delayedOutOfMemory; - JS_LOCK_GC(rt); - delayedOutOfMemory = (rt->gcBytes > rt->gcMaxBytes); - JS_UNLOCK_GC(rt); - if (delayedOutOfMemory) { + if (rt->gcIsNeeded || rt->overQuota()) { + JS_GC(cx); + /* If we are still over quota after the GC, report an error. */ + if (rt->overQuota()) { js_ReportOutOfMemory(cx); return false; } } -#ifdef JS_THREADSAFE - else { - JS_YieldRequest(cx); - } -#endif - JSOperationCallback cb = cx->operationCallback; + /* + * Automatically yield the current context every time the operation callback + * is hit since we might be called as a result of an impending GC, which + * would deadlock if we do not yield. Operation callbacks are supposed to + * happen rarely (seconds, not milliseconds) so it is acceptable to yield at + * every callback. + */ +#ifdef JS_THREADSAFE + JS_YieldRequest(cx); +#endif /* * Important: Additional callbacks can occur inside the callback handler * if it re-enters the JS engine. The embedding must ensure that the * callback is disconnected before attempting such re-entry. */ - + JSOperationCallback cb = cx->operationCallback; return !cb || cb(cx); } @@ -2155,47 +2147,6 @@ JSContext::containingSegment(const JSStackFrame *target) return NULL; } -void -JSContext::checkMallocGCPressure(void *p) -{ - if (!p) { - js_ReportOutOfMemory(this); - return; - } - -#ifdef JS_THREADSAFE - JS_ASSERT(thread); - JS_ASSERT(thread->gcThreadMallocBytes <= 0); - ptrdiff_t n = JS_GC_THREAD_MALLOC_LIMIT - thread->gcThreadMallocBytes; - thread->gcThreadMallocBytes = JS_GC_THREAD_MALLOC_LIMIT; - - AutoLockGC lock(runtime); - runtime->gcMallocBytes -= n; - - /* - * Trigger the GC on memory pressure but only if we are inside a request - * and not inside a GC. - */ - if (runtime->isGCMallocLimitReached() && requestDepth != 0) -#endif - { - if (!runtime->gcRunning) { - JS_ASSERT(runtime->isGCMallocLimitReached()); - runtime->gcMallocBytes = -1; - - /* - * Empty the GC free lists to trigger a last-ditch GC when any GC - * thing is allocated later on this thread. This makes unnecessary - * to check for the memory pressure on the fast path of the GC - * allocator. We cannot touch the free lists on other threads as - * their manipulation is not thread-safe. - */ - JS_THREAD_DATA(this)->gcFreeLists.purge(); - js_TriggerGC(this, true); - } - } -} - bool JSContext::isConstructing() { diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 914b44f860a1..5a7b706bbccc 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -976,13 +976,6 @@ struct JSThreadData { /* Keeper of the contiguous stack used by all contexts in this thread. */ js::StackSpace stackSpace; - /* - * Flag indicating that we are waiving any soft limits on the GC heap - * because we want allocations to be infallible (except when we hit - * a hard quota). - */ - bool waiveGCQuota; - /* * The GSN cache is per thread since even multi-cx-per-thread embeddings * do not interleave js_GetSrcNote calls. @@ -1066,12 +1059,6 @@ struct JSThread { /* Indicates that the thread is waiting in ClaimTitle from jslock.cpp. */ JSTitle *titleToShare; - /* - * Thread-local version of JSRuntime.gcMallocBytes to avoid taking - * locks on each JS_malloc. - */ - ptrdiff_t gcThreadMallocBytes; - /* * This thread is inside js_GC, either waiting until it can start GC, or * waiting for GC to finish on another thread. This thread holds no locks; @@ -1090,13 +1077,6 @@ struct JSThread { JSThreadData data; }; -/* - * Only when JSThread::gcThreadMallocBytes exhausts the following limit we - * update JSRuntime::gcMallocBytes. - * . - */ -const size_t JS_GC_THREAD_MALLOC_LIMIT = 1 << 19; - #define JS_THREAD_DATA(cx) (&(cx)->thread->data) extern JSThread * @@ -1305,7 +1285,7 @@ struct JSRuntime { * Malloc counter to measure memory pressure for GC scheduling. It runs * from gcMaxMallocBytes down to zero. */ - ptrdiff_t gcMallocBytes; + volatile ptrdiff_t gcMallocBytes; /* See comments before DelayMarkingChildren is jsgc.cpp. */ JSGCArena *gcUnmarkedArenaStackTop; @@ -1587,16 +1567,35 @@ struct JSRuntime { void setGCTriggerFactor(uint32 factor); void setGCLastBytes(size_t lastBytes); - void* malloc(size_t bytes) { return ::js_malloc(bytes); } + bool gcQuotaReached() { + return gcBytes >= gcMaxBytes; + } - void* calloc(size_t bytes) { return ::js_calloc(bytes); } + void updateMallocCounter(size_t bytes) { + gcMallocBytes -= bytes; /* We tolerate races and lost counts here. */ + } - void* realloc(void* p, size_t bytes) { return ::js_realloc(p, bytes); } + bool mallocQuotaReached() { + return gcMallocBytes <= 0; + } + + bool overQuota() { + return gcQuotaReached() || mallocQuotaReached(); + } + + void* malloc(size_t bytes) { updateMallocCounter(bytes); return ::js_malloc(bytes); } + + void* calloc(size_t bytes) { updateMallocCounter(bytes); return ::js_calloc(bytes); } + + void* realloc(void* p, size_t bytes) { + void* q = ::js_realloc(p, bytes); + if (p != q) + updateMallocCounter(bytes); + return q; + } void free(void* p) { ::js_free(p); } - bool isGCMallocLimitReached() const { return gcMallocBytes <= 0; } - void resetGCMallocBytes() { gcMallocBytes = ptrdiff_t(gcMaxMallocBytes); } void setGCMaxMallocBytes(size_t value) { @@ -1690,6 +1689,9 @@ struct JSRegExpStatics { void clear(); }; +extern JS_FRIEND_API(void) +js_ReportOutOfMemory(JSContext *cx); + struct JSContext { explicit JSContext(JSRuntime *rt); @@ -1970,76 +1972,40 @@ struct JSContext js::BackgroundSweepTask *gcSweepTask; #endif - ptrdiff_t &getMallocCounter() { -#ifdef JS_THREADSAFE - return thread->gcThreadMallocBytes; -#else - return runtime->gcMallocBytes; -#endif - } - - /* - * Call this after allocating memory held by GC things, to update memory - * pressure counters or report the OOM error if necessary. - */ - inline void updateMallocCounter(void *p, size_t nbytes) { - JS_ASSERT(ptrdiff_t(nbytes) >= 0); - ptrdiff_t &counter = getMallocCounter(); - counter -= ptrdiff_t(nbytes); - if (!p || counter <= 0) - checkMallocGCPressure(p); - } - - /* - * Call this after successfully allocating memory held by GC things, to - * update memory pressure counters. - */ - inline void updateMallocCounter(size_t nbytes) { - JS_ASSERT(ptrdiff_t(nbytes) >= 0); - ptrdiff_t &counter = getMallocCounter(); - counter -= ptrdiff_t(nbytes); - if (counter <= 0) { - /* - * Use 1 as an arbitrary non-null pointer indicating successful - * allocation. - */ - checkMallocGCPressure(reinterpret_cast(jsuword(1))); + inline void triggerGC() { + if (!runtime->gcIsNeeded) { + runtime->gcIsNeeded = true; + JS_TriggerOperationCallback(this); } } + inline void *reportIfOutOfMemory(void *p) { + if (p) { + if (runtime->mallocQuotaReached()) + triggerGC(); + return p; + } + js_ReportOutOfMemory(this); + return NULL; + } + inline void* malloc(size_t bytes) { JS_ASSERT(bytes != 0); - void *p = runtime->malloc(bytes); - updateMallocCounter(p, bytes); - return p; + return reportIfOutOfMemory(runtime->malloc(bytes)); } inline void* mallocNoReport(size_t bytes) { JS_ASSERT(bytes != 0); - void *p = runtime->malloc(bytes); - if (!p) - return NULL; - updateMallocCounter(bytes); - return p; + return runtime->malloc(bytes); } inline void* calloc(size_t bytes) { JS_ASSERT(bytes != 0); - void *p = runtime->calloc(bytes); - updateMallocCounter(p, bytes); - return p; + return reportIfOutOfMemory(runtime->calloc(bytes)); } inline void* realloc(void* p, size_t bytes) { - void *orig = p; - p = runtime->realloc(p, bytes); - - /* - * For compatibility we do not account for realloc that increases - * previously allocated memory. - */ - updateMallocCounter(p, orig ? 0 : bytes); - return p; + return reportIfOutOfMemory(runtime->realloc(p, bytes)); } inline void free(void* p) { @@ -2106,16 +2072,6 @@ struct JSContext #else void assertValidStackDepth(uintN /*depth*/) {} #endif - -private: - - /* - * The allocation code calls the function to indicate either OOM failure - * when p is null or that a memory pressure counter has reached some - * threshold when p is not null. The function takes the pointer and not - * a boolean flag to minimize the amount of code in its inlined callers. - */ - JS_FRIEND_API(void) checkMallocGCPressure(void *p); }; JS_ALWAYS_INLINE JSObject * @@ -2874,9 +2830,6 @@ js_ExpandErrorArguments(JSContext *cx, JSErrorCallback callback, bool charArgs, va_list ap); #endif -extern void -js_ReportOutOfMemory(JSContext *cx); - /* * Report that cx->scriptStackQuota is exhausted. */ diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index fcc6325d178a..e9e9ff7a950e 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -641,19 +641,8 @@ static JSGCArena * NewGCArena(JSContext *cx) { JSRuntime *rt = cx->runtime; - if (!JS_THREAD_DATA(cx)->waiveGCQuota && rt->gcBytes >= rt->gcMaxBytes) { - /* - * FIXME bug 524051 We cannot run a last-ditch GC on trace for now, so - * just pretend we are out of memory which will throw us off trace and - * we will re-try this code path from the interpreter. - */ - if (!JS_ON_TRACE(cx)) - return NULL; - js_TriggerGC(cx, true); - } size_t nchunks = rt->gcChunks.length(); - JSGCChunkInfo *ci; for (;; ++rt->gcChunkCursor) { if (rt->gcChunkCursor == nchunks) { @@ -664,6 +653,7 @@ NewGCArena(JSContext *cx) if (ci->numFreeArenas != 0) break; } + if (!ci) { if (!rt->gcChunks.reserve(nchunks + 1)) return NULL; @@ -1723,41 +1713,6 @@ JSGCFreeLists::moveTo(JSGCFreeLists *another) JS_ASSERT(isEmpty()); } -static inline bool -IsGCThresholdReached(JSRuntime *rt) -{ -#ifdef JS_GC_ZEAL - if (rt->gcZeal >= 1) - return true; -#endif - - /* - * Since the initial value of the gcLastBytes parameter is not equal to - * zero (see the js_InitGC function) the return value is false when - * the gcBytes value is close to zero at the JS engine start. - */ - return rt->isGCMallocLimitReached() || rt->gcBytes >= rt->gcTriggerBytes; -} - -static void -LastDitchGC(JSContext *cx) -{ - JS_ASSERT(!JS_ON_TRACE(cx)); - - /* The last ditch GC preserves weak roots and all atoms. */ - AutoPreserveWeakRoots save(cx); - AutoKeepAtoms keep(cx->runtime); - - /* - * Keep rt->gcLock across the call into the GC so we don't starve and - * lose to racing threads who deplete the heap just after the GC has - * replenished it (or has synchronized with a racing GC that collected a - * bunch of garbage). This unfair scheduling can happen on certain - * operating systems. For the gory details, see bug 162779. - */ - js_GC(cx, GC_LOCK_HELD); -} - static JSGCThing * RefillFinalizableFreeList(JSContext *cx, unsigned thingKind) { @@ -1774,45 +1729,28 @@ RefillFinalizableFreeList(JSContext *cx, unsigned thingKind) return NULL; } - bool canGC = !JS_ON_TRACE(cx) && !JS_THREAD_DATA(cx)->waiveGCQuota; - bool doGC = canGC && IsGCThresholdReached(rt); arenaList = &rt->gcArenaList[thingKind]; - for (;;) { - if (doGC) { - LastDitchGC(cx); - METER(cx->runtime->gcStats.arenaStats[thingKind].retry++); - canGC = false; - - /* - * The JSGC_END callback can legitimately allocate new GC - * things and populate the free list. If that happens, just - * return that list head. - */ - JSGCThing *freeList = JS_THREAD_DATA(cx)->gcFreeLists.finalizables[thingKind]; - if (freeList) - return freeList; + while ((a = arenaList->cursor) != NULL) { + JSGCArenaInfo *ainfo = a->getInfo(); + arenaList->cursor = ainfo->prev; + JSGCThing *freeList = ainfo->freeList; + if (freeList) { + ainfo->freeList = NULL; + return freeList; } - - while ((a = arenaList->cursor) != NULL) { - JSGCArenaInfo *ainfo = a->getInfo(); - arenaList->cursor = ainfo->prev; - JSGCThing *freeList = ainfo->freeList; - if (freeList) { - ainfo->freeList = NULL; - return freeList; - } - } - - a = NewGCArena(cx); - if (a) - break; - if (!canGC) { - METER(cx->runtime->gcStats.arenaStats[thingKind].fail++); - return NULL; - } - doGC = true; } + /* + * If we have to allocate a new arena, check whether are bumping + * against our GC heap quota. If so, request a GC to happen soon. + */ + if (rt->gcQuotaReached()) + cx->triggerGC(); + + a = NewGCArena(cx); + if (!a) + return NULL; + /* * Do only minimal initialization of the arena inside the GC lock. We * can do the rest outside the lock because no other threads will see @@ -2527,26 +2465,6 @@ js_TraceRuntime(JSTracer *trc) rt->gcExtraRootsTraceOp(trc, rt->gcExtraRootsData); } -void -js_TriggerGC(JSContext *cx, JSBool gcLocked) -{ - JSRuntime *rt = cx->runtime; - -#ifdef JS_THREADSAFE - JS_ASSERT(cx->requestDepth > 0); -#endif - JS_ASSERT(!rt->gcRunning); - if (rt->gcIsNeeded) - return; - - /* - * Trigger the GC when it is safe to call an operation callback on any - * thread. - */ - rt->gcIsNeeded = JS_TRUE; - js_TriggerAllOperationCallbacks(rt, gcLocked); -} - void js_DestroyScriptsToGC(JSContext *cx, JSThreadData *data) { diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 50fa703542cd..eaeb24d8d042 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -167,16 +167,6 @@ js_TraceRuntime(JSTracer *trc); extern JS_REQUIRES_STACK JS_FRIEND_API(void) js_TraceContext(JSTracer *trc, JSContext *acx); -/* - * Schedule the GC call at a later safe point. - */ -#ifndef JS_THREADSAFE -# define js_TriggerGC(cx, gcLocked) js_TriggerGC (cx) -#endif - -extern void -js_TriggerGC(JSContext *cx, JSBool gcLocked); - /* * Kinds of js_GC invocation. */ diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 39c9c06e3436..9241d160c128 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -86,7 +86,7 @@ js_GenerateShape(JSContext *cx, bool gcLocked) */ rt->shapeGen = SHAPE_OVERFLOW_BIT; shape = SHAPE_OVERFLOW_BIT; - js_TriggerGC(cx, gcLocked); + cx->triggerGC(); } return shape; } @@ -200,7 +200,7 @@ JSScope::createTable(JSContext *cx, bool report) METER(tableAllocFails); return false; } - cx->updateMallocCounter(JS_BIT(sizeLog2) * sizeof(JSScopeProperty *)); + cx->runtime->updateMallocCounter(JS_BIT(sizeLog2) * sizeof(JSScopeProperty *)); hashShift = JS_DHASH_BITS - sizeLog2; for (sprop = lastProp; sprop; sprop = sprop->parent) { @@ -516,7 +516,7 @@ JSScope::changeTable(JSContext *cx, int change) table = newtable; /* Treat the above calloc as a JS_malloc, to match CreateScopeTable. */ - cx->updateMallocCounter(nbytes); + cx->runtime->updateMallocCounter(nbytes); /* Copy only live entries, leaving removed and free ones behind. */ for (oldspp = oldtable; oldsize != 0; oldspp++) { diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 33848b44d850..da9a18ebef36 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2962,7 +2962,6 @@ public: JS_REQUIRES_STACK JS_ALWAYS_INLINE void visitGlobalSlot(Value *vp, unsigned n, unsigned slot) { debug_only_printf(LC_TMTracer, "global%d=", n); - JS_ASSERT(JS_THREAD_DATA(mCx)->waiveGCQuota); NativeToValue(mCx, *vp, *mTypeMap++, &mGlobal[slot]); } }; @@ -2996,7 +2995,6 @@ public: JS_REQUIRES_STACK JS_ALWAYS_INLINE bool visitStackSlots(Value *vp, size_t count, JSStackFrame* fp) { - JS_ASSERT(JS_THREAD_DATA(mCx)->waiveGCQuota); for (size_t i = 0; i < count; ++i) { if (vp == mStop) return false; @@ -3012,7 +3010,6 @@ public: JS_REQUIRES_STACK JS_ALWAYS_INLINE bool visitFrameObjPtr(JSObject **p, JSStackFrame* fp) { - JS_ASSERT(JS_THREAD_DATA(mCx)->waiveGCQuota); if ((Value *)p == mStop) return false; debug_only_printf(LC_TMTracer, "%s%u=", stackSlotKind(), 0); @@ -6714,21 +6711,6 @@ ExecuteTree(JSContext* cx, TreeFragment* f, uintN& inlineCallCount, return ok; } -class Guardian { - bool *flagp; -public: - Guardian(bool *flagp) { - this->flagp = flagp; - JS_ASSERT(!*flagp); - *flagp = true; - } - - ~Guardian() { - JS_ASSERT(*flagp); - *flagp = false; - } -}; - static JS_FORCES_STACK void LeaveTree(TraceMonitor *tm, TracerState& state, VMSideExit* lr) { @@ -6736,9 +6718,6 @@ LeaveTree(TraceMonitor *tm, TracerState& state, VMSideExit* lr) JSContext* cx = state.cx; - /* Temporary waive the soft GC quota to make sure LeaveTree() doesn't fail. */ - Guardian waiver(&JS_THREAD_DATA(cx)->waiveGCQuota); - FrameInfo** callstack = state.callstackBase; double* stack = state.stackBase; From 4f316be2538f4288e0873c5058292e37f9eea746 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 15:21:13 -0700 Subject: [PATCH 13/19] Trigger all operation callbacks from triggerGC (follow-up for 580803). --- js/src/jscntxt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 5a7b706bbccc..edaa1252d8d6 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1975,7 +1975,7 @@ struct JSContext inline void triggerGC() { if (!runtime->gcIsNeeded) { runtime->gcIsNeeded = true; - JS_TriggerOperationCallback(this); + JS_TriggerAllOperationCallbacks(runtime); } } From 3b3f1f1e4195b55c9530df33df53a86bd7a9a2a2 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 15:25:42 -0700 Subject: [PATCH 14/19] Don't use broken js_NewArrayObjectWithCapacity API (581264, r=bz,jst,dwitte). --- content/base/src/nsFrameMessageManager.cpp | 13 +++-------- .../canvas/src/nsCanvasRenderingContext2D.cpp | 8 ------- js/src/ctypes/CTypes.cpp | 23 +++++++++---------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp index 4315f20b1e4e..0d539055b789 100644 --- a/content/base/src/nsFrameMessageManager.cpp +++ b/content/base/src/nsFrameMessageManager.cpp @@ -222,26 +222,20 @@ nsFrameMessageManager::SendSyncMessage() JSAutoRequest ar(ctx); PRUint32 len = retval.Length(); - jsval* dest = nsnull; - JSObject* dataArray = js_NewArrayObjectWithCapacity(ctx, len, &dest); + JSObject* dataArray = js_NewArrayObject(ctx, len, NULL); NS_ENSURE_TRUE(dataArray, NS_ERROR_OUT_OF_MEMORY); - nsAutoGCRoot arrayGCRoot(&dataArray, &rv); - NS_ENSURE_SUCCESS(rv, rv); for (PRUint32 i = 0; i < len; ++i) { - dest[i] = JSVAL_NULL; if (!retval[i].Length()) continue; jsval ret = JSVAL_VOID; - nsAutoGCRoot root(&ret, &rv); - NS_ENSURE_SUCCESS(rv, rv); JSONParser* parser = JS_BeginJSONParse(ctx, &ret); JSBool ok = JS_ConsumeJSONText(ctx, parser, (jschar*)retval[i].get(), (uint32)retval[i].Length()); ok = JS_FinishJSONParse(ctx, parser, JSVAL_NULL) && ok; if (ok) { - dest[i] = ret; + NS_ENSURE_TRUE(JS_SetElement(ctx, dataArray, i, &ret), NS_ERROR_OUT_OF_MEMORY); } } @@ -350,10 +344,9 @@ nsFrameMessageManager::ReceiveMessage(nsISupports* aTarget, // To keep compatibility with e10s message manager, // define empty objects array. if (!aObjectsArray) { - jsval* dest = nsnull; // Because we want JS messages to have always the same properties, // create array even if len == 0. - aObjectsArray = js_NewArrayObjectWithCapacity(ctx, 0, &dest); + aObjectsArray = js_NewArrayObject(ctx, 0, NULL); if (!aObjectsArray) { return false; } diff --git a/content/canvas/src/nsCanvasRenderingContext2D.cpp b/content/canvas/src/nsCanvasRenderingContext2D.cpp index 7529f178da26..d0f19d88433d 100644 --- a/content/canvas/src/nsCanvasRenderingContext2D.cpp +++ b/content/canvas/src/nsCanvasRenderingContext2D.cpp @@ -3825,14 +3825,6 @@ nsCanvasRenderingContext2D::AsyncDrawXULElement(nsIDOMXULElement* aElem, float a // // device pixel getting/setting // -extern "C" { -#include "jstypes.h" -JS_FRIEND_API(JSBool) -js_CoerceArrayToCanvasImageData(JSObject *obj, jsuint offset, jsuint count, - JSUint8 *dest); -JS_FRIEND_API(JSObject *) -js_NewArrayObjectWithCapacity(JSContext *cx, jsuint capacity, jsval **vector); -} void nsCanvasRenderingContext2D::EnsureUnpremultiplyTable() { diff --git a/js/src/ctypes/CTypes.cpp b/js/src/ctypes/CTypes.cpp index e882116bba6a..f623dbc770cb 100644 --- a/js/src/ctypes/CTypes.cpp +++ b/js/src/ctypes/CTypes.cpp @@ -3911,16 +3911,15 @@ ExtractStructField(JSContext* cx, jsval val, JSObject** typeObj) return name; } -// For a struct field with 'name' and 'type', add an element to field -// descriptor array 'arrayObj' of the form { name : type }. +// For a struct field with 'name' and 'type', add an element of the form +// { name : type }. static JSBool AddFieldToArray(JSContext* cx, - JSObject* arrayObj, jsval* element, JSString* name, JSObject* typeObj) { - JSObject* fieldObj = JS_NewObject(cx, NULL, NULL, arrayObj); + JSObject* fieldObj = JS_NewObject(cx, NULL, NULL, NULL); if (!fieldObj) return false; @@ -4325,22 +4324,22 @@ StructType::BuildFieldsArray(JSContext* cx, JSObject* obj) size_t len = fields->count(); // Prepare a new array for the 'fields' property of the StructType. - jsval* fieldsVec; - JSObject* fieldsProp = - js_NewArrayObjectWithCapacity(cx, len, &fieldsVec); - if (!fieldsProp) + Array fieldsVec; + if (!fieldsVec.resize(len)) return NULL; - js::AutoObjectRooter root(cx, fieldsProp); - JS_ASSERT(len == 0 || fieldsVec); for (FieldInfoHash::Range r = fields->all(); !r.empty(); r.popFront()) { const FieldInfoHash::Entry& entry = r.front(); // Add the field descriptor to the array. - if (!AddFieldToArray(cx, fieldsProp, &fieldsVec[entry.value.mIndex], - entry.key, entry.value.mType)) + if (!AddFieldToArray(cx, &fieldsVec[entry.value.mIndex], + entry.key, entry.value.mType)) return NULL; } + JSObject* fieldsProp = JS_NewArrayObject(cx, len, fieldsVec.begin()); + if (!fieldsProp) + return NULL; + // Seal the fields array. if (!JS_SealObject(cx, fieldsProp, JS_FALSE)) return NULL; From 283b5621fc10d125085608b80ee1dcbfa6930cd4 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 15:29:02 -0700 Subject: [PATCH 15/19] Always allocate dslots for dense arrays (580877, r=njn). --- js/src/jsarray.cpp | 174 +++++++++++++++++------------------------- js/src/jsbuiltins.h | 1 - js/src/jsobj.h | 7 +- js/src/jsobjinlines.h | 3 +- js/src/jstracer.cpp | 13 +--- 5 files changed, 79 insertions(+), 119 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index c30c2c629cad..6332ffe06d05 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -309,21 +309,19 @@ BigIndexToId(JSContext *cx, JSObject *obj, jsuint index, JSBool createAtom, } bool -JSObject::resizeDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap, - bool initializeAllSlots) +JSObject::growDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap) { JS_ASSERT(isDenseArray()); - - if (newcap == 0) { - freeDenseArrayElements(cx); - return JS_TRUE; - } + JS_ASSERT(newcap >= ARRAY_CAPACITY_MIN); + JS_ASSERT(newcap >= oldcap); if (newcap > MAX_DSLOTS_LENGTH32) { - js_ReportAllocationOverflow(cx); + if (!JS_ON_TRACE(cx)) + js_ReportAllocationOverflow(cx); return JS_FALSE; } + /* dslots can be briefly NULL during array creation */ Value *slots = dslots ? dslots - 1 : NULL; Value *newslots = (Value *) cx->realloc(slots, (size_t(newcap) + 1) * sizeof(Value)); if (!newslots) @@ -332,17 +330,15 @@ JSObject::resizeDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap, dslots = newslots + 1; setDenseArrayCapacity(newcap); - if (initializeAllSlots) { - Value *base = addressOfDenseArrayElement(0); - for (Value *vp = base + oldcap, *end = base + newcap; vp < end; ++vp) - vp->setMagic(JS_ARRAY_HOLE); - } + Value *base = addressOfDenseArrayElement(0); + for (Value *vp = base + oldcap, *end = base + newcap; vp < end; ++vp) + vp->setMagic(JS_ARRAY_HOLE); return true; } bool -JSObject::ensureDenseArrayElements(JSContext *cx, uint32 newcap, bool initializeAllSlots) +JSObject::ensureDenseArrayElements(JSContext *cx, uint32 newcap) { /* * When a dense array with CAPACITY_DOUBLING_MAX or fewer slots needs to @@ -359,7 +355,8 @@ JSObject::ensureDenseArrayElements(JSContext *cx, uint32 newcap, bool initialize */ static const size_t CAPACITY_CHUNK = 1024 * 1024 / sizeof(Value); - uint32 oldcap = getDenseArrayCapacity(); + /* While creating arrays, dslots can be NULL. */ + uint32 oldcap = dslots ? getDenseArrayCapacity() : 0; if (newcap > oldcap) { /* @@ -382,21 +379,39 @@ JSObject::ensureDenseArrayElements(JSContext *cx, uint32 newcap, bool initialize else if (actualCapacity < ARRAY_CAPACITY_MIN) actualCapacity = ARRAY_CAPACITY_MIN; - if (!resizeDenseArrayElements(cx, oldcap, actualCapacity, initializeAllSlots)) + if (!growDenseArrayElements(cx, oldcap, actualCapacity)) return false; - - if (!initializeAllSlots) { - /* - * Initialize the slots caller didn't actually ask for. - */ - for (uint32 i = newcap; i < actualCapacity; i++) { - setDenseArrayElement(i, MagicValue(JS_ARRAY_HOLE)); - } - } } return true; } +bool +JSObject::shrinkDenseArrayElements(JSContext *cx, uint32 newcap) +{ + JS_ASSERT(isDenseArray()); + JS_ASSERT(newcap < getDenseArrayCapacity()); + JS_ASSERT(dslots); + + uint32 fill = newcap; + + if (newcap < ARRAY_CAPACITY_MIN) + newcap = ARRAY_CAPACITY_MIN; + + Value *newslots = (Value *) cx->realloc(dslots - 1, (size_t(newcap) + 1) * sizeof(Value)); + if (!newslots) + return false; + + dslots = newslots + 1; + setDenseArrayCapacity(newcap); + + /* we refuse to shrink below a minimum value, so we have to clear the excess space */ + Value *base = addressOfDenseArrayElement(0); + while (fill < newcap) + base[fill++].setMagic(JS_ARRAY_HOLE); + + return true; +} + static bool ReallyBigIndexToId(JSContext* cx, jsdouble index, jsid* idp) { @@ -635,8 +650,8 @@ array_length_setter(JSContext *cx, JSObject *obj, jsid id, Value *vp) * right of newlen with JS_ARRAY_HOLE. This permits us to disregard * length when reading from arrays as long we are within the capacity. */ - jsuint capacity = obj->getDenseArrayCapacity(); - if (capacity > newlen && !obj->resizeDenseArrayElements(cx, capacity, newlen)) + jsuint oldcap = obj->getDenseArrayCapacity(); + if (oldcap > newlen && !obj->shrinkDenseArrayElements(cx, newlen)) return false; obj->setArrayLength(newlen); } else if (oldlen - newlen < (1 << 24)) { @@ -984,8 +999,10 @@ array_trace(JSTracer *trc, JSObject *obj) JS_ASSERT(obj->isDenseArray()); obj->traceProtoAndParent(trc); - size_t holes = 0; + if (!obj->dslots) + return; + size_t holes = 0; uint32 capacity = obj->getDenseArrayCapacity(); for (uint32 i = 0; i < capacity; i++) { Value v = obj->getDenseArrayElement(i); @@ -1075,7 +1092,8 @@ JSObject::makeDenseArraySlow(JSContext *cx) if (!scope) return JS_FALSE; - uint32 capacity = obj->getDenseArrayCapacity(); + /* For a brief moment the class object has NULL dslots until we slowify it during construction. */ + uint32 capacity = dslots ? obj->getDenseArrayCapacity() : 0; if (capacity) { scope->freeslot = obj->numSlots() + JS_INITIAL_NSLOTS; // XXX: changing the capacity like this is awful. Bug 558263 will remove @@ -1454,20 +1472,14 @@ InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector { JS_ASSERT(obj->isArray()); - if (vector) { - JS_ASSERT(obj->isDenseArray()); - obj->setArrayLength(length); - if (!obj->ensureDenseArrayElements(cx, length)) - return JS_FALSE; - memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value)); - } else { - if (obj->isDenseArray()) { - obj->setArrayLength(length); - } else { - obj->setArrayLength(length); - } - } - return JS_TRUE; + JS_ASSERT(obj->isDenseArray()); + obj->setArrayLength(length); + if (!vector || !length) + return obj->ensureDenseArrayElements(cx, ARRAY_CAPACITY_MIN); + if (!obj->ensureDenseArrayElements(cx, length)) + return false; + memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value)); + return true; } /* @@ -2986,7 +2998,7 @@ js_Array(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value *rval) } JSObject* JS_FASTCALL -js_NewEmptyArray(JSContext* cx, JSObject* proto) +js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len) { JS_ASSERT(proto->isArray()); @@ -2996,39 +3008,9 @@ js_NewEmptyArray(JSContext* cx, JSObject* proto) /* Initialize all fields of JSObject. */ obj->map = const_cast(&SharedArrayMap); - obj->init(&js_ArrayClass, proto, proto->getParent(), NullValue()); - obj->setArrayLength(0); - return obj; -} -#ifdef JS_TRACER -JS_DEFINE_CALLINFO_2(extern, OBJECT, js_NewEmptyArray, CONTEXT, OBJECT, 0, nanojit::ACC_STORE_ANY) -#endif - -JSObject* JS_FASTCALL -js_NewEmptyArrayWithLength(JSContext* cx, JSObject* proto, int32 len) -{ - if (len < 0) - return NULL; - JSObject *obj = js_NewEmptyArray(cx, proto); - if (!obj) - return NULL; obj->setArrayLength(len); - return obj; -} -#ifdef JS_TRACER -JS_DEFINE_CALLINFO_3(extern, OBJECT, js_NewEmptyArrayWithLength, CONTEXT, OBJECT, INT32, 0, - nanojit::ACC_STORE_ANY) -#endif - -JSObject* JS_FASTCALL -js_NewArrayWithSlots(JSContext* cx, JSObject* proto, uint32 len) -{ - JSObject* obj = js_NewEmptyArray(cx, proto); - if (!obj) - return NULL; - obj->setArrayLength(len); - if (!obj->resizeDenseArrayElements(cx, 0, JS_MAX(len, ARRAY_CAPACITY_MIN))) + if (!obj->growDenseArrayElements(cx, 0, JS_MAX(len, ARRAY_CAPACITY_MIN))) return NULL; return obj; } @@ -3037,15 +3019,24 @@ JS_DEFINE_CALLINFO_3(extern, OBJECT, js_NewArrayWithSlots, CONTEXT, OBJECT, UINT nanojit::ACC_STORE_ANY) #endif +JSObject* JS_FASTCALL +js_NewEmptyArray(JSContext* cx, JSObject* proto) +{ + return js_NewArrayWithSlots(cx, proto, 0); +} +#ifdef JS_TRACER +JS_DEFINE_CALLINFO_2(extern, OBJECT, js_NewEmptyArray, CONTEXT, OBJECT, 0, nanojit::ACC_STORE_ANY) +#endif + JSObject * js_InitArrayClass(JSContext *cx, JSObject *obj) { JSObject *proto = js_InitClass(cx, obj, NULL, &js_ArrayClass, js_Array, 1, NULL, array_methods, NULL, array_static_methods); - - /* Initialize the Array prototype object so it gets a length property. */ - if (!proto || !InitArrayObject(cx, proto, 0, NULL)) + if (!proto) return NULL; + proto->setArrayLength(0); + return proto; } @@ -3173,26 +3164,6 @@ js_CoerceArrayToCanvasImageData(JSObject *obj, jsuint offset, jsuint count, return JS_TRUE; } -JS_FRIEND_API(JSObject *) -js_NewArrayObjectWithCapacity(JSContext *cx, uint32_t capacity, jsval **vector) -{ - JSObject *obj = js_NewArrayObject(cx, capacity, NULL); - if (!obj) - return NULL; - - AutoObjectRooter tvr(cx, obj); - if (!obj->ensureDenseArrayElements(cx, capacity, JS_FALSE)) - obj = NULL; - - /* Set/clear newborn root, in case we lost it. */ - cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] = obj; - if (!obj) - return NULL; - - *vector = Jsvalify(obj->getDenseArrayElements()); - return obj; -} - JS_FRIEND_API(JSBool) js_IsDensePrimitiveArray(JSObject *obj) { @@ -3255,14 +3226,9 @@ js_CloneDensePrimitiveArray(JSContext *cx, JSObject *obj, JSObject **clone) vector.append(val); } - jsval *buffer; - *clone = js_NewArrayObjectWithCapacity(cx, jsvalCount, &buffer); + *clone = js_NewArrayObject(cx, jsvalCount, vector.begin()); if (!*clone) return JS_FALSE; - - AutoObjectRooter cloneRoot(cx, *clone); - - memcpy(buffer, vector.begin(), jsvalCount * sizeof (jsval)); (*clone)->setArrayLength(length); return JS_TRUE; diff --git a/js/src/jsbuiltins.h b/js/src/jsbuiltins.h index b0cb4aec1e87..6eec44d7355b 100644 --- a/js/src/jsbuiltins.h +++ b/js/src/jsbuiltins.h @@ -577,7 +577,6 @@ JS_DECLARE_CALLINFO(js_Array_dense_setelem) JS_DECLARE_CALLINFO(js_Array_dense_setelem_int) JS_DECLARE_CALLINFO(js_Array_dense_setelem_double) JS_DECLARE_CALLINFO(js_NewEmptyArray) -JS_DECLARE_CALLINFO(js_NewEmptyArrayWithLength) JS_DECLARE_CALLINFO(js_NewArrayWithSlots) JS_DECLARE_CALLINFO(js_ArrayCompPush_tn) diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 4c0bcd5cd26b..ddc0d2a77f37 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -494,10 +494,9 @@ struct JSObject { inline void setDenseArrayElement(uint32 i, const js::Value &v); inline js::Value *getDenseArrayElements() const; // returns pointer to the Array's elements array - bool resizeDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap, - bool initializeAllSlots = true); - bool ensureDenseArrayElements(JSContext *cx, uint32 newcap, - bool initializeAllSlots = true); + bool growDenseArrayElements(JSContext *cx, uint32 oldcap, uint32 newcap); + bool ensureDenseArrayElements(JSContext *cx, uint32 newcap); + bool shrinkDenseArrayElements(JSContext *cx, uint32 newcap); inline void freeDenseArrayElements(JSContext *cx); inline void voidDenseOnlyArraySlots(); // used when converting a dense array to a slow array diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index da3e2e0c484d..94e3d8f1b225 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -150,7 +150,8 @@ inline uint32 JSObject::getDenseArrayCapacity() const { JS_ASSERT(isDenseArray()); - return dslots ? dslots[-1].toPrivateUint32() : 0; + JS_ASSERT(dslots); + return dslots[-1].toPrivateUint32(); } inline void diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index da9a18ebef36..cae3803d2205 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -11004,7 +11004,7 @@ TraceRecorder::newArray(JSObject* ctor, uint32 argc, Value* argv, Value* rval) } else if (argc == 1 && argv[0].isNumber()) { // arr_ins = js_NewEmptyArray(cx, Array.prototype, length) LIns *args[] = { d2i(get(argv)), proto_ins, cx_ins }; // FIXME: is this 64-bit safe? - arr_ins = lir->insCall(&js_NewEmptyArrayWithLength_ci, args); + arr_ins = lir->insCall(&js_NewArrayWithSlots_ci, args); guard(false, lir->insEqP_0(arr_ins), OOM_EXIT); } else { // arr_ins = js_NewArrayWithSlots(cx, Array.prototype, argc) @@ -13641,18 +13641,15 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ */ LIns* dslots_ins = addName(lir->insLoad(LIR_ldp, obj_ins, offsetof(JSObject, dslots), ACC_OTHER), "dslots"); + LIns* capacity_ins = + addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity"); + jsuint capacity = obj->getDenseArrayCapacity(); bool within = (jsuint(idx) < capacity); if (!within) { - /* Also stay on trace if dslots is NULL. */ - LIns *br = lir->insBranch(LIR_jt, lir->insEqP_0(dslots_ins), NULL); - /* If not idx < capacity, stay on trace (and read value as undefined). */ - LIns* capacity_ins = addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity"); guard(true, lir->ins2(LIR_geui, idx_ins, capacity_ins), exit); - br->setTarget(lir->ins0(LIR_label)); - CHECK_STATUS(guardPrototypeHasNoIndexedProperties(obj, obj_ins, MISMATCH_EXIT)); // Return undefined and indicate that we didn't actually read this (addr_ins). @@ -13662,8 +13659,6 @@ TraceRecorder::denseArrayElement(Value& oval, Value& ival, Value*& vp, LIns*& v_ } /* Guard that index is within capacity. */ - guard(false, lir->insEqP_0(dslots_ins), exit); - LIns* capacity_ins = addName(lir->insLoad(LIR_ldi, dslots_ins, -int(sizeof(jsval)), ACC_OTHER), "capacity"); guard(true, lir->ins2(LIR_ltui, idx_ins, capacity_ins), exit); /* Load the value and guard on its type to unbox it. */ From c54e460702bef353a53cee8a273f1fb04595c8d4 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 16:05:36 -0700 Subject: [PATCH 16/19] Remove leftover use of js_NewArrayObjectWithCapacity (follow-up for bug 581264). --- js/src/ctypes/CTypes.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/src/ctypes/CTypes.cpp b/js/src/ctypes/CTypes.cpp index f623dbc770cb..e2e3da13882f 100644 --- a/js/src/ctypes/CTypes.cpp +++ b/js/src/ctypes/CTypes.cpp @@ -5038,17 +5038,17 @@ FunctionType::ArgTypesGetter(JSContext* cx, JSObject* obj, jsid idval, jsval* vp size_t len = fninfo->mArgTypes.length(); // Prepare a new array. - jsval* vec; - JSObject* argTypes = - js_NewArrayObjectWithCapacity(cx, len, &vec); - if (!argTypes) + Array vec; + if (!vec.resize(len)) return JS_FALSE; - js::AutoObjectRooter argsroot(cx, argTypes); - JS_ASSERT(len == 0 || vec); for (size_t i = 0; i < len; ++i) vec[i] = OBJECT_TO_JSVAL(fninfo->mArgTypes[i]); + JSObject* argTypes = JS_NewArrayObject(cx, len, vec.begin()); + if (!argTypes) + return JS_FALSE; + // Seal and cache it. if (!JS_SealObject(cx, argTypes, JS_FALSE) || !JS_SetReservedSlot(cx, obj, SLOT_ARGS_T, OBJECT_TO_JSVAL(argTypes))) From 1c0d1640dce00a0376e96ac25e43d66d12ba1217 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 16:08:59 -0700 Subject: [PATCH 17/19] Use proper exported JS_PUBLIC_API function (follow-up for 581264). --- content/base/src/nsFrameMessageManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp index 0d539055b789..ef57b1881592 100644 --- a/content/base/src/nsFrameMessageManager.cpp +++ b/content/base/src/nsFrameMessageManager.cpp @@ -222,7 +222,7 @@ nsFrameMessageManager::SendSyncMessage() JSAutoRequest ar(ctx); PRUint32 len = retval.Length(); - JSObject* dataArray = js_NewArrayObject(ctx, len, NULL); + JSObject* dataArray = JS_NewArrayObject(ctx, len, NULL); NS_ENSURE_TRUE(dataArray, NS_ERROR_OUT_OF_MEMORY); for (PRUint32 i = 0; i < len; ++i) { @@ -346,7 +346,7 @@ nsFrameMessageManager::ReceiveMessage(nsISupports* aTarget, if (!aObjectsArray) { // Because we want JS messages to have always the same properties, // create array even if len == 0. - aObjectsArray = js_NewArrayObject(ctx, 0, NULL); + aObjectsArray = JS_NewArrayObject(ctx, 0, NULL); if (!aObjectsArray) { return false; } From 1b57392126a8fffd70fe30a63ed7117f6243c787 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Fri, 23 Jul 2010 18:30:34 -0500 Subject: [PATCH 18/19] Followup to bug 465199: properly perform ToInteger rather than just ToNumber when determining lastIndex in RegExp.prototype.exec; could have sworn I did this already... --HG-- extra : rebase_source : c63fa22c169751610bc09e7d2c91858ec9cd56b5 --- js/src/jsregexp.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/js/src/jsregexp.cpp b/js/src/jsregexp.cpp index cd688fe69a29..bf1bdbc7ae99 100644 --- a/js/src/jsregexp.cpp +++ b/js/src/jsregexp.cpp @@ -5643,11 +5643,14 @@ regexp_exec_sub(JSContext *cx, JSObject *obj, uintN argc, Value *argv, sticky = (re->flags & JSREG_STICKY) != 0; if (re->flags & (JSREG_GLOB | JSREG_STICKY)) { const Value &v = obj->getRegExpLastIndex(); - if (v.isNumber()) { - lastIndex = v.toNumber(); + if (v.isInt32()) { + lastIndex = v.toInt32(); } else { - if (!ValueToNumber(cx, v, &lastIndex)) + if (v.isDouble()) + lastIndex = v.toDouble(); + else if (!ValueToNumber(cx, v, &lastIndex)) return JS_FALSE; + lastIndex = js_DoubleToInteger(lastIndex); } } else { lastIndex = 0; From 0d6cf59819e07c75ca3642ed52c977b3badf8189 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Fri, 23 Jul 2010 17:24:55 -0700 Subject: [PATCH 19/19] More fallout from bug 581264. --- dom/ipc/TabParent.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index d3ac17723f4e..ee5e4b62adfc 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -524,20 +524,14 @@ TabParent::ReceiveMessage(const nsString& aMessage, nsFrameMessageManager* manager = frameLoader->GetFrameMessageManager(); JSContext* ctx = manager->GetJSContext(); JSAutoRequest ar(ctx); - jsval* dest; PRUint32 len = 0; //TODO: obtain a real value in bug 572685 // Because we want JS messages to have always the same properties, // create array even if len == 0. - JSObject* objectsArray = - js_NewArrayObjectWithCapacity(ctx, len, &dest); + JSObject* objectsArray = JS_NewArrayObject(ctx, len, NULL); if (!objectsArray) { return false; } - nsresult rv = NS_OK; - nsAutoGCRoot arrayGCRoot(&objectsArray, &rv); - NS_ENSURE_SUCCESS(rv, false); - manager->ReceiveMessage(mFrameElement, aMessage, aSync,