979043 - simplify/rename forEachCanonicalActualArg, simplify fun_apply

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Luke Wagner [:luke]]

Several things have changes that allow some nice simplifications of the whole forEachCanonicalActualArg business as well as fun_apply:
 - the stack redux took out the arg-duplication; all actual args are canonical
 - the are no more uses of the (start, count) args
 - with the f.apply optimization in Ion, fun_apply is almost never called anymore with JS_OPTIMIZED_ARGUMENTS (1059 times for all of octane, zero in ss/kraken)
 - even if f.apply were called with JS_OPTIMIZED_ARGUMENTS, ScriptFrameIter construction isn't terribly slow anymore since JM and ExpandAllInlineFrames was removed

The patches also apply modern idioms to fun_call/apply.

Comment 1

[Luke Wagner [:luke]]

Attachment: tidy-up-args-1

Comment 2

[Luke Wagner [:luke]]

Attachment: tidy-up-args-2

Comment 3

[Shu-yu Guo [:shu]]

Luke, I also have a refactor patch for readFrameArgs in flight (for a long while now, because bug 716647 is taking forever).

Do you mind rebasing this off of that? I could just push that part, actually...

Comment 4

[Luke Wagner [:luke]]

If you want to land whatever part is already reviewed, go for it.  Otherwise, this patch is really pretty simple changes, so it should be pretty easy to rebase if it went in first.

Comment 5

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8384961 [details] [diff] [review]
tidy-up-args-2

Review of attachment 8384961 [details] [diff] [review]:
-----------------------------------------------------------------

Nice patch. :)

::: js/src/jsfun.cpp
@@ +963,5 @@
>  
> +    // A JS_OPTIMIZED_ARGUMENTS magic value means that 'arguments' flows into
> +    // this apply call and, as an optimization, we've avoided creating it since
> +    // apply can simply pull the argument values from the calling frame.
> +    if (args[1].isMagic(JS_OPTIMIZED_ARGUMENTS)) {

nit: Also mention in the that JS_OPTIMIZED_ARGUMENTS implies that the frame above is a scripted frame.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8384960 [details] [diff] [review]
tidy-up-args-1

Review of attachment 8384960 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Luke Wagner [:luke] from comment #4)
> If you want to land whatever part is already reviewed, go for it. 
> Otherwise, this patch is really pretty simple changes, so it should be
> pretty easy to rebase if it went in first.

Thanks, I did.

Comment 8

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83b071c4a4df
https://hg.mozilla.org/integration/mozilla-inbound/rev/703bd71926e4

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83b071c4a4df
https://hg.mozilla.org/mozilla-central/rev/703bd71926e4