[Startup Performance] Defer loading bookmarks until UI has finished loading

RESOLVED FIXED in mozilla0.9.7

Status

SeaMonkey
Bookmarks & History
P1
normal
RESOLVED FIXED
19 years ago
13 years ago

People

(Reporter: Robert John Churchill, Assigned: Paul Chen)

Tracking

Trunk
mozilla0.9.7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

19 years ago
Defer loading bookmarks.html until UI has finished loading. On the Mac, this
should shave off another second or so from the perceived startup time... the UI
will be displayed, and THEN we'll load in the bookmark file.
(Reporter)

Updated

19 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: M11
(Reporter)

Comment 1

19 years ago
Note: to get this to work, nsBookmarkService::Init() is changed to NOT call
ReadBookmarks(). Also, in navigator.js, at the end of the Startup() function, add
a SetTimeout() function for a half-second or so later (the timer will fire after
the UI has finished loading and laying out) which, when it fires, calls
ReadBookmarks(). Also had to make a small change to the bookmark's parsing code
to recurve on a node's children BEFORE adding the node itself into the graph so
that all "containment" issues are resolved properly.
(Reporter)

Updated

19 years ago
Blocks: 11417

Updated

19 years ago
Blocks: 7251

Comment 2

19 years ago
That would be good. Will this ensure that the personal toolbar will just fillin
as we readin the bookmarks...
(Reporter)

Comment 3

19 years ago
Yes.
(Reporter)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 4

19 years ago
Marking this as WONTFIX.  A poll of various Mac-heads concluded that they prefer
having 100% of the chrome and its contents loaded in before the browser window
appears.

Updated

18 years ago
Status: RESOLVED → VERIFIED

Comment 5

18 years ago
marking verified per engineer's last comments

Updated

17 years ago
No longer blocks: 7251
(Assignee)

Comment 6

17 years ago
reopening and taking this bug
Status: VERIFIED → REOPENED
Priority: P2 → P1
Resolution: WONTFIX → ---
Target Milestone: M11 → mozilla0.9.7
(Assignee)

Comment 7

17 years ago
taking
Status: REOPENED → ASSIGNED
(Assignee)

Comment 8

17 years ago
Argh, no, really, taking it this time, sorry for the spam.
Assignee: rjc → pchen
Status: ASSIGNED → NEW
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Component: RDF → Bookmarks
(Assignee)

Comment 9

17 years ago
Created attachment 56310 [details] [diff] [review]
call ReadBookmarks() after timeout in Startup()
(Assignee)

Comment 10

17 years ago
Created attachment 56312 [details] [diff] [review]
add protected initDatasource() method
(Assignee)

Comment 11

17 years ago
Created attachment 56314 [details] [diff] [review]
nsBookmarksService::Init() now just calls initDatasource, protect against loading bookmarks file every call to ReadBookmarks, and reset loaded flag when profile has changed
(Assignee)

Updated

17 years ago
Attachment #56310 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Attachment #56312 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Attachment #56314 - Attachment is obsolete: true
(Assignee)

Comment 12

17 years ago
Created attachment 56355 [details] [diff] [review]
load bookmarks from a timer setup in Startup()
(Assignee)

Comment 13

17 years ago
Created attachment 56356 [details] [diff] [review]
add new protected members LoadBookmarks and initDatasource
(Assignee)

Comment 14

17 years ago
Created attachment 56357 [details] [diff] [review]
don't load bookmarks in nsBookmarkService::Init but do initialize datasource, in ReadBookmarks check to see if bookmarks are already loaded
(Assignee)

Comment 15

17 years ago
showed diffs to rjc earlier, verbal r=rjc
(Assignee)

Updated

17 years ago
Attachment #56355 - Flags: review+
(Assignee)

Updated

17 years ago
Attachment #56356 - Flags: review+
(Assignee)

Updated

17 years ago
Attachment #56357 - Flags: review+

Comment 16

17 years ago
cool. sr=blake on those.
(Assignee)

Comment 17

17 years ago
checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago17 years ago
Resolution: --- → FIXED

Comment 18

17 years ago
question, wouldn't a timeout of 0 still have the same effect (forcing the 
window to show before loading the bookmarks)?
(Reporter)

Comment 19

17 years ago
Paul, just thought of something.  This change may have exposed a race condition
between when ReadBookmarks() is called via the timer firing from Startup(), and
the Bookmarks sidebar panel.  You might want to test... and potentially add a
ReadBookmarks()s call and ___.rebuild() into the bookmark sidebar panel's
onload() function.

I'll reopen this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 20

17 years ago
Please change the 100ms timeout to a 0ms timeout.  That is sufficient to delay 
until after the window shows, and prevents us from basically sitting around for 
100ms doing nothing.

Comment 21

17 years ago
Leaks are up following this checkin.

Comment 22

17 years ago
yes... 

it appears that nsBookmarksService::initDataSource(...) is called multiple
times...  however, mInner is not released (if non-null) so previous instances
get leaked :-(

-- rick

Comment 24

17 years ago
We should change that to NS_IF_RELEASE(mInner).
(Assignee)

Updated

17 years ago
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

17 years ago
leak was fixed in bugzilla 108539, marking fixed

Comment 26

17 years ago
>Please change the 100ms timeout to a 0ms timeout.  That is sufficient to delay 
>until after the window shows, and prevents us from basically sitting around 
>for 100ms doing nothing.

New bug?
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.