622053 - Devolve CallPropertyOp into its callers, for great assertiveness/readability

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

CallPropertyOp is a gargantuan Swiss army knife -- or rather, it would be if you mean a knife with a fork whose tines are a bottle opener, a leather hole punch, and a toothpick, with a spoon whose bowl is a compass, and with a magnifying glass embedded in the largest blade on it.  In other words, it's thoroughly intermingled madness.  Getting rid of it and moving all its code into its callers would vastly simplify the code, and it would also enable doing a ton more asserting for in-range vector accesses and the like.  I did this as a prerequisite to optimizing strict eval frames.  That's more effort than is feasible right now (or so it seems to me), but the cleanups and extra assertions here are well worth having even without an immediate motivation in optimizing strict mode eval.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: The guts of it

This is the guts of the patch: the new implementations for the methods, in jsfun.cpp.  The diff is not exactly the most readable thing in the world, although I am sure Bugzilla's diff viewer probably improves on reading the patch file directly.  :-)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Fwiw, this might have perf benefits too; back when I was optimizing call/arguments stuff in TM we ended up handwriting inline LIR after trying custom getters/setters, but even the custom things were way faster than the CallPropertyOp mess.  But the new things seem to be closer to that custom code, so if JM calls them it could benefit.

Comment 3

[David Mandelin [:dmandelin]]

Comment on attachment 500305 [details] [diff] [review]
Patch

It's pretty readable in the diff viewer. I'm glad someone is finally ripping that thing out.

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/c5d43dfafcbc

Comment 5

[Nicholas Nethercote [inactive]]

Waldo++

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/c5d43dfafcbc