Closed
Bug 542797
Opened 15 years ago
Closed 15 years ago
change obj_eval from JSNative to JSFastNative
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: luke, Assigned: luke)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
Because obj_eval is a slow native, an extra JSStackFrame gets created which is immediately ignored by when js_Execute is called with down = cx->fp->down. This requires creating an extra JSCallStack and taking the SlowVarObj path in js_Execute. Also, its really tricky to reason about. This patch makes obj_eval a fast native and removes some dead/confounding logic in obj_eval.
The patch is on top of the remove-var-obj patch in bug 535656.
Attachment #424025 -
Flags: review?(mrbkap)
|
Assignee | |
Comment 1•15 years ago
|
||
Assignee: general → lw
Attachment #424025 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #424429 -
Flags: review?(mrbkap)
Attachment #424025 -
Flags: review?(mrbkap)
|
||
Comment 2•15 years ago
|
||
Comment on attachment 424429 [details] [diff] [review]
freshend up, tweaked
I complained on IRC about using JS_ARGV and JS_THIS_OBJECT instead of hand-unrolling them.
Also (more substantially) js_CheckScopeChainValidity can fail.
It's nice to see that maze of declarations die.
Attachment #424429 -
Flags: review?(mrbkap) → review-
|
|
Assignee | |
Comment 3•15 years ago
|
||
Nice catch, thanks.
Attachment #424429 -
Attachment is obsolete: true
Attachment #425584 -
Flags: review?(mrbkap)
|
|
||
Comment 4•15 years ago
|
||
Comment on attachment 425584 [details] [diff] [review]
less wrong
Thanks!
Attachment #425584 -
Flags: review?(mrbkap) → review+
|
||
Comment 5•15 years ago
|
||
Comment on attachment 425584 [details] [diff] [review]
less wrong
Nice goto elimination via RAII!
> if (callbacks && callbacks->findObjectPrincipals) {
>- principals = callbacks->findObjectPrincipals(cx, fp->callee());
>+ principals = callbacks->findObjectPrincipals(cx, callee);
> } else {
> principals = NULL;
> }
Dunno why this was over-braced, but it need not be.
/be
Attachment #425584 -
Flags: review+ → review?(mrbkap)
|
|
||
Updated•15 years ago
|
Attachment #425584 -
Flags: review?(mrbkap) → review+
|
|
Assignee | |
Comment 6•15 years ago
|
||
With less bracing:
http://hg.mozilla.org/tracemonkey/rev/e91417e33a53
Whiteboard: fixed-in-tracemonkey
|
||
Comment 7•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•