Closed Bug 585231 Opened 14 years ago Closed 14 years ago

Remove ArgsPrivateNative

Categories

(Core :: JavaScript Engine, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: luke)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Attached patch patch (deleted) — Splinter Review
The only use of ArgsPrivateNative is to allow ArgGetter to return an argument of a stack frame on trace. (ArgSetter just leaves trace.) Since any access from trace of any arguments object for any frame on trace is going to go directly to the argument slot (skipping the arguments object), it seems that we could just drop the whole ArgsPrivateNative business altogether and just LeaveTrace in ArgGetter. Since it is extremely difficult to even reach ArgGetter for an arguments object with an ArgsPrivateNative (you have to leave through a C++ native and then do a getProperty without hitting a LeaveTree, no small feat), this shouldn't affect perf. It definitely doesn't hit on SS/V8. Removing this also avoids adding further complexity in bug 539144.
Attachment #463724 - Flags: review?(dmandelin)
Attachment #463724 - Flags: review?(dmandelin) → review+
As Dave pointed out, a further comment is warranted for this change in js::Execute: - fp->argsobj = down->argsobj; + fp->argsobj = NULL; FWIW, the fp->argsobj field for an eval frame is basically dead: - js_GetArgsValue specifically walks over eval frames to find the right fp->argsobj - js::Execute does not call putActivationObjects or copy the fp->argsobj back to down->argsobj, so if somehow, magically, an eval frame grew an argsobj, it would be a Bad Thing. Once place where the nullness of fp->argsobj *does* matter, though, is LeaveTree, where it is important that, when fixing up frames in FlushNativeStackFrame, that if two frames (an eval and non-eval) share an args object, that the args object's private be fixed up to point to the non-eval frame. The current code is correct due to the order of FlushNativeStackFrame calls and the particular conditions used, but its subtle. Its simpler just to have only one frame ever point to an args object. Hence the null. It seemed simpler just to have an eval frame's argsobj always be null.
Hurrah! I am not going to worry about ArgsPrivateNative for bug 558451 any longer (I was actually deferring any attempt to emulate JSC's fixed first free slot per class architecture, anyway). Yeah, eval is evil, don't let it complicate other code. Penalize it! See my JSConf 2010 talk, I got the words wrong (the last one... "definitely a W-word" [with object]). /be
Whiteboard: fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: