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)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: klassphere, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [tsnap])

Attachments

(1 file, 1 obsolete file)

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.
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.
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?
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?
(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+.
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.
Flags: blocking-firefox3? → blocking-firefox3+
Dietrich can you find the right owner? Or maybe Ondrej wants to take this?
Assignee: nobody → dietrich
I'm waiting for review of 385245 and have time for this.
Assignee: dietrich → ondrej
Priority: -- → P2
Target Milestone: --- → Firefox 3 beta4
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).
Target Milestone: Firefox 3 beta4 → Firefox 3
Depends on: 420078
Whiteboard: [needs status update]
Whiteboard: [needs status update] → [has patch in bug 420078]
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
Attached patch Optimize livemark checking (obsolete) (deleted) — Splinter Review
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)
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
(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?
(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 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.
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)
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]
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.
(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.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Whiteboard: [3.0.1]
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.
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).
Target Milestone: Firefox 3 → ---
Depends on: 490664
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [3.0.1] → [tsnap]
not duping this against bug 490664 because we should check the log attached to this bug to look for additional optimization opportunities.
How bad is this on trunk?
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
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.

Attachment

General

Creator:
Created:
Updated:
Size: