Bug 412340: avois rehashing of alreday atomized strings. r,a=brendan

This commit is contained in:
igor@mir2.org 2008-01-23 05:17:47 -08:00
Родитель 812a206f33
Коммит 8c529adb1c
8 изменённых файлов: 106 добавлений и 70 удалений

Просмотреть файл

@ -1694,7 +1694,7 @@ DumpHeap(JSContext *cx, uintN argc, jsval *vp)
static JSBool
DoExport(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)
{
JSAtom *atom;
jsid id;
JSObject *obj2;
JSProperty *prop;
JSBool ok;
@ -1707,19 +1707,18 @@ DoExport(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)
if (!JS_ValueToObject(cx, argv[0], &obj))
return JS_FALSE;
argv[0] = OBJECT_TO_JSVAL(obj);
atom = js_ValueToStringAtom(cx, argv[1]);
if (!atom)
if (!js_ValueToStringId(cx, argv[1], &id))
return JS_FALSE;
if (!OBJ_LOOKUP_PROPERTY(cx, obj, ATOM_TO_JSID(atom), &obj2, &prop))
if (!OBJ_LOOKUP_PROPERTY(cx, obj, id, &obj2, &prop))
return JS_FALSE;
if (!prop) {
ok = OBJ_DEFINE_PROPERTY(cx, obj, id, JSVAL_VOID, NULL, NULL,
JSPROP_EXPORTED, NULL);
} else {
ok = OBJ_GET_ATTRIBUTES(cx, obj, ATOM_TO_JSID(atom), prop, &attrs);
ok = OBJ_GET_ATTRIBUTES(cx, obj, id, prop, &attrs);
if (ok) {
attrs |= JSPROP_EXPORTED;
ok = OBJ_SET_ATTRIBUTES(cx, obj, ATOM_TO_JSID(atom), prop, &attrs);
ok = OBJ_SET_ATTRIBUTES(cx, obj, id, prop, &attrs);
}
OBJ_DROP_PROPERTY(cx, obj2, prop);
}

Просмотреть файл

@ -2563,23 +2563,15 @@ JS_DestroyIdArray(JSContext *cx, JSIdArray *ida)
JS_PUBLIC_API(JSBool)
JS_ValueToId(JSContext *cx, jsval v, jsid *idp)
{
JSAtom *atom;
CHECK_REQUEST(cx);
if (JSVAL_IS_INT(v)) {
if (JSVAL_IS_INT(v))
*idp = INT_JSVAL_TO_JSID(v);
} else {
#if JS_HAS_XML_SUPPORT
if (JSVAL_IS_OBJECT(v)) {
*idp = OBJECT_JSVAL_TO_JSID(v);
return JS_TRUE;
}
else if (!JSVAL_IS_PRIMITIVE(v))
*idp = OBJECT_JSVAL_TO_JSID(v);
#endif
atom = js_ValueToStringAtom(cx, v);
if (!atom)
return JS_FALSE;
*idp = ATOM_TO_JSID(atom);
}
else
return js_ValueToStringId(cx, v, idp);
return JS_TRUE;
}

Просмотреть файл

@ -690,12 +690,13 @@ js_AtomizeString(JSContext *cx, JSString *str, uintN flags)
++state->tablegen;
}
}
JS_ASSERT(JSSTRING_IS_FLAT(str) && !JSSTRING_IS_MUTABLE(str));
INIT_ATOM_ENTRY(entry, key);
JSFLATSTR_SET_ATOMIZED(key);
}
finish:
ADD_ATOM_ENTRY_FLAGS(entry, flags & (ATOM_PINNED | ATOM_INTERNED));
JS_ASSERT(JSSTRING_IS_ATOMIZED(key));
v = STRING_TO_JSVAL(key);
cx->weakRoots.lastAtom = v;
JS_UNLOCK(&state->lock, cx);
@ -795,15 +796,36 @@ js_AtomizePrimitiveValue(JSContext *cx, jsval v, JSAtom **atomp)
return JS_TRUE;
}
JSAtom *
js_ValueToStringAtom(JSContext *cx, jsval v)
JSBool
js_ValueToStringId(JSContext *cx, jsval v, jsid *idp)
{
JSString *str;
JSAtom *atom;
str = js_ValueToString(cx, v);
if (!str)
return NULL;
return js_AtomizeString(cx, str, 0);
/*
* Optimize for the common case where v is an already-atomized string. The
* comment in jsstr.h before the JSSTRING_SET_ATOMIZED macro's definition
* explains why this is thread-safe. The extra rooting via lastAtom (which
* would otherwise be done in js_js_AtomizeString) ensures the caller that
* the resulting id at is least weakly rooted.
*/
if (JSVAL_IS_STRING(v)) {
str = JSVAL_TO_STRING(v);
if (JSSTRING_IS_ATOMIZED(str)) {
cx->weakRoots.lastAtom = v;
*idp = ATOM_TO_JSID((JSAtom *) v);
return JS_TRUE;
}
} else {
str = js_ValueToString(cx, v);
if (!str)
return JS_FALSE;
}
atom = js_AtomizeString(cx, str, 0);
if (!atom)
return JS_FALSE;
*idp = ATOM_TO_JSID(atom);
return JS_TRUE;
}
#ifdef DEBUG

Просмотреть файл

@ -416,10 +416,10 @@ JSBool
js_AtomizePrimitiveValue(JSContext *cx, jsval v, JSAtom **atomp);
/*
* Convert v to an atomized string.
* Convert v to an atomized string and wrap it as an id.
*/
extern JSAtom *
js_ValueToStringAtom(JSContext *cx, jsval v);
extern JSBool
js_ValueToStringId(JSContext *cx, jsval v, jsid *idp);
#ifdef DEBUG

Просмотреть файл

@ -675,9 +675,9 @@ js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, JSPropertyOp setter)
if (JSID_IS_ATOM(id)) {
atom = JSID_TO_ATOM(id);
} else if (JSID_IS_INT(id)) {
atom = js_ValueToStringAtom(cx, INT_JSID_TO_JSVAL(id));
if (!atom)
if (!js_ValueToStringId(cx, INT_JSID_TO_JSVAL(id), &id))
return NULL;
atom = JSID_TO_ATOM(id);
} else {
atom = NULL;
}
@ -690,10 +690,9 @@ js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, JSPropertyOp setter)
}
JS_PUBLIC_API(JSBool)
JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval idval,
JSWatchPointHandler handler, void *closure)
{
JSAtom *atom;
jsid propid;
JSObject *pobj;
JSProperty *prop;
@ -709,15 +708,10 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
return JS_FALSE;
}
if (JSVAL_IS_INT(id)) {
propid = INT_JSVAL_TO_JSID(id);
atom = NULL;
} else {
atom = js_ValueToStringAtom(cx, id);
if (!atom)
return JS_FALSE;
propid = ATOM_TO_JSID(atom);
}
if (JSVAL_IS_INT(idval))
propid = INT_JSVAL_TO_JSID(idval);
else if (!js_ValueToStringId(cx, idval, &propid))
return JS_FALSE;
if (!js_LookupProperty(cx, obj, propid, &pobj, &prop))
return JS_FALSE;
@ -752,8 +746,8 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
flags = sprop->flags;
shortid = sprop->shortid;
} else {
if (!OBJ_GET_PROPERTY(cx, pobj, id, &value) ||
!OBJ_GET_ATTRIBUTES(cx, pobj, id, prop, &attrs)) {
if (!OBJ_GET_PROPERTY(cx, pobj, propid, &value) ||
!OBJ_GET_ATTRIBUTES(cx, pobj, propid, prop, &attrs)) {
OBJ_DROP_PROPERTY(cx, pobj, prop);
return JS_FALSE;
}

Просмотреть файл

@ -807,12 +807,11 @@ call_enumerate(JSContext *cx, JSObject *obj)
}
static JSBool
call_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
call_resolve(JSContext *cx, JSObject *obj, jsval idval, uintN flags,
JSObject **objp)
{
JSStackFrame *fp;
JSString *str;
JSAtom *atom;
jsid id;
JSLocalKind localKind;
JSPropertyOp getter, setter;
uintN slot, attrs;
@ -824,15 +823,13 @@ call_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
JS_ASSERT(fp->fun);
JS_ASSERT(GET_FUNCTION_PRIVATE(cx, fp->callee) == fp->fun);
if (!JSVAL_IS_STRING(id))
if (!JSVAL_IS_STRING(idval))
return JS_TRUE;
str = JSVAL_TO_STRING(id);
atom = js_AtomizeString(cx, str, 0);
if (!atom)
if (!js_ValueToStringId(cx, idval, &id))
return JS_FALSE;
localKind = js_LookupLocal(cx, fp->fun, atom, &slot);
localKind = js_LookupLocal(cx, fp->fun, JSID_TO_ATOM(id), &slot);
if (localKind != JSLOCAL_NONE) {
if (localKind == JSLOCAL_ARG) {
JS_ASSERT(slot < fp->fun->nargs);
@ -850,9 +847,9 @@ call_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
? JSPROP_PERMANENT | JSPROP_READONLY
: JSPROP_PERMANENT;
}
if (!js_DefineNativeProperty(cx, obj, ATOM_TO_JSID(atom), vp[slot],
getter, setter, attrs,
SPROP_HAS_SHORTID, (int) slot, NULL)) {
if (!js_DefineNativeProperty(cx, obj, id, vp[slot], getter, setter,
attrs, SPROP_HAS_SHORTID, (int) slot,
NULL)) {
return JS_FALSE;
}
*objp = obj;
@ -863,10 +860,8 @@ call_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
* Resolve arguments so that we never store a particular Call object's
* arguments object reference in a Call prototype's |arguments| slot.
*/
atom = cx->runtime->atomState.argumentsAtom;
if (id == ATOM_KEY(atom)) {
if (!js_DefineNativeProperty(cx, obj,
ATOM_TO_JSID(atom), JSVAL_VOID,
if (id == ATOM_TO_JSID(cx->runtime->atomState.argumentsAtom)) {
if (!js_DefineNativeProperty(cx, obj, id, JSVAL_VOID,
NULL, NULL, JSPROP_PERMANENT,
SPROP_HAS_SHORTID, CALL_ARGUMENTS,
NULL)) {

Просмотреть файл

@ -1629,8 +1629,6 @@ js_InvokeConstructor(JSContext *cx, jsval *vp, uintN argc)
static JSBool
InternNonIntElementId(JSContext *cx, JSObject *obj, jsval idval, jsid *idp)
{
JSAtom *atom;
JS_ASSERT(!JSVAL_IS_INT(idval));
#if JS_HAS_XML_SUPPORT
@ -1646,11 +1644,7 @@ InternNonIntElementId(JSContext *cx, JSObject *obj, jsval idval, jsid *idp)
}
#endif
atom = js_ValueToStringAtom(cx, idval);
if (!atom)
return JS_FALSE;
*idp = ATOM_TO_JSID(atom);
return JS_TRUE;
return js_ValueToStringId(cx, idval, idp);
}
/*

Просмотреть файл

@ -70,6 +70,10 @@ JS_BEGIN_EXTERN_C
* such string but extreme care must be taken to ensure that no other code
* relies on the original length of the string.
*
* A flat string with JSSTRFLAG_ATOMIZED set means that the string is hashed
* as an atom. This flag is used to avoid re-hashing of the already-atomized
* string.
*
* When JSSTRFLAG_DEPENDENT is set, the string depends on characters of
* another string strongly referenced by the u.base field. The base member may
* point to another dependent string if JSSTRING_CHARS has not been called
@ -100,12 +104,14 @@ struct JSString {
* JSSTRFLAG_PREFIX and JSSTRFLAG_MUTABLE are two aliases for the same value.
* JSSTRFLAG_PREFIX should be used only if JSSTRFLAG_DEPENDENT is set and
* JSSTRFLAG_MUTABLE should be used only if the string is flat.
* JSSTRFLAG_ATOMIZED is used only with the flat immutable strings.
*/
#define JSSTRFLAG_DEPENDENT JSSTRING_BIT(JS_BITS_PER_WORD - 1)
#define JSSTRFLAG_PREFIX JSSTRING_BIT(JS_BITS_PER_WORD - 2)
#define JSSTRFLAG_MUTABLE JSSTRFLAG_PREFIX
#define JSSTRFLAG_ATOMIZED JSSTRING_BIT(JS_BITS_PER_WORD - 3)
#define JSSTRING_LENGTH_BITS (JS_BITS_PER_WORD - 2)
#define JSSTRING_LENGTH_BITS (JS_BITS_PER_WORD - 3)
#define JSSTRING_LENGTH_MASK JSSTRING_BITMASK(JSSTRING_LENGTH_BITS)
/* Universal JSString type inquiry and accessor macros. */
@ -117,6 +123,9 @@ struct JSString {
#define JSSTRING_IS_MUTABLE(str) (((str)->length & (JSSTRFLAG_DEPENDENT | \
JSSTRFLAG_MUTABLE)) == \
JSSTRFLAG_MUTABLE)
#define JSSTRING_IS_ATOMIZED(str) (((str)->length & (JSSTRFLAG_DEPENDENT | \
JSSTRFLAG_ATOMIZED)) ==\
JSSTRFLAG_ATOMIZED)
#define JSSTRING_CHARS(str) (JSSTRING_IS_DEPENDENT(str) \
? JSSTRDEP_CHARS(str) \
@ -137,7 +146,7 @@ struct JSString {
? JSSTRDEP_LENGTH(str) + ((chars_) = JSSTRDEP_CHARS(str)) \
: JSFLATSTR_LENGTH(str) + ((chars_) = JSFLATSTR_CHARS(str))))
/* Specific flat string initializer, accessor and mutator macros. */
/* Specific flat string initializer and accessor macros. */
#define JSFLATSTR_INIT(str, chars_, length_) \
((void)(JS_ASSERT(((length_) & ~JSSTRING_LENGTH_MASK) == 0), \
(str)->length = (length_), (str)->u.chars = (chars_)))
@ -148,13 +157,44 @@ struct JSString {
#define JSFLATSTR_CHARS(str) \
(JS_ASSERT(JSSTRING_IS_FLAT(str)), (str)->u.chars)
/*
* Macros to manipulate atomized and mutable flags of flat strings. It is safe
* to use these without extra locking due to the following properties:
*
* * We do not have a macro like JSFLATSTR_CLEAR_ATOMIZED as a string
* remains atomized until the GC collects it.
*
* * A thread may call JSFLATSTR_SET_MUTABLE only when it is the only thread
* accessing the string until a later call to JSFLATSTR_CLEAR_MUTABLE.
*
* * Multiple threads can call JSFLATSTR_CLEAR_MUTABLE but the macro
* actually clears the mutable flag only when the flag is set -- in which
* case only one thread can access the string (see previous property).
*
* Thus, when multiple threads access the string, JSFLATSTR_SET_ATOMIZED is
* the only macro that can update the length field of the string by changing
* the mutable bit from 0 to 1. We call the macro only after the string has
* been hashed. When some threads in js_ValueToStringId see that the flag is
* set, it knows that the string was atomized.
*
* On the other hand, if the thread sees that the flag is unset, it could be
* seeing a stale value when another thread has just atomized the string and
* set the flag. But this can lead only to an extra call to js_AtomizeString.
* This function would find that the string was already hashed and return it
* with the atomized bit set.
*/
#define JSFLATSTR_SET_ATOMIZED(str) \
((void)(JS_ASSERT(JSSTRING_IS_FLAT(str) && !JSSTRING_IS_MUTABLE(str)), \
(str)->length |= JSSTRFLAG_ATOMIZED))
#define JSFLATSTR_SET_MUTABLE(str) \
((void)(JS_ASSERT(JSSTRING_IS_FLAT(str)), \
((void)(JS_ASSERT(JSSTRING_IS_FLAT(str) && !JSSTRING_IS_ATOMIZED(str)), \
(str)->length |= JSSTRFLAG_MUTABLE))
#define JSFLATSTR_CLEAR_MUTABLE(str) \
((void)(JS_ASSERT(JSSTRING_IS_FLAT(str)), \
(str)->length &= ~JSSTRFLAG_MUTABLE))
JSSTRING_HAS_FLAG(str, JSSTRFLAG_MUTABLE) && \
((str)->length &= ~JSSTRFLAG_MUTABLE)))
/* Specific dependent string shift/mask accessor and mutator macros. */
#define JSSTRDEP_START_BITS (JSSTRING_LENGTH_BITS-JSSTRDEP_LENGTH_BITS)