570662 - TM: eagerly call thisObject hook and cache result in global objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

This patch does less than we could do, but I ran into a few complications. Also, its not entirely clear this is safe. If this works, we can rip out JSFRAME_CALCULATED_THIS and remove fp->thisv, but that patch is pretty involved, so lets do this one by one.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Andreas Gal :gal]

Things that worry me: thisObject hook runs when the global object is made, not when accessing .this, so stack and scope chain might look different. Also, invalidation? Or is this always safe? With our cross compartment wrappers this should be safe for sure. Right now not so sure.

Comment 3

[Andreas Gal :gal]

Note the code around this comment:

/* Now that the frame has been pushed, we can call the thisObject hook. */

Thats basically exactly what worries me.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: new version, don't cache so eagerly

Blake noticed that if you call thisObject from JS_SetGlobalObject, it will call JS_GetScopeChain and get back a wrong answer (likely NULL). So we wait until the first time you need it.

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: v2

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 454096 [details] [diff] [review]
v2

mrbkap says it's ok

Comment 7

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/9ae06b151fdf

Comment 8

[Jason Orendorff [:jorendorff]]

Also requires this follow-up fix: http://hg.mozilla.org/tracemonkey/rev/f454bf6a0693

Comment 9

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/9ae06b151fdf
http://hg.mozilla.org/mozilla-central/rev/f454bf6a0693