532563 - XPCOMUtils has anonymous functions

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Dietrich Ayala (:dietrich)]

Attachment: v1

makes profiling difficult

Comment 1

[Shawn Wilsher :sdwilsh]

Comment on attachment 415761 [details] [diff] [review]
v1

r=sdwilsh

Comment 2

[Dietrich Ayala (:dietrich)]

Comment on attachment 415761 [details] [diff] [review]
v1

requesting approval: no logic changes, just naming anonymous functions so they're detectable in performance profiling logs.

Comment 3

[Dietrich Ayala (:dietrich)]

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

Comment 4

[Justin Dolske [:Dolske]]

Do non-anonymous functions still hurt performance? At least in the past, this was a reason to keep anonymous functions. [Though I don't think the functions here are called often enough for it to matter.]

Comment 5

[Dietrich Ayala (:dietrich)]

Shaver said named functions are more costly than anonymous functions, but he didn't say to what degree.

I assuming it's not much, or we'd have a strict "no named functions!" policy across the codebase. If anything, there's been a trend towards naming, since the advent of the dtrace probes.

Comment 6

[Robert Strong (they/them - no direct email)]

Some numbers from a tegra with this patch that I sent to sdwilsh last night

When running WinCE Firefox on the Tegra
File           line function                     Calls Total Avg   Max   Min
XPCOMUtils.jsm 246  XPCU_defineLazyGetter        10    4.153 0.415 0.492 0.343
XPCOMUtils.jsm 267  XPCU_defineLazyServiceGetter  6    3.945 0.657 0.73  0.57

The first usage of any JS always has a hefty one time cost in js_Execute which is around 16ms for XPCOMUtils.jsm and around 5ms to import the first time and around .6ms each subsequent import.

If there is a cost it is either front loaded and / or very small

Comment 7

[Robert Strong (they/them - no direct email)]

(In reply to comment #5)
> Shaver said named functions are more costly than anonymous functions, but he
> didn't say to what degree.
> 
> I assuming it's not much, or we'd have a strict "no named functions!" policy
> across the codebase. If anything, there's been a trend towards naming, since
> the advent of the dtrace probes.
I removed the 325 named anonymous functions in the startup path and there is no apparent cost associated with naming anonymous functions. I just talked with Brendan and he said that his upvar2 work for 3.6 removed the cost associated with naming anonymous functions. Yay!

Comment 8

[Nickolay_Ponomarev]

Heh, note that Brendan also commented on this very code in bug 381651 comment 3:
> Name your functions for venkman or firebug? Is that really necessary for
> firebug? It adds redundancy for no other benefit. I think debuggers should
> cope.

Not arguing though, if it makes Firefox faster :)

Comment 9

[Robert Strong (they/them - no direct email)]

Yep, there used to be a cost associated with it and now there isn't after Brendan's upvar2 work. It doesn't make it faster or slower but it does make it easier to identify the code in profiles, etc.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 415761 [details] [diff] [review]
v1

Do we still need this on the branches?

Comment 11

[Shawn Wilsher :sdwilsh]

(In reply to comment #10)
> (From update of attachment 415761 [details] [diff] [review])
> Do we still need this on the branches?
It would be nice to have, but we can live without it.  It's not like it's any code change; just makes profiling tools more usable.