to the JS API, for per class extra slots beyond JSSLOT_PRIVATE (or starting
there for a class that lacks JSCLASS_HAS_PRIVATE). To avoid penalizing all
instances, these slots are allocated only upon first property-owned slot
allocation, or upon first JS_SetReservedSlot.
This entailed adding getRequiredSlot and setRequiredSlot hooks to the
JSObjectOps struct, and making obj->slots self-describing, a la BSTR. It
also afforded me a chance to clean up obj->slots locking so that non-native
JSObjectOps didn't risk unlocked accesses! Now there are thread-safe hooks
for all uses of obj.
First consumer is the new, DOM-glue-unifying XPConnect, which needs two
slots per wrapped function. Hence the change to js_FunctionClass.flags'
initializer.
- Commented the heck out of JSClass and JSObjectOps function typedefs in
jspubtd.h. I hope embedders see these comments!
- Fix JS_XDRValue's default case to handle int exclusively, there is no other
possible type (and therefore no JSMSG_BAD_JVAL_TYPE error).
- Clean up tabs in select old, tab-ridden files and sections.
- s/\<fh\>/file/g for stdio FILE * canonical variable names.
especially to jband for his great stress-test setup and particularly helpful
(in terms of reproducing bugs in draft patches) MP and laptop machines.
- Radical(*) object (scope) locking optimization: don't lock if a scope is
accessed on the context that exclusively owns it (initially, the context
on which the scope was created). Once a scope becomes shared among more
than one owner-context, give it the usual thin or fat lock, per existing
jslock.c code.
I did this at the memory cost of another word per JSScope, ownercx, which
raised scope size from 12 to 13 words if !DEBUG. I also added a linked
list head pointer, rt->scopeSharingTodo, and a scopeSharingDone condition
variable to JSRuntime, and a scopeToShare pointer to JSContext that's
necessary for deadlock avoidance.
The rt->scopeSharingTodo list links JSScopes through the scope->u.link
union arm, which overlays the pre-existing scope->count (now u.count)
member. This list holds scopes still exclusively owned by a context, but
wanted by js_LockScope calls active on other threads. Those calls wait
on the rt->scopeSharingDone condition, which is notified every time an
owner-context ends the request running on it, in which code active on
that context may be using scope freely until end of request.
The code that waits on rt->scopeSharingDone must first suspend any and
all requests active on the calling context, and resume those contexts
after the wait is notified. This means a GC could happen while the
thread locking a scope owned by another thread's context blocks; all
calls to JS_LOCK_OBJ must therefore first home fp->sp above any live
operands, e.g. The interpreter takes care to do that already.
To avoid AB-BA deadlocks, if a js_LockScope attempt on one context finds
that the owner-context of the scope is already waiting on a scope owned
by the current context (or indirectly depending on such a scope lock),
the attempt converts the scope from lock-free exclusive ownership to
shared ownership (thin or fat lock).
- Fix js_SetupLocks and the js_LockGlobal/js_UnlockGlobal code to avoid
divmod instruction costs, strength-reducing to bit-mask instructions.
- The radical lock-free scope change required care in handling the 0=>1
and 1=>0 transitions of cx->requestDepth, which was till now thread-local
because part of the JSContext not manipulated by other threads. It's
still updated only by cx's thread, but it is read by other threads in
the course of attempting to claim exclusive ownership of a scope for more
lock-free JS object operations.
- The JS_SuspendRequest and JS_ResumeRequest APIs have changed incompatibly
to require their caller to save and restore the requestCount found when
JS_SuspendRequest is called. This is necessary to avoid deadlock; sorry
for the incompatible change.
- Fixed various nits in jslock.[ch], including using Init/Finish rather
than New/Destroy for the methods that take a JSThinLock and initialize
and finish/free its members. Another example: JS_ATOMIC_ADDREF is now
JS_ATOMIC_INCREMENT and JS_ATOMIC_DECREMENT, so the two cases can be
mapped to PR_AtomicIncrement and PR_AtomicDecrement. This entailed
changing jsrefcount from jsword to int32 (PRInt32).
- No need to use JS_ATOMIC_INCREMENT on JSScopeProperty.nrefs, as it is
always and everywhere protected by the property's JSScope.lock.
- Cleaned up gratuitous casts in jscntxt.c by using &cx->links, etc.
- The lock used for mutual exclusion around both request begin and end vs.
GC synchronization is rt->gcLock, and this lock now also protects all
scope->ownercx pointer changes from non-null (exclusive) to null (shared),
the rt->scopeSharingTodo/scope->u.link list operations, and of course the
rt->scopeSharingDone condition.
But this means that js_GC cannot hold rt->gcLock across the bulk of its
body, in particular the mark phase, during which JS_GetPrivate calls,
e.g., may need to "promote" scope locks from lock-free to thin or fat,
because doing so would double-trip. There never was any good reason to
hold rt->gcLock so long, of course -- locks are for mutual exclusion, not
for waiting or notifying a thread -- those operations require a condition,
rt->gcDone, which we already use along with rt->gcLevel to keep racing GC
attempts at bay.
So now that rt->gcLock does not protect the mark phase, the enumeration
of rt->gcRootsHash can race badly with JS_RemoveRootRT, an API that may
legitimately be called outside of a request, without even a context. It
turns out that people may be cheating on the request model even with
JS_AddRoot, JS_AddNamedRoot, and JS_RemoveRoot calls, so we must make
all of those interlock with the GC using gcLevel and gcDone, unless they
are called on the gcThread.
Also, since bug 49816 was fixed, there has been no need for a separate
finalize phase, or for rt->gcFinalVec. Finalizers can no longer allocate
newborn GC-things that might be swept (because unmarked), or double-trip
on rt->gcLock (which is no longer held). So js_GC finalizes as it sweeps,
just as it did in days of old.
- I added comments to jslock.h making it plain that callers of JS_LOCK_OBJ
and JS_UNLOCK_OBJ must either be implementations of js_ObjectOps hooks,
or code reachable only from those hooks; or else must be predicated on
OBJ_IS_NATIVE tests. It turns out jsinterp.c's CACHED_GET and CACHED_SET
macros neglected to do such tests, limiting the ability of JS embeddings
to implement JSObjectOps with their own non-JSScope JSObjectMap subclass.
Fixed, small performance hit that the lock-free optimization should more
than make up for.
- jslock.c now gives a #error if you try to compile it on a platform that
lacks a compare-and-swap instruction. The #error says to use NSPR locks.
Before this change, some platforms would emulate compare-and-swap using
a global PRLock, which is always worse in runtime than using per-scope
PRLocks.
Bug #14462, lot's of discussin there about these changes, but here's
Brendan's description :
In order, changes in the patch are:
- Rename JSSLOT_ITR_STATE to be JSSLOT_ITER_STATE (avoid cybercrud abbreviation
as cbrcrd, no more six-char id limits!).
- Property cache tests must occur with the object's scope-lock held, to close a
race with delete (js_DestroyProperty, always called with the property's scope
locked). Once the cache has been hit, and before the lock is released, the
property's refcount must be bumped. This requires re-acquisition of the lock
and js_DropScopeProperty afterward.
- Reworked js_FindProperty to use a do-while loop, as cx->fp->scopeChain must be
non-null. This avoids a gratuitous lastobj init done to "Suppress gcc warning"
in the old revision.
- Akin to the property cache hit cases in jsinterp.c and jsobj.c's
js_FindProperty, code to hold and drop the scope-property by its refcount that
was #ifdef JS_THREADSAFE must be unconditional, now that user-defined getters
and setters may delete the property id they're getting or setting.
- Fixed overlong continuation line in jsobj.h.
/be
code review and fixes (r=chouck@geocast.com). He needs this cuz he has no
knowledge of exact number of properties before new-style enumerating them.
- Patch up jsdbgapi.c a bit -- it needs to use OBJ_GET_ATTRIBUTES and new APIs
to do a better job describing properties to a debugger.
- Add JSMSG_CANT_DESCRIBE_PROPS for bogus non-native error case in jsdbgapi.c.
- Fix "Inappropriate" => "invalid" in JSMSG_BAD_ARRAY_LENGTH message.
+ merging of js/src and js/ref
+ elimination of most dependencies on NSPR
+ JS1.4 feature additions and accumulated bug fixes
More details are in last week's mozilla status report.
brendan%mozilla.org
gerv%gerv.net
timeless%mac.com
scc%mozilla.org
rogerl%netscape.com
dmose%mozilla.org
jband%netscape.com
fur%netscape.com
fur
ltabb