Closed
Bug 471425
Opened 16 years ago
Closed 15 years ago
Slim down JSStackFrame
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: gal, Assigned: brendan)
References
Details
Our stack frames are fat, making function calls slow. We should slim them down. Shaver suggested a JSStackFrameExtra pointer that is NULL by default and is allocated only on demand.
struct JSStackFrame {
JSFrameRegs *regs;
jsbytecode *imacpc; /* null or interpreter macro call pc */
jsval *slots; /* variables, locals and operand stack */
JSObject *callobj; /* lazily created Call object */
JSObject *argsobj; /* lazily created arguments object */
JSObject *varobj; /* variables object, where vars go */
JSObject *callee; /* function or script object */
JSScript *script; /* script being interpreted */
JSFunction *fun; /* function being called or null */
JSObject *thisp; /* "this" pointer if in method */
uintN argc; /* actual argument count */
jsval *argv; /* base of argument stack slots */
jsval rval; /* function return value */
JSStackFrame *down; /* previous frame */
void *annotation; /* used by Java security */
JSObject *scopeChain; /* scope chain */
uintN sharpDepth; /* array/object initializer depth */
JSObject *sharpArray; /* scope for #n= initializer vars */
uint32 flags; /* frame flags -- see below */
JSStackFrame *dormantNext; /* next dormant frame chain */
JSObject *xmlNamespace; /* null or default xml namespace in E4X */
JSObject *blockChain; /* active compile-time block scopes */
JSStackFrame *displaySave; /* previous value of display entry for
script->staticDepth */
#ifdef DEBUG
jsrefcount pcDisabledSave; /* for balanced property cache control */
#endif
};
Assignee | ||
Comment 1•16 years ago
|
||
We don't need no stinking pointers. All of the following should be detected at parse time and assigned fixed slots indexed from fp->slots at run-time:
argsobj
annotation
sharpDepth/sharpArray
xmlNamespace
See bug 412571 for a change from thisp to thisv. We can't use argv[-1] for global code, because global argv is null. Consider fixing so even global frames have a non-null argv, just to eliminate thisv.
This highlights the priority target: lightweight function calls. These should need only the following, at first cut (we can squeeze out more with inline+call threading and work to minimize sp stores and pc updates; see bug 442379, bug 448828, and probably others):
regs
imacpc
slots
callee (no fun required; script too can be got from callee)
argc
argv
rval
down
(no scopeChain, it's callee->fslots[JSSLOT_PARENT])
not sure how few bits of flags -- ideally 0
displaySave (could avoid via more static analysis; later cut)
We need to make indirect eval use its global scope only, then we can make callobj an optional var, just like argsobj.
We could use C++ to make a shallow class hierarchy (struct hierarchy :-P) of common, light- and heavyweight function, global, and eval frame types. No vtbls if you please! Inline sugar as needed.
Cc'ing igor, he's thought about this before too.
/be
Assignee | ||
Comment 2•16 years ago
|
||
Main thing is to avoid forking too many code paths elsewhere, that should not have to care about which optimized frame subtype is at hand.
Someone should take this bug and get it going for 1.9.2.
/be
Assignee | ||
Comment 4•16 years ago
|
||
Enough's enough, the high cost of calling is skewing our perf numbers and order of work. All the flab in JSStackFrame discussed in comment 1 can easily become optional (per-script or -function) local variables.
/be
Assignee: general → brendan
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Assignee | ||
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 5•15 years ago
|
||
Another idea, just discussed with danderson: replace argc and argv with helper methods that look at the caller frame to compute these two values. For argc we could GET_ARGC(caller->regs->pc). For argv we would want caller->regs->sp - argc.
/be
Comment 6•15 years ago
|
||
We're changing the goal of this effort a bit, so I'm closing this bug. Bug 536275 is the tracking bug for further work in this area.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → WONTFIX
Assignee | ||
Comment 7•15 years ago
|
||
Or you could mark this fixed. Or INCOMPLETE, but that seems wrong if you have a metabug whose dependencies were fixed by patches, and then a new metabug for more patching with a different emphasis.
Of course you might object that JSStackFrame is not yet slim, just less fat. But the new bug 536275 is going to make it slimmer, so how is the topic of this bug, the problem to be solved, resolved "WONTFIX"?
/be
Comment 8•15 years ago
|
||
I agree, WONTFIX doesn't make sense, but I couldn't find a disposition that really fit, except DUPLICATE, but I thought people would object to duping to a meta bug. I guess FIXED is OK since the stack frame did in fact get slimmed down with this bug.
Resolution: WONTFIX → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•