Bug 793345 Comment 25 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Yes, the issue is around the top-level variables.

`executeInGlobalWithBindings` is almost equivalent of `with (nenv) { ... }` statement, with `nenv` object created below.

this means:
  * if the variable name exists in the passed `bindings`, it becomes the `nenv`'s propety, even if `var` declaration is inside the code
  * if the variable name doesn't exist in the passed `bindings`, it becomes the current `this`'s property

and this operation is not optimized.

https://searchfox.org/mozilla-central/rev/ffb50da3ca89100b6ae5054cfe69c187679515f0/js/src/debugger/Frame.cpp#1006-1009,1049-1051,1056,1061
```cpp
Result<Completion> js::DebuggerGenericEval(
    JSContext* cx, const mozilla::Range<const char16_t> chars,
    HandleObject bindings, const EvalOptions& options, Debugger* dbg,
    HandleObject envArg, FrameIter* iter) {
...
  // If evalWithBindings, create the inner environment.
  if (bindings) {
    Rooted<PlainObject*> nenv(cx, NewPlainObjectWithProto(cx, nullptr));
...
    for (size_t i = 0; i < keys.length(); i++) {
...
          !NativeDefineDataProperty(cx, nenv, id, val, 0)) {
```

Then, if you enclose the code with function, top-level variables become function's local slot, and the access is optimized.

Also, the effect of this issue is slightly smaller for non-top-level lexical variables, because it doesn't lookup in `nenv` and location is known:
```
var now = Date.now();
for (let i = 0; i < 100000000; i++) { }
console.log('loop took ' + (Date.now() - now) + ' ms');
```

Then, I think the solution depends on what actual behavior the Console and WebDriver expects for Debugger.

Given the passed `bindings` is copied and treated like read-only, some behavior shared with `with` statement aren't really necessary, and the handling of the top-level variables could be simplified, such like, "if there's variable declaration, don't use the passed `bindings`, and always use local binding".
The passed `bindings` date needs to be checked only for accesses for unknown variables, both for get and set.

This way, the not-optimized variable handling doesn't happen inside the loop,
and possibly reduce the effect of non-syntactic scope to the JIT compilation.
Yes, the issue is around the top-level variables.

`executeInGlobalWithBindings` is almost equivalent of `with (nenv) { ... }` statement, with `nenv` object created below.

this means:
  * if the variable name exists in the passed `bindings`, it becomes the `nenv`'s propety, even if `var` declaration is inside the code
  * if the variable name doesn't exist in the passed `bindings`, it becomes the current `this`'s property

and this operation is not optimized.

https://searchfox.org/mozilla-central/rev/ffb50da3ca89100b6ae5054cfe69c187679515f0/js/src/debugger/Frame.cpp#1006-1009,1049-1051,1056,1061
```cpp
Result<Completion> js::DebuggerGenericEval(
    JSContext* cx, const mozilla::Range<const char16_t> chars,
    HandleObject bindings, const EvalOptions& options, Debugger* dbg,
    HandleObject envArg, FrameIter* iter) {
...
  // If evalWithBindings, create the inner environment.
  if (bindings) {
    Rooted<PlainObject*> nenv(cx, NewPlainObjectWithProto(cx, nullptr));
...
    for (size_t i = 0; i < keys.length(); i++) {
...
          !NativeDefineDataProperty(cx, nenv, id, val, 0)) {
```

Then, if you enclose the code with function, top-level variables become function's local slot, and the access is optimized.

Also, the effect of this issue is slightly smaller for non-top-level lexical variables, because it doesn't lookup in `nenv` and location is known:
```
var now = Date.now();
for (let i = 0; i < 100000000; i++) { }
console.log('loop took ' + (Date.now() - now) + ' ms');
```

Then, I think the solution depends on what actual behavior the Console and WebDriver expects for Debugger.

Given the passed `bindings` is copied and treated like read-only, some behavior shared with `with` statement aren't really necessary, and the handling of the top-level variables could be simplified, such like, "if there's variable declaration, don't use the passed `bindings`, and always use local binding".
The passed `bindings` data needs to be checked only for accesses for unknown variables, both for get and set.

This way, the not-optimized variable handling doesn't happen inside the loop,
and possibly reduce the effect of non-syntactic scope to the JIT compilation.

Back to Bug 793345 Comment 25