Closed Bug 539065 Opened 12 years ago Closed 12 years ago

JSFRAME_ROOTED_ARGV not set for inline call stack frames

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 540706

People

(Reporter: luke, Unassigned)

Details

When JSFRAME_ROOTED_ARGV is set for a frame, the GC knows the caller has rooted argv and allows the GC to avoid redundant computation.  In the normal case where argv does not need to be copied, an inline caller roots the callee's argv, since [argv[-2], argv[argc]) is in the marked range [slots, sp).  However, JSFRAME_ROOTED_ARGV is never set for inline calls.  Is there any reason for this?

In the same area, the call in jsgc.cpp:
  JS_CALL_VALUE_TRACER(trc, fp->calleeValue(), "callee");
seems redundant since calleeValue() is, by definition, argv[-2].
This is a bug indeed. But I suggest to remove JSFRAME_ROOTED_ARGV all together and always root the args for code simplicity. Doing the optimization for the inlined frames may in fact offset potential saving from less marking activities. Also, the bug 528200 and its followups should make the marking of the already marked GC things faster making JSFRAME_ROOTED_ARGV even less relevant.
(In reply to comment #1)
So, are you are saying to find all places where stack frames are *not* marked JSFRAME_ROOTED_ARGV, make them root argv, and then have the GC *always* assume argv has already been marked?  Is that right?
(In reply to comment #2)
> (In reply to comment #1)
> So, are you are saying to find all places where stack frames are *not* marked
> JSFRAME_ROOTED_ARGV, make them root argv, and then have the GC *always* assume
> argv has already been marked?  Is that right?

No - I suggest to remove JSFRAME_ROOTED_ARGV and *always* mark JSStackFrame.argv for code simplicity.
If we get rid of argv (yay) then this is moot. GC mark phase dominates GC runtime, I'm told. If we find redundant marking of inline calls' argvs is in the noise, then code simplicity wins. But profile data would be better than assuming it is in the noise. That, or get rid of argv altogether ;-).

/be
Depends on: 539144
Bug 540706 will remove the need for this flag.
Status: NEW → RESOLVED
Closed: 12 years ago
No longer depends on: 539144
Resolution: --- → DUPLICATE
Duplicate of bug: 540706
You need to log in before you can comment on or make changes to this bug.