Closed
Bug 409526
Opened 17 years ago
Closed 17 years ago
QueryInterface functions in calUtils.js can be faster
Categories
(Calendar :: Internal Components, defect)
Calendar
Internal Components
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: mvl, Assigned: mvl)
Details
Attachments
(2 files)
929 bytes,
text/plain
|
Details | |
1.60 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
The QueryInterface related functions (doQueryInterface and ensureIID) in calUtils.js are not taking a lot of time, but they are called very often, so any improvement helps. They should be as fast as possible. (I found out that they take a noticeable amount of time when using the propertybag from bug 382121 and then profiling the parsing of an ics file into a calendar. The time spend doing QueryInterface on the properties was a few percent of the total time)
Flags: wanted-calendar0.8?
Assignee | ||
Comment 1•17 years ago
|
||
I tried a different approach that prevents a function call for every check. It's a bit faster. See the testscript. The 'for' approach takes about 3000msec instead of 4600msec for the 'some'. The 'some' is the one that's currently in calUtils.js.
Comment 2•17 years ago
|
||
This should block 0.8 as efforts to improve performance are a major goal for 0.8.
Flags: wanted-calendar0.8? → blocking-calendar0.8+
Assignee | ||
Comment 3•17 years ago
|
||
This patch implements the 'for' approach from the xpcshell script.
Comment 4•17 years ago
|
||
Comment on attachment 294348 [details] [diff] [review] patch v1 >+ var iid; >+ for each (iid in aList) { Any reason for outlining the declaration of iid? Why not for each (var iid on aList)? looks good; r=dbo
Attachment #294348 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 5•17 years ago
|
||
patch checked in
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Target Milestone: --- → 0.8
Version: Trunk → unspecified
You need to log in
before you can comment on or make changes to this bug.
Description
•