Closed Bug 397117 Opened 17 years ago Closed 17 years ago

perf win: avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately

Categories

(Firefox :: Bookmarks & History, defect, P3)

Product:
Firefox
Component:
Bookmarks & History
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3 beta1

People

(Reporter: moco, Assigned: asaf)

References

Details

(Keywords: perf)

perf win: use _ignoreContainerStateChange to avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately I ran into this when working on bug #387996 and bug #385831 for the most use tags, I need the child count. to get the child count for a folder, you need to have it expanded. so I added a count column to the tree in the organizer, but to get the count, I need to open the container, get the child count, and close it. but that invalidates the container, so as you can imagine that slows things down. but, since I know I'm going to open then close the container, I can set a bool to bail out in containerOpened() and containerClosed() without invalidating the container, and performance is not impacted. we do this sort of thing a lot, look for "wasOpen". I'll attach a wip patch to bug #387996, but if dietrich and mano agree this makes sense, this bug will be used for fixing the other callers. I predict this will lead to some nice perf wins.
Flags: blocking-firefox3?
>I predict this will lead to some nice perf wins. for example, copying a lot of bookmarks in the organizer. to build of the json representation, we open / close containers.
Status: NEW → ASSIGNED
Definitely wanted for the perf win ..
Flags: blocking-firefox3? → blocking-firefox3-
Whiteboard: [wanted-firefox3]
clearning tm, as this no longer blocks bug #387996 see that bug for a patch (https://bugzilla.mozilla.org/attachment.cgi?id=283676) that implements this. we will want this (for perf reasons) if we add back a "count" column to the places organizer (for the "tag" tree.)
Assignee: sspitzer → nobody
Status: ASSIGNED → NEW
No longer depends on: 387996
Target Milestone: Firefox 3 M9 → ---
ended up needing this for bug #387996 after all.
Assignee: nobody → sspitzer
Blocks: 387996
Summary: perf win: use _ignoreContainerStateChange to avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately → perf win: use ignoreInvalidateContainer to avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately
Target Milestone: --- → Firefox 3 M9
Depends on: 399729
Patch on bug 399729 with a-little-bit different solution.
Assignee: sspitzer → mano
Summary: perf win: use ignoreInvalidateContainer to avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately → perf win: avoid invalidating the container in the tree view, if all we are going to do is open and close it immediately
Fixed under bug 399729.
Status: NEW → RESOLVED
Closed: 17 years ago
Priority: -- → P3
Resolution: --- → FIXED
Flags: wanted-firefox3+
Whiteboard: [wanted-firefox3]
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.