340537 - 7% Tdhtml regression on June 5

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Graph:  http://build-graphs.mozilla.org/graph/query.cgi?testname=dhtml&tbox=luna.mozilla.org&autoscale=1&days=7&avg=1&showpoint=2006:06:05:16:01:23,1595

Checkin range: http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=MozillaTinderboxAll&date=explicit&mindate=1149540900&maxdate=1149546299

If I were a betting man, I'd bet on bug 339918.  With that change, once we have a single toplevel script access on a window (before any accesses from functions), all following accesses on that window will do security checks, as far as I can tell.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Potential fix

This patch attempts to undo some of the damage by not really caching the value if we were unable to find a function object on the stack (so we'll do the security checks for top level scripts, but only cache the result when we actually found a function object). This might still ding perf on scripts who only access window or document properties from top level scripts, but I'm hoping it will save Tdhtml setInterval functions, etc.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

I checked the potential fix into the trunk. All tinderboxes running Tdhtml except for argo and argo-vm showed drops back to pre-bug 339918 numbers. I'm marking this bug as fixed, though I cannot explain argo's reaction.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Nominating to match bug 339918.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 224604 [details] [diff] [review]
Potential fix

This is a companion patch for bug 339918.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.8 branch.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 224604 [details] [diff] [review]
Potential fix

approved for 1.8.0 branch, a=dveditz for drivers

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.8.0 branch.