532043 - Using destructuring assignment to swap array elements doesn't trace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

js shell testcase attached.  When I run it, I get:

Abort recording of tree /Users/bzbarsky/test.js:7@224 at /Users/bzbarsky/test.js:8@248: enumelem.
Abort recording of tree /Users/bzbarsky/test.js:4@192 at /Users/bzbarsky/test.js:7@224: No compatible inner tree.
Abort recording of tree /Users/bzbarsky/test.js:7@224 at /Users/bzbarsky/test.js:8@248: enumelem.

recorder: started(3), aborted(3), completed(0), different header(0), trees trashed(0), slot promoted(0), unstable loop variable(0), breaks(0), returns(0), merged loop exits(0), unstableInnerCalls(1), blacklisted(0)
monitor: exits(0), timeouts(0), type mismatch(0), triggered(0), global mismatch(0), flushed(0)

I don't understand why this hits unstable inner stuff (is it because the inner failed to trace to start with?), but the enumelem thing seems to be pretty specific to this use of destructuring assignment.

This came up in a xulrunner app in which someone was blaming imagedata performance for his performance woes, wheareas in reality it mostly doesn't trace well.

Comment 1

[Andreas Gal :gal]

I this this as success. Our failures to trace are definitely getting more obscure now. End in sight?

Comment 2

[Boris Zbarsky [:bzbarsky]]

The actual xul app in question has a few dozen other aborts; it's just pointless to worry about them for its purposes until this one is fixed, since this one is in its innermost loops.

I'm also still waiting on permission from the author to attach the app as a whole to a bug; this particular abort was just easy to write a testcase for....

So end might be in sight, but dunno that this bug is evidence either way.  ;)

Comment 3

[Brendan Eich [:brendan]]

Attachment: easy to fix

Comment 4

[Brendan Eich [:brendan]]

Super-safe, could ride along into 3.6 after baking IMHO -- realize it's too late, just sayin'. Maybe 3.6.1.

/be

Comment 5

[Andreas Gal :gal]

Comment on attachment 415448 [details] [diff] [review]
easy to fix

Wow. Our bytecode set is ... interesting.

Comment 6

[Brendan Eich [:brendan]]

(In reply to comment #5)
> (From update of attachment 415448 [details] [diff] [review])
> Wow. Our bytecode set is ... interesting.

JSOP_ENUMELEM predates JSOP_PICK by a decade give or take, and it could be eliminated at the cost of pick before setelem. As the name suggests, enumelem came in with ECMA-262 compatible for-in implementation, done by rogerl in the late '90s. I may have advised ;-).

/be

Comment 7

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/03a0733a1cc3
http://hg.mozilla.org/tracemonkey/rev/d2fe76589155 (the trace-test testcase)

/be

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/03a0733a1cc3