477092 - Several frequently-called AvmCore methods could be static, but aren't

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

e.g., number(), integer(), boolean(). Means one less argument to pass, and a tiny reduction in jitted code size.

Comment 1

[Edwin Smith]

these have to call valueOf() indirectly, they aren't static on purpose

Comment 2

[Steven Johnson]

(In reply to comment #1)
> these have to call valueOf() indirectly, they aren't static on purpose

I don't understand... valueOf is a pure AS3 function, it doesn't exist in C++.

Comment 3

[Edwin Smith]

i stand corrected, i was thinking of double->Atom conversion too, but not in play here.  sure, if these become static and stuff compiles, super.  (CodegenLIR/MIR need updating, but otherwise no biggie).

Comment 4

[Steven Johnson]

yep, and it moves the needle a little on some perf tests... up to 3% in a handful. Patch forthcoming later.

Comment 5

[Steven Johnson]

Turns out that istype() can also be made static (and faster) by rewriting using the BUILTIN_ types, which allows a bunch of others to be staticified as well. Perf increase is modest but worthwhile.

Comment 6

[Steven Johnson]

Attachment: Patch

Initial patch that converts a slew of AvmCore methods to static. Performance increase is modest in JIT mode on most benchmarks, but math-centric benchmarks (eg primes, crypto) show 15-25% improvement in JIT mode. Also added some minor but worthwhile optimizations to the bodies of AvmCore::number, AvmCore::istype, Toplevel::add2

Further benchmarking and testing is necessary before landing (only MacTel benchmarking has been done), this is just an initial advisory patch... no rush on a review :-)

Comment 7

[Lars T Hansen]

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

General approach seems fine.

Comment 8

[Steven Johnson]

Attachment: Patch, rebased

same patch, rebased for current tip. this would be nice to get into redux before the next release to TC.

Comment 9

[Steven Johnson]

pushed to redux as changeset:   1581:0af36f31b8fc

Comment 10

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.

Comment 11

[Brent Baker]

removing QRB request, bug resolved/verified