redundant calls to unifinder refreshEventTree

RESOLVED WORKSFORME

Status

Calendar
Calendar Views
--
trivial
RESOLVED WORKSFORME
11 years ago
9 years ago

People

(Reporter: gekacheka, Assigned: gekacheka)

Tracking

({perf})

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.13pre) Gecko/20080222 Calendar/0.8pre

Unifinder refreshEventTree is called multiple times when a calendar is (re)loaded.

Reproducible: Always

Steps to Reproduce:
0. If needed, Install venkman javascript debugger 
   https://addons.mozilla.org/en-US/sunbird/addon/216
1. Tools | Javascript Debugger
1a. Profile | Collect Profile Data (toggle on)
2. Sunbird | Calendars (tab) | right click | Reload remote calendars
3. JavaScript Debugger | Profile | Collect Profile Data (toggle off)
4. JavaScript Debugger | Profile | Save Profile Data as... (profile.txt)
5. open profile.txt file in a text editor, 
   search for refreshEventTree and refreshEventTreeInternal
3.
Actual Results:  
refreshEventTree is called more than once

    Function Name: refreshEventTree  (Lines 784 - 889)
    Total Calls: 2 (max recurse 0)
    Total Time: 10.01 (min/max/avg 0/10.01/5.01)
    Time (ex. calls): 0 (min/max/avg 0/0/0)

    Function Name: refreshEventTreeInternal  (Lines 891 - 899)
    Total Calls: 2 (max recurse 0)
    Total Time: 3054.39 (min/max/avg 1011.46/2042.94/1527.2)
    Time (ex. calls): 40.06 (min/max/avg 20.03/20.03/20.03)



Expected Results:  
refreshEventTree is called once

    Function Name: refreshEventTree  (Lines 774 - 879)
    Total Calls: 1 (max recurse 0)
    Total Time: 0 (min/max/avg 0/0/0)
    Time (ex. calls): 0 (min/max/avg 0/0/0)

    Function Name: refreshEventTreeInternal  (Lines 881 - 889)
    Total Calls: 1 (max recurse 0)
    Total Time: 1662.39 (min/max/avg 1662.39/1662.39/1662.39)
    Time (ex. calls): 10.01 (min/max/avg 10.01/10.01/10.01)


refreshEventTree doesn't take much time, but refreshEventTreeInternal does, expecially on a calendar with many repeating occurrences, such as
http://ical.mac.com/ebony.ivory/Musicians%27%20Birthdays.ics
(Assignee)

Updated

11 years ago
Blocks: 415442
Keywords: perf
(Assignee)

Comment 1

11 years ago
Created attachment 305245 [details] [diff] [review]
v1 patch: remove redundant unifinder refreshes (onEndBatch is sufficient)

(patch -l -p 1 -i file.patch)

The refreshEvenTree() calls in prepareCalendarUnifinder and onLoad seem to be redundant.  Refreshing the unifinder in these cases is covered by onEndBatch.
Assignee: nobody → gekacheka
Status: NEW → ASSIGNED
Attachment #305245 - Flags: review?(philipp)
Comment on attachment 305245 [details] [diff] [review]
v1 patch: remove redundant unifinder refreshes (onEndBatch is sufficient)

Moving review to berend, I will be away till thursday.
Attachment #305245 - Flags: review?(philipp) → review?(Berend.Cornelius)
Comment on attachment 305245 [details] [diff] [review]
v1 patch: remove redundant unifinder refreshes (onEndBatch is sufficient)

>-        }
>-
>-        // Display something upon first load. onLoad doesn't work properly for
>-        // observers
>-        if (!isUnifinderHidden()) {
>-            gUnifinderNeedsRefresh = false;
>-            refreshEventTree();
This change seems to break things for me: When the application is started, no events are shown in the unifinder. Hiding the unifinder and showing it again is the only way to get events into the unifinder.

r- for now, followup welcome.
Attachment #305245 - Flags: review?(Berend.Cornelius) → review-
gekacheka, please change the status to NEW if you no longer work on this issue.
Thanks.
(Assignee)

Comment 5

9 years ago
Seems ok now, refreshEventTreeInternal is only called once.

    Function Name: refreshEventTreeInternal  (Lines 924 - 936)
    Total Calls: 1 (max recurse 0)
    Total Time: 1527.34 (min/max/avg 1527.34/1527.34/1527.34)
    Time (ex. calls): 7.82 (min/max/avg 7.82/7.82/7.82)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.