Closed
Bug 454543
Opened 16 years ago
Closed 16 years ago
Calendar Multiday-View: Performance improvement is required
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
VERIFIED
FIXED
1.0b1
People
(Reporter: berend.cornelius09, Assigned: berend.cornelius09)
Details
Attachments
(1 file, 1 obsolete file)
17.13 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
One of the first steps in the process of arranging the eventboxes in the calendar-multiday-view (calendar-day-view or calendar-week-view) is to sort the eventboxes according to the start and end date. One other step is to calculate the duration of the event considering a certain minimum. Both steps should be consolidated because they contain constant repeating calculations processed in a loop. This has been noticed already by mvl and me (see Bug 454195 comment#5 and bug437412 comment#9).
Flags: wanted-calendar0.9?
Assignee | ||
Comment 1•16 years ago
|
||
In this patch I wrapped an EventInfo object around the event array and added the 'layoutStart' and 'layoutEnd' members according to which the events are now sorted. By doing so I reduce the sorting process and also take care that the 'layoutStart' and 'layoutEnd' values are not calculated again and again. Besides that it was a bug to sort the events according to the 'startDate' and "endDate' and not according to the 'visual' representations "layoutStart' and 'layoutEnd'
Assignee: nobody → Berend.Cornelius
Status: NEW → ASSIGNED
Attachment #337841 -
Flags: review?(daniel.boelzle)
Comment 2•16 years ago
|
||
I think the changes are too risky for 0.9 this late in the game. Setting qawanted on the patch, since view changes are quite too often good for regressions.
Flags: wanted-calendar0.9? → wanted-calendar0.9-
Whiteboard: qawanted
Updated•16 years ago
|
Flags: wanted-calendar0.9- → wanted-calendar1.0+
Assignee | ||
Comment 3•16 years ago
|
||
I unbitrotted the first patch...
Attachment #337841 -
Attachment is obsolete: true
Attachment #343211 -
Flags: review?(daniel.boelzle)
Attachment #337841 -
Flags: review?(daniel.boelzle)
Comment 4•16 years ago
|
||
I checked this patch and it looks okay. One issuee I found was bug 460030, but this bug is not caused by Berends patch.
Comment 5•16 years ago
|
||
Comment on attachment 343211 [details] [diff] [review] patch v. #2 I think you could use |let| instead of |var| throughout the patch. >+ var item = aEventInfo.event.clone(); >+ var start = item.startDate || item.entryDate; >+ if (!compareObjects(start.timezone, self.mTimezone)) { >+ start = start.getInTimezone(self.mTimezone); >+ } >+ aEventInfo.layoutStart = start; >+ var end = item.endDate || item.dueDate >+ if (!compareObjects(end.timezone, self.mTimezone)) { >+ end = end.getInTimezone(self.mTimezone); >+ } I am not sure if the "compare-then-convert" really buys us better performance. IMO it's worth to measure whether simply calling end = end.getInTimezone(self.mTimezone) in any case is better. r=dbo
Attachment #343211 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 6•16 years ago
|
||
I observed the comments of Daniel. I think that the replacement of all "vars" agains "lets" should be done in a separate bug over the whole file. pushed patch to comm-central http://hg.mozilla.org/comm-central/rev/588651b7b680
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Updated•16 years ago
|
Whiteboard: qawanted
Comment 7•16 years ago
|
||
Checked in lightning build 2008102203557 and sunbird 20081022 -> VERIFIED.
Status: RESOLVED → VERIFIED
Comment 8•13 years ago
|
||
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in
before you can comment on or make changes to this bug.
Description
•