Closed Bug 385208 Opened 13 years ago Closed 13 years ago

slow to open the bookmark organize dialog (again)

Categories

(Firefox :: Bookmarks & History, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3 alpha6

People

(Reporter: moco, Assigned: moco)

References

Details

(Keywords: perf, regression)

slow to open the bookmark organize dialog (again?)

with jay patel's bookmarks, opening the bookmark organizer dialog is fast with

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070617 Minefield/3.0a6pre 

but noticably slower with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070620 Minefield/3.0a6pre

additionally, the history sidebar has gotten slower to open to "by last visited" and other views.

Anyone else seeing this recent performance regression?
Flags: blocking-firefox3?
my change for bug #373944 appears to have caused the perf regresssion for the "bookmark organizer dialog".

Not sure yet about the history sidebar.
Assignee: nobody → sspitzer
Tried a 16 June build and a 20 June build and history By Last Visited takes in both cases 35 seconds. 
By Last Visited was kind of broken (shows only the history of the last day) before 18 June so you need to toggle between the views before it finally works, but it was repaired by something in here: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1182209400&maxdate=1182212039  
Ria, thanks for the info.

I'm going to keep the bookmark organizer issue in this bug, and start a new one on the history regression (and confirm we have regressed.)

> By Last Visited was kind of broken (shows only the history of the last day)
> before 18 June

Was there a bug on that?  It would be worth figuring out which change fixed it (possibly #384677, looking at your bonsai query.)

> history By Last Visited takes in both cases 35 seconds. 

yikes.  That alone deserves a new bug.  I'll spin one off about that.
Status: NEW → ASSIGNED
updating summary.

>> By Last Visited was kind of broken (shows only the history of the last day)
>> before 18 June
>
>Was there a bug on that?  It would be worth figuring out which change fixed it
>(possibly #384677, looking at your bonsai query.)

It was fixed bug #384677.  

The previous query was only showing history of the last day!  And, I think this is why I thought the history sidebar had regressed.  With Jay Patel's large history.dat, the sidebar was "fast" because our query was bad.  Now that we are querying everything properly, it's slow.

[Schrep also has a large history.dat, and I bet he will see the slow down, too]

Going forward, I'll log the bug about how we only showed hits from the last day (but that is now fixed), and see why our previous query didn't work (a bug in our query parsing code?), and finally a new bug about "history by last visited" being slow.

From profiling, we seem to be sending a lot of time attempting to collapse duplicates in treeView.js, but I think there is a way to avoid doing that all all in treeView.js, and collapse the duplicates before returning the results to the view.
Summary: slow to open the bookmark organize dialog / history sidebar (again?) → slow to open the bookmark organize dialog (again)
> and see why our previous query didn't work (a bug in our query parsing code?)

It did work as expected, it was just wrong, see bug #384677 comment #3 for details.

> The previous query was only showing history of the last day!

correction, the last 30 days.  That issue is fixed and is covered by bug #384677

> a new bug about "history by last visited" being slow.

tracked by bug #385245

the performance issue with the history sidebar is covered by bug #
fixed by backing the fix for bug #373944 out.

Checking in browser/components/places/content/moveBookmarks.js;
/cvsroot/mozilla/browser/components/places/content/moveBookmarks.js,v  <--  move
Bookmarks.js
new revision: 1.8; previous revision: 1.7
done
Checking in browser/components/places/content/places.js;
/cvsroot/mozilla/browser/components/places/content/places.js,v  <--  places.js
new revision: 1.91; previous revision: 1.90
done
Checking in browser/components/places/content/tree.xml;
/cvsroot/mozilla/browser/components/places/content/tree.xml,v  <--  tree.xml
new revision: 1.75; previous revision: 1.74
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Verified fixed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200707050404 Minefield/3.0a7pre.
Status: RESOLVED → VERIFIED
Flags: blocking-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.