730836 - IonMonkey: Don't call GetPcScript from SetObjectElement

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

The Kraken audio tests spend quite some time recovering the script/pc in SetObjectElement. SetObjectElement doesn't need the script/pc if we:

1) templatize SetObjectElement to get rid of script->strictMode
2) Remove the script/pc arguments from MonitorAssign (they are unused)
3) Only update the arrayWriteHole ScriptAnalysis flag when running in the interpreter.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch for inbound

When this merges to Ion we can add the cx->fp()->runningInIon() check.

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 600956 [details] [diff] [review]
Patch for inbound

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

Apply nit, and r=me.

::: js/src/jsinfer.h
@@ +1121,1 @@
>  

Good catch.

::: js/src/jsinterp.cpp
@@ +2641,5 @@
>      jsid id;
>      FETCH_ELEMENT_ID(obj, -2, id);
>      Value &value = regs.sp[-1];
> +    if (script->strictModeCode) {
> +        if (!SetObjectElementOperation<true>(cx, obj, id, value))

nit: Use an argument instead of the template parameter.  Based on the size of the function and on the number of jump made with this boolean, there is no good reasons to use a template here, even if this remove one constant argument from IonMonkey stack.

Comment 3

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fed91e95417

Please leave this bug open when merging this to m-c.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Ion patch

Comment 5

[Nicolas B. Pierron [:nbp]]

Comment on attachment 601373 [details] [diff] [review]
Ion patch

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

Looks good.

::: js/src/jsinterpinlines.h
@@ +816,5 @@
>                  }
>                  obj->setDenseArrayElementWithType(cx, i, value);
>                  return true;
>              } else {
> +                if (!cx->fp()->runningInIon()) {

May be add comment which explain that if we ever hit this branch this means that we have already visit it during the interpreter phase.

By the way, I will soon land a patch to improve GetPcScript, so this would not be necessary.

Comment 6

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/9fed91e95417

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

checked-in as: http://hg.mozilla.org/projects/ionmonkey/rev/3a1754a67644