Closed
Bug 532563
Opened 15 years ago
Closed 15 years ago
XPCOMUtils has anonymous functions
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: dietrich, Assigned: dietrich)
Details
Attachments
(1 file)
3.18 KB,
patch
|
sdwilsh
:
review+
|
Details | Diff | Splinter Review |
makes profiling difficult
Assignee | ||
Updated•15 years ago
|
Attachment #415761 -
Flags: review?(sdwilsh)
Updated•15 years ago
|
Attachment #415761 -
Attachment is patch: true
Attachment #415761 -
Attachment mime type: application/octet-stream → text/plain
Updated•15 years ago
|
Assignee: nobody → dietrich
Status: NEW → ASSIGNED
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9.3a1
Updated•15 years ago
|
Attachment #415761 -
Flags: review?(sdwilsh) → review+
Comment 1•15 years ago
|
||
Comment on attachment 415761 [details] [diff] [review]
v1
r=sdwilsh
Assignee | ||
Comment 2•15 years ago
|
||
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.
Attachment #415761 -
Flags: approval1.9.2?
Assignee | ||
Comment 3•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 4•15 years ago
|
||
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.]
Assignee | ||
Comment 5•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
(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•15 years ago
|
||
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•15 years ago
|
||
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.
Updated•15 years ago
|
Attachment #415761 -
Flags: approval1.9.2? → approval1.9.2.2?
Comment 10•15 years ago
|
||
Comment on attachment 415761 [details] [diff] [review]
v1
Do we still need this on the branches?
Attachment #415761 -
Flags: approval1.9.2.2?
Comment 11•15 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•