517519 - Several minor optimizations suggested by profiling

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, enhancement)

Description

[Steven Johnson]

roll-up bug for a few minor optimizations suggested by VTune/Shark.

Comment 1

[Steven Johnson]

Attachment: unroll findInterfaceAddr into callers

The vast majority of calls to findInterfaceAddr are from containsInterface, but that code path requires and extra load-and-test (unless the compiler is far smarter than is likely). Unrolling into addOneInterface/containsInterface drops the next Instructions Retired and Clockticks for containsInterface by ~50%, according to VTune. (ie, a tiny but positive gain.)

Comment 2

[Steven Johnson]

Attachment: remove inner-loop test from Dictionary::nextNameIndex

Dictionary::nextNameIndex can be fairly hot in some flex apps, and has a fairly-uncommon special case (weak keys) in its inner loop that can easily be hoisted. Along with some other trivial optimization, this drops Clockticks/IR by ~40% for typical testcases.

Comment 3

[Edwin Smith]

Comment on attachment 401488 [details] [diff] [review]
unroll findInterfaceAddr into callers

is it at all possible to do this inlining with REALLY_INLINE, perhaps?  manually is okay given no alternatives but we're starting to do lots of it, i worry about maintenance getting more expensive.

Comment 4

[Steven Johnson]

(In reply to comment #3)
> (From update of attachment 401488 [details] [diff] [review])
> is it at all possible to do this inlining with REALLY_INLINE, perhaps? 

It possible, I guess -- there is one subtle difference between the two loops (order of checking) but it's pretty trivial performance-wise. I guess I'm a little shy about using REALLY_INLINE where unrolling looks like a maintainable alternative -- IIRC didn't Tommy/Lars recently have an issue where they had to go back to explicit code, because some compiler wasn't REALLY_INLINE-ing things?

Comment 5

[Lars T Hansen]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 401488 [details] [diff] [review] [details])
> > is it at all possible to do this inlining with REALLY_INLINE, perhaps? 
> 
> It possible, I guess -- there is one subtle difference between the two loops
> (order of checking) but it's pretty trivial performance-wise. I guess I'm a
> little shy about using REALLY_INLINE where unrolling looks like a maintainable
> alternative -- IIRC didn't Tommy/Lars recently have an issue where they had to
> go back to explicit code, because some compiler wasn't REALLY_INLINE-ing
> things?

I can't remember having problems with REALLY_INLINE; beyond that I have no opinion.  I don't /trust/ the compiler with REALLY_INLINE, because not all compilers have it, but macros get unmaintainable very quickly.

Comment 6

[Steven Johnson]

well, to reiterate, it's certainly possible to encapsulate it with REALLY_INLINE, but we will end up re-introducing one of the outer tests that we eliminated.

Comment 7

[Steven Johnson]

pushed as changeset:   2595:1d0a7298b2c6 and changeset:   2596:a702626ebbf3

Comment 8

[Chris Peyer]

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