JS-IRC spends more time than it needs in hasOwnProperty calls

RESOLVED FIXED in 1.5

Status

Chat Core
IRC
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
*** Original post on bio 2166 at 2013-09-10 22:38:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
(Assignee)

Comment 1

4 years ago
Created attachment 8354631 [details] [diff] [review]
Patch

*** Original post on bio 2166 as attmnt 2861 at 2013-09-10 22:38:00 UTC ***

When calling our hasOwnProperty() helper, most of the time seems to be spent traversing cross compartment wrappers, so I think inlining the method makes sense in the _handleMessage method which is called plenty of times.

In ircWatchMonitor.jsm I just simplified (or I think so at least).

The portion of the jank time spent in IRC code on hasOwnProperty and irc*.isEnabled() calls while receiving LIST results seems to have decreased from ~2% to 0.5% after applying this patch, but my machine may be way too fast to have results that are precise enough here.

I don't necessarily care about the attached patch specifically; I'm more offering it as a productive way to start a discussion about these hasOwnProperty calls to see how we can improve the situation.
Attachment #8354631 - Flags: review?(clokep)
Comment on attachment 8354631 [details] [diff] [review]
Patch

*** Original change on bio 2166 attmnt 2861 at 2013-09-10 23:33:50 UTC ***

I'm OK with these changes.
Attachment #8354631 - Flags: review?(clokep) → review+
*** Original post on bio 2166 at 2013-09-10 23:48:51 UTC ***

Assigning.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Whiteboard: [checkin-needed]
Comment on attachment 8354631 [details] [diff] [review]
Patch

*** Original change on bio 2166 attmnt 2861 at 2013-09-11 17:41:00 UTC ***

As far as I can tell, we'll no longer need imXPCOMUtils.jsm in ircHandlers.jsm, we should remove it.
Attachment #8354631 - Flags: review+ → review-
Whiteboard: [checkin-needed]
(Assignee)

Comment 5

4 years ago
Created attachment 8354650 [details] [diff] [review]
Patch v2

*** Original post on bio 2166 as attmnt 2880 at 2013-09-12 17:04:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354650 - Flags: review?(clokep)
(Assignee)

Comment 6

4 years ago
Comment on attachment 8354631 [details] [diff] [review]
Patch

*** Original change on bio 2166 attmnt 2861 at 2013-09-12 17:04:23 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354631 - Attachment is obsolete: true
Comment on attachment 8354650 [details] [diff] [review]
Patch v2

*** Original change on bio 2166 attmnt 2880 at 2013-09-12 17:05:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354650 - Flags: review?(clokep) → review+
*** Original post on bio 2166 at 2013-09-12 23:03:40 UTC ***

http://hg.instantbird.org/instantbird/rev/39d7b20159b3
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
You need to log in before you can comment on or make changes to this bug.