Closed
Bug 405765
Opened 17 years ago
Closed 14 years ago
Opening folder in the right pane of the Places Organizer is slow
Categories
(Firefox :: Bookmarks & History, defect, P2)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: klassphere, Unassigned)
References
Details
(Keywords: perf, Whiteboard: [tsnap])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112705 Minefield/3.0b2pre ID:2007112705
Opening folder in the right pane of the Places Organizer is slow (around 5 seconds for a relatively large folder).
It's OK in the left pane, you can open it immediately as it was in the old Places.
Comment 1•17 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112810 Minefield/3.0b2pre
Slower than the Bookmarks-menu but less than 1 second here.
Reporter | ||
Comment 2•17 years ago
|
||
Forgot to add the size of places.sqlite, it's 23MB. While opening a folder in the right pane all windows freeze.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007122308 Minefield/3.0b3pre ID:2007122308
Flags: blocking-firefox3?
Comment 3•17 years ago
|
||
what folder? a self created one or one of the default folders?
how many items are in that folder?
what are your system spec? is still 5 seconds in the latest nightly?
Reporter | ||
Comment 4•17 years ago
|
||
(In reply to comment #3)
> what folder? a self created one or one of the default folders?
> how many items are in that folder?
>
> what are your system spec? is still 5 seconds in the latest nightly?
>
Basically every folder.
It takes time regardless of the number of items in it, right now it takes 5 sec for a folder with 5 items on
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007122715 Minefield/3.0b3pre ID:2007122715
The CPU is Athlon 64 X2 4800+.
Reporter | ||
Comment 5•17 years ago
|
||
Sorry, it seems the default folders such as Bookmarks Toolbar and the top level of Bookmarks Menu seem responsive enough. However, self-created ones in Bookmarks Menu and the top level of Unified Bookmarks are slow.
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Comment 6•17 years ago
|
||
Dietrich can you find the right owner? Or maybe Ondrej wants to take this?
Assignee: nobody → dietrich
Comment 7•17 years ago
|
||
I'm waiting for review of 385245 and have time for this.
Assignee: dietrich → ondrej
Updated•17 years ago
|
Priority: -- → P2
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 beta4
Comment 8•17 years ago
|
||
I can see again the very same problems with annotations which are repeatedly read for all items and even for their parents.
For instance:
nodeIsReadOnly: function PU_nodeIsReadOnly(aNode) {
NS_ASSERT(aNode, "null node");
if (this.nodeIsFolder(aNode))
return this.bookmarks.getFolderReadonly(asQuery(aNode).folderItemId);
if (this.nodeIsQuery(aNode))
return asQuery(aNode).childrenReadOnly;
return false;
},
This always does lookup in the database - result is huge number of database queries. Similar to 399566, but now the annotation is READ_ONLY_ANNO and it is requested from the parent (not the child).
Updated•17 years ago
|
Target Milestone: Firefox 3 beta4 → Firefox 3
Updated•17 years ago
|
Whiteboard: [needs status update]
Updated•17 years ago
|
Whiteboard: [needs status update] → [has patch in bug 420078]
Reporter | ||
Comment 9•17 years ago
|
||
A bit quicker now on this build but still have to wait for 2-3 seconds.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041017 Minefield/3.0pre ID:2008041017
Comment 10•17 years ago
|
||
This patch solves part of the problem. The remaining part seems to be checking whether a folder is not read only and may be something more. I will check whether this could be optimized too. With my current configuration I can see a 1 second delay when clicking on the item on the right side.
Attachment #315858 -
Flags: review?(dietrich)
Comment 11•17 years ago
|
||
When I click on a folder in the left pane it does following number of annotation queries:
00002=>3
00091=>9
00109=>7
PlacesOrganizer/OrganizerQuery=>7
placesInternal/READ_ONLY=>12
That is already a lot, you can see, that we ask for two annotation on three nodes. And we do 19! queries instead of 6.
And now when we click on it in the right pane:
00002=>6
00091=>121
00109=>150
PlacesOrganizer/OrganizerQuery=>114
bookmarkProperties/description=>1
bookmarks/staticTitle=>1
placesInternal/READ_ONLY=>161
We should need up to 3x4 = 12 queries and we perform 277 queries. That is 14 times as much as if we clicked on the item on the left side.
I will analyze this deeper, but it looks like we should be caching the result of annotation queries. However, it should be investigated why we need so many queries when clicking on the item on the right side.
Status: NEW → ASSIGNED
Comment 12•17 years ago
|
||
(In reply to comment #11)
> I will analyze this deeper, but it looks like we should be caching the result
> of annotation queries. However, it should be investigated why we need so many
> queries when clicking on the item on the right side.
how many bookmarks/folders are there in the right pane root (not only visible, full tree hierarchy)? left side is an excludeItems list, items are excluded in the backend... right side is a flatList, IIRC items are excluded in the frontend, so we could still ask annotation also for hidden items?
Comment 13•17 years ago
|
||
(In reply to comment #12)
> (In reply to comment #11)
> > I will analyze this deeper, but it looks like we should be caching the result
> > of annotation queries. However, it should be investigated why we need so many
> > queries when clicking on the item on the right side.
>
> how many bookmarks/folders are there in the right pane root (not only visible,
> full tree hierarchy)? left side is an excludeItems list, items are excluded in
> the backend... right side is a flatList, IIRC items are excluded in the
> frontend, so we could still ask annotation also for hidden items?
I do not think this is related to the problem. The list of nodes that annotations are read for is the same if I click on the folder in the left pane or in the right pane.
Comment 14•17 years ago
|
||
Comment on attachment 315858 [details] [diff] [review]
Optimize livemark checking
>
> isLivemark: function LS_isLivemark(aFolderId) {
>- return this._ans.itemHasAnnotation(aFolderId, LMANNO_FEEDURI);
>+ if (aFolderId == -1)
>+ return false;
>+ for (var i=0; i < this._livemarks.length; ++i) {
>+ if (this._livemarks[i].folderId == aFolderId)
>+ return true;
>+ }
>+ return false;
> },
you could use _getLivemarkIndex() here, instead of duplicating that loop in this method.
>Index: toolkit/components/places/src/utils.js
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/places/src/utils.js,v
can you confirm that these changed methods are not called during startup? if they are, maybe you should add back in the _livemarksInitialized approach in your first patch to bug 420078, in order to not regress bug 398300.
Comment 15•17 years ago
|
||
This patch reduced number of requests for read only and organizer queries. So maybe about 200 queries less, but it still does 32 queries which seem to be slow. So further optimization is needed.
(In reply to comment #14)
> (From update of attachment 315858 [details] [diff] [review])
> ..
> you could use _getLivemarkIndex() here, instead of duplicating that loop in
> this method.
This was an optimization on my side, I wanted to avoid exception throwing and catching, what would happen in 99% of cases.
> ..
> can you confirm that these changed methods are not called during startup? if
> they are, maybe you should add back in the _livemarksInitialized approach in
> your first patch to bug 420078, in order to not regress bug 398300.
I think the second patch in bug 420078 solved this on the necessary place and I have now fixed it in this patch and hopefully a little more elegantly.
Attachment #315858 -
Attachment is obsolete: true
Attachment #315858 -
Flags: review?(dietrich)
Comment 16•17 years ago
|
||
The reason for this slowness is actually the endless rebuilds caused by leftPane.selectPlaceURI, a rebuilt is caused by each container-opening.
Assignee: ondrej → mano
Status: ASSIGNED → NEW
Whiteboard: [has patch in bug 420078]
Comment 17•17 years ago
|
||
Could we employ a chunking strategy like what is used in the Download Manager to get instant response here, even if opening the full list takes a long time?
Or an expedient fix that shows some form of progress indication so that the user knows that something's happening.
Comment 18•17 years ago
|
||
(In reply to comment #17)
> Could we employ a chunking strategy like what is used in the Download Manager
> to get instant response here, even if opening the full list takes a long time?
that's high on the list for 3.next
>
> Or an expedient fix that shows some form of progress indication so that the
> user knows that something's happening.
>
this would likely not be expedient, and would require UI, string changes, etc.
mano: do you have a strategy for resolving comment #16? also, is there a reason why we don't want to take at least some of the changes from Ondrej's patch? i haven't read through the read-only caching bit, but the livemark checking optimizations look good.
Updated•17 years ago
|
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Whiteboard: [3.0.1]
Comment 19•17 years ago
|
||
We'll take a patch here, but we're not hearing enough complaints for this to block, which makes it a good candidate for stability and optimization in a branch release.
Marking wanted, not-blocking, and adding [3.0.1] to the whiteboard.
Comment 20•16 years ago
|
||
Don't know if the same issue. but even clicking folder inside Places Organizer is also very slow. And i dont have much data there still (~11MB).
Updated•16 years ago
|
Target Milestone: Firefox 3 → ---
Updated•15 years ago
|
Comment 21•15 years ago
|
||
not duping this against bug 490664 because we should check the log attached to this bug to look for additional optimization opportunities.
Comment 22•15 years ago
|
||
How bad is this on trunk?
Comment 23•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
Comment 24•14 years ago
|
||
I don't think this is still slow, mostly thanks to bug 472343, but also other improvements. The data here is outdated so that a new bug would be needed even we'd still see some slowdown (reasons could be completely different).
Assignee: mano → nobody
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•