Closed
Bug 477092
Opened 15 years ago
Closed 15 years ago
Several frequently-called AvmCore methods could be static, but aren't
Categories
(Tamarin Graveyard :: Virtual Machine, defect)
Tamarin Graveyard
Virtual Machine
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: stejohns, Assigned: stejohns)
Details
Attachments
(1 file, 1 obsolete file)
111.35 KB,
patch
|
lhansen
:
review+
|
Details | Diff | Splinter Review |
e.g., number(), integer(), boolean(). Means one less argument to pass, and a tiny reduction in jitted code size.
Comment 1•15 years ago
|
||
these have to call valueOf() indirectly, they aren't static on purpose
Assignee | ||
Comment 2•15 years ago
|
||
(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•15 years ago
|
||
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).
Assignee | ||
Comment 4•15 years ago
|
||
yep, and it moves the needle a little on some perf tests... up to 3% in a handful. Patch forthcoming later.
Assignee | ||
Comment 5•15 years ago
|
||
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.
Updated•15 years ago
|
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
Assignee | ||
Comment 6•15 years ago
|
||
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 :-)
Attachment #362623 -
Flags: review?(lhansen)
Comment 7•15 years ago
|
||
Comment on attachment 362623 [details] [diff] [review] Patch General approach seems fine.
Attachment #362623 -
Flags: review?(lhansen) → review+
Assignee | ||
Comment 8•15 years ago
|
||
same patch, rebased for current tip. this would be nice to get into redux before the next release to TC.
Assignee: nobody → stejohns
Attachment #362623 -
Attachment is obsolete: true
Attachment #366626 -
Flags: review?(lhansen)
Updated•15 years ago
|
Attachment #366626 -
Flags: review?(lhansen) → review+
Assignee | ||
Comment 9•15 years ago
|
||
pushed to redux as changeset: 1581:0af36f31b8fc
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 10•15 years ago
|
||
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•