ATOMIC_DECREMENT turns out to be _extremely_ expensive due to the lock prefix. This is a major cost during GC. The attached patch tells js_DropScope whether the gc lock is currently being held, in which case a simple non-atomic decrement is sufficient.

Comment on attachment 387508 [details] [diff] [review] patch No, even simpler: assert CX_THREAD_IS_RUNNING_GC(cx) and always use the unlocked decrement. /be

No, GetMutableScope calls DropScope from outside the GC.

(In reply to comment #3) > No, GetMutableScope calls DropScope from outside the GC. Oops, but that is a different call. It can never destroy a scope, in particular. Instead of adding flag param overhead, specialize just that function. /be

Don't want to r+ myself, this is different-looking by enough code, but provably equivalent (check out the comments and assertion). /be

Want to get this in soon, scope-related patches stacking up all over. /be

Comment on attachment 387683 [details] [diff] [review] fix to #ifdef JS_THREADSAFE that assertion This test flunks the assertion in js_GetMutableScope: // a -> b -> c function B(){} var c = B.prototype = {x: 1}; function A(){} var b = A.prototype = new B; var a = new A; // Kill c. b.__proto__ = null; c = B.prototype = null; gc(); a.y = 2;

Thanks, will fix. /be

Comment on attachment 387692 [details] [diff] [review] inline a bit more of js_DropScope into js_GetMutableScope Looks good.

I cloned the JS_ASSERT(scope->nrefs > 0); from js_DropScope into the tail of js_GetMutableScope too. http://hg.mozilla.org/tracemonkey/rev/51ed66617fb5 /be