Closed
Bug 401753
Opened 17 years ago
Closed 17 years ago
creating the "Places" folder (see bug #387996) for an existing profile with lots of bookmarks takes too long, the personal toolbar is blank for several seconds.
Categories
(Firefox :: Bookmarks & History, defect, P3)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3 beta2
People
(Reporter: moco, Assigned: asaf)
References
Details
(Keywords: perf)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
moco
:
review+
|
Details | Diff | Splinter Review |
creating the "Places" folder (see bug #387996) for an existing profile with lots of bookmarks takes too long, the personal toolbar is blank for several seconds.
this was with "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102923 Minefield/3.0a9pre"
Assignee | ||
Comment 1•17 years ago
|
||
We should fix this by creating the places folder _before_ importing bookmarks.html, otherwise AdjustIndicies executes a pretty heavy UPDATE statement.
Assignee: nobody → mano
Flags: blocking-firefox3?
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 M11
Assignee | ||
Comment 2•17 years ago
|
||
Attachment #290280 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #290280 -
Flags: review? → review?(sspitzer)
Reporter | ||
Comment 3•17 years ago
|
||
1)
+ // Call it here for Fx3 profiles created brefore the Places folder
+ // has been added, otherwise it's called during import.
+// this.ensurePlacesDefaultQueriesInitialized();
+
shouldn't the last line be uncommented?
nit: s/brefore/before
2)
+ // XXXmano: this should be batched even if we're not called from the
import
+ // service. However, calling runInBatchedMode from whithin a RunBatched
+ // implementation hangs the browser.
can you add a reference back to bug #405497?
[Bug 405497] New: Calling RunInBatchedMode within a RunBatched implementation
hangs"
nit: s/whithin/within
3)
What about making ensurePlacesDefaultQueriesInitialized take a boolean for
whether or not to batch?
Assignee | ||
Comment 4•17 years ago
|
||
(In reply to comment #3)
> 1)
>
> + // Call it here for Fx3 profiles created brefore the Places folder
> + // has been added, otherwise it's called during import.
> +// this.ensurePlacesDefaultQueriesInitialized();
> +
> shouldn't the last line be uncommented?
>
yeah, oops
> + // XXXmano: this should be batched even if we're not called from the
> import
> + // service. However, calling runInBatchedMode from whithin a RunBatched
> + // implementation hangs the browser.
>
> can you add a reference back to bug #405497?
>
done
>
> What about making ensurePlacesDefaultQueriesInitialized take a boolean for
> whether or not to batch?
I don't it's worth bloating the api for this kinda edge case (which I'm hoping to fix right before final).
Reporter | ||
Comment 5•17 years ago
|
||
Comment on attachment 290280 [details] [diff] [review]
patch
clearing review request.
mano, can you attach an updated version?
Attachment #290280 -
Flags: review?(sspitzer)
Assignee | ||
Comment 6•17 years ago
|
||
Attachment #290280 -
Attachment is obsolete: true
Attachment #290288 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #290288 -
Flags: review? → review?(sspitzer)
Reporter | ||
Comment 7•17 years ago
|
||
Comment on attachment 290288 [details] [diff] [review]
patch
r=sspitzer, with one concern.
over irc you wrote:
<Mano> fyi, my testing shows that we don't need to call savePrefFile once i moved the call to setBoolPref to browserglue
<sspitzerMsgMe> is that because something else is calling savePrefFile()?
<Mano> very likely
can you set a breakpoint and figure out who is calling savePrefFile() in both the existing (but no "places" folder) and the new profile scenario?
I just want to confirm that removing the call savePrefFile() is correct.
Attachment #290288 -
Flags: review?(sspitzer) → review+
Assignee | ||
Comment 8•17 years ago
|
||
Seth, thanks for asking me to double check this, savePrefFile isn't called apparently, so I've added it back.
Assignee | ||
Comment 9•17 years ago
|
||
Attachment #290288 -
Attachment is obsolete: true
Assignee | ||
Comment 10•17 years ago
|
||
mozilla/browser/base/content/browser.js 1.903
mozilla/browser/components/nsBrowserGlue.js 1.38
mozilla/browser/components/nsIBrowserGlue.idl 1.4
mozilla/browser/components/places/src/Makefile.in 1.25
mozilla/browser/components/places/src/nsPlacesImportExportService.cpp 1.36
mozilla/browser/components/places/tests/unit/test_bookmarks_html.js 1.14
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 M11 → Firefox 3 M10
Reporter | ||
Comment 11•17 years ago
|
||
Comment on attachment 290300 [details] [diff] [review]
for checkin
r=sspitzer, thanks for double checking savePrefFile()
Attachment #290300 -
Flags: review+
Assignee | ||
Comment 12•17 years ago
|
||
12ms Ts drop on bl-bldlnx03
9ms Ts drop on bm-xserve08
Comment 13•17 years ago
|
||
verified with monster bookmark profile on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007112804 Minefield/3.0b2pre
Status: RESOLVED → VERIFIED
Comment 14•17 years ago
|
||
this regressed the case when i clear my profile dir content and start minefiled, the places folder is not created.
works in 26/11, don't work in 27/11 (win) probably due to this checkin
1. delete the profile dir contents (not the dir)
2. launch minefield
3. places folder not created
i had not filled a new report since i don't know if it's expected behaviour, i often clear profile contents to test builds with a new clean profile. Should i fill a report on this?
Comment 15•15 years ago
|
||
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".
In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body contains places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.
Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.
Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•