532696 - Aborts on prototype code due to "lambda" (dromaeo and otherwise)

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Still looking at the dromaeo style-prototype tests.  With the change I mention in bug 532695 I get new interesting aborts:

   3 Abort recording of tree /web/lib/prototype.js:85@90 at web/lib/prototype.js:90@166: lambda.
   3 Abort recording of tree web/tests/jslib-style-prototype.html:22@9 at dromaeo/web/lib/prototype.js:712@25: lambda.

Code around line 90:

        value = (function(m) {
          return function() { return ancestor[m].apply(this, arguments) };
        })(property).wrap(method);

(in the addMethods function in prototype.js).  Line 90 is the return function() bit there.

Code around line 712:

  invoke: function(method) {
    var args = $A(arguments).slice(1);
    return this.map(function(value) {
      return value[method].apply(value, args);
    });
  },

Line 712 is the |return this.map(...)| line.

Comment 1

[Boris Zbarsky [:bzbarsky]]

ccing usual suspects.

Comment 2

[Brendan Eich [:brendan]]

This looks like bug 493754 -- feel free to dup. Anyone want to take that bug?

/be

Comment 3

[Brendan Eich [:brendan]]

Actually, the line 712 case:

  invoke: function(method) {
    var args = $A(arguments).slice(1);
    return this.map(function(value) {
      return value[method].apply(value, args);
    });
  },

is not aborting due to bug 493754, i.e. because the inner lambda can't be optimized to a flat closure due to looking through a funarg -- note how the inner lambda uses only its argument (value) and two upvars that are only initialized, where the initializations dominate the inner lambda (method, args).

But a flat closure cannot be formed if such an upvar could be assigned after being initialized. That's the problem here: the outer lambda uses arguments, and we don't know how it might abuse it to alias |method|.

Indeed without more analysis, $A could do something crazy such as mutate its one formal parameter, iterable, via iterable.method = 42. So the flat closure optimization is kicking in for good reason here.

/be

Comment 4

[Brendan Eich [:brendan]]

The line 90 case is bug 493754, though. Boris, feel free to comment there and decouple these bugs if you like. This one can be all about arguments aliasing defeating flat closure optimization.

To handle the arguments aliasing hazard is hard without fuller-program analysis. If we try at runtime to compensate, we may have to deoptimize flat closures into full ones. We have some code to do this in certain cases, due to the debugger API and legit demand for debuggers to be able to evaluate in an optimized scope. But it is not complete.

/be

Comment 5

[David Mandelin [:dmandelin]]

Line 712 looks like bug 495331. There is a patch that's basically done, but it caused a regression on some platform on tss, so it got backed out. I haven't had time to figure that out and get it landed.

Comment 6

[André Bargull [:anba]]

Old TM bug, no longer valid.