Closed Bug 601256 Opened 10 years ago Closed 9 years ago

JM: optimize globals in global-called eval

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dmandelin, Assigned: dmandelin)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Spun off from bug 599009 comment 8 and comment 12. We need to be faster on this:

  eval("var z = 0; function g() { for (var y = 0; y < 10000000; ++y) ++z }");
  g();

The problem is that the "++z" ends up as JSOP_INCNAME. This gets IC'd, but the IC is longer and slower than the IC for JSOP_INCGNAME. So, we should compile to JSOP_INCGNAME in this case instead.

In general, the GNAME ops apply to a name reference where the name cannot be refer to anything other than a global variable. When |eval| is called at the top level, if a variable inside is free within the eval script, then the only scope left is the global scope.

The GLOBAL ops are even faster, but I don't think they apply here. They require that the name is a non-configurable global variable. I didn't recheck the spec, but testing with the shell says they are configurable.
Accursed ES1, due to MSFT contributions whose rationale I forget or never heard, makes eval-created vars (which go in eval's dynamic scope) configurable (or as the old specs said, not DontDelete).

Good news is ES5 strict and any future JS standard (which will be based on ES5 strict by the Harmony accords) won't let eval create vars in the dynamic scope, but that's cold comfort for now. Although we *could* start optimizing strict eval now as it makes the vars not only bound in a declarative environment record for the eval, so not configurable.

/be
Attached patch PatchSplinter Review
Attachment #481057 - Flags: review?(dvander)
Comment on attachment 481057 [details] [diff] [review]
Patch


>+          case JSOP_SETCONST:
>+          case JSOP_DELNAME:
>+          case JSOP_FORNAME:
>+            /* Not supported. */
>+            return JS_TRUE;

This should return false, and

>+        /* Optimize accesses to undeclared globals. */
>+        TryConvertToGname(cg, pn, &op);

This should check for false, and in that case, return JS_TRUE

r=me with those fixed
Attachment #481057 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/d3a651536469
Status: NEW → ASSIGNED
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/d3a651536469
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.