551633 - js_LockObj is O(N) in number of JSContexts

Status: RESOLVED DUPLICATE of bug 606029

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

js_LockObj calls ClaimTitle, which calls js_ValidContextPointer on the common codepath as far as I can see.  This does a linear walk over all live JSContexts (likely several hundred in my default browsing profile) looking for the given ownercx....

Worth worrying about this, or just ignore for now until we get rid of JS_THREADSAFE?

Comment 1

[Brendan Eich [:brendan]]

Ignore till you have real-world profile data. This is a case where an object is being traded among contexts. Nutty, we do not want to optimize. Even before fixing this bug we'd want to fix the (probably unintended) sharing of a mutable object. Immutable (frozen) objects shouldn't entail locking at all.

/be

Comment 2

[Boris Zbarsky [:bzbarsky]]

I noticed this because a testcase I was profiling was showing about 15% of its L2 cache misses in js_ValidContextPointer.  Didn't seem like an obvious time hotspot, though....

Comment 3

[Brendan Eich [:brendan]]

What testcase? What object?

We expect cross-thread sharing of mutable objects to be vanishingly rare in shipped Firefox code and worthy add-on code, in the proposed JS threads and GC world of isolated "shires" or "compartments".

/be

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #2)
> I noticed this because a testcase I was profiling was showing about 15% of its
> L2 cache misses in js_ValidContextPointer.

Any chance that was a nearby static method or function being blamed on the wrong extern symbol?

/be

Comment 5

[Boris Zbarsky [:bzbarsky]]

> What testcase?

A somewhat modified form of one of the dromaeo regexp testcases.

> What object?

Wish I could tell you....

If this situation is expected to be rare, I'm not going to worry about it for now.

> Any chance that was a nearby static method or function being blamed on the
> wrong extern symbol?

I don't know enough about shark guts to answer that question.

If I ever hit this again, I'll make sure to take better notes.

Comment 6

[Brendan Eich [:brendan]]

Argh, Dromaeo. I smell a window/iframe context-pair, with an object shared across contexts (but not threads).

Dromaeo is so not representative in this regard.

/be

Comment 7

[Boris Zbarsky [:bzbarsky]]

> I smell a window/iframe context-pair

Ah, very likely.

> Dromaeo is so not representative in this regard.

Are we sure?  gmail has a bunch of iframes on it; do they not run script?

Comment 8

[Brendan Eich [:brendan]]

Worth checking gmail, but you have to be taking turns with an object from code running from two different contexts -- not simply functions in two windows called from one control flow (which run on one context, barring native-path context switching, e.g., under scriptable-helpers such as AddProperty).

/be

Comment 9

[André Bargull [:anba]]

js_LockObj was removed as part of http://hg.mozilla.org/mozilla-central/rev/60bdafdffdb9 (bug 606029)