471425 - Slim down JSStackFrame

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Andreas Gal :gal]

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
};

Comment 1

[Brendan Eich [:brendan]]

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

Comment 2

[Brendan Eich [:brendan]]

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

Comment 4

[Brendan Eich [:brendan]]

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

Comment 5

[Brendan Eich [:brendan]]

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

[David Mandelin [:dmandelin]]

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.

Comment 7

[Brendan Eich [:brendan]]

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

[David Mandelin [:dmandelin]]

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.