Closed Bug 431548 Opened 14 years ago Closed 13 years ago

Tagging history item takes a long time without progress indication (slight hang / freeze of UI)

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: zurtex, Assigned: mak)

References

Details

(Keywords: perf, verified1.9.1)

Attachments

(1 file)

On:

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9pre) Gecko/2008042906 Minefield/3.0pre ID:2008042906

STR:

1. Go to the Places Library
2. Go to the History section
3. Click on an unbookmarked history item
4. In the bottom pane, type some tag in to the tag section
5. Click on the unbookmarked history item
6. Firefox goes weird! Including temporarily freezing up.
Flags: blocking-firefox3?
wierd?  are you seeing a flickering bug?  if so this is likely a dupe.
What's happening is that it's creating a bookmark and adding tags to it for that history item, and I bet that operation is taking time.

This chalks up to "we have no good progress indication" as well as a bit of "why should that operation take a lot of time"?

Doubt that this will be a common operation, and it's just sucky not crashy, so don't think it blocks.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Summary: Tagging history item causes Firefox to freeze up → Tagging history item takes a long time without progress indication (slight hang / freeze of UI)
(In reply to comment #1)
> wierd?  are you seeing a flickering bug?  if so this is likely a dupe.
> 

Well, for example, lots of elements in my Bugzilla report page got highlighted in a blue rectangle. 
I can't reproduce that anymore :/

Guess it's just what Beltner says :-). Still, I'm on a pretty fast system and it comes off to me as Firefox totally freezing up (although yeah, I get control back 10 - 15 seconds later). Can't imagine what it might be like on slow computers.
OS: Windows Server 2003 → All
yes the problem is probably not having a progress indicator but not taking a century for doing simple things like adding a tag... last days patch (flickering related) could have mitigated, but leave this open for further investigation on tag adding perf
Given a focus on places performance and tagging features in 3.1, I'm nominating it for blocking, though I'd understand if it's only wanted.
Flags: blocking-firefox3.1?
Could be mitigated or even resolved by bug 432706, and helped even more by the fsync work in 3.1.

Marking blocking, will re-evaluate after more perf work lands.
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Keywords: perf
Priority: -- → P2
Target Milestone: --- → Firefox 3.1
see also bug 460830, Tags has a wrong history observer.
Depends on: 432706, 460830
Whiteboard: [patch in dependent bugs]
Whiteboard: [patch in dependent bugs] → [patch in dependent bugs][re-triage after dependent bugs get fixed]
Assignee: nobody → dietrich
ok, i can definately reproduce this, notice that history is a query node, so when you change something the full query is refreshed (and the full tree rebuilt), so for example, how much time does take the library to open history tree? at least that time will be needed to do the update.
Hwv another strange thing is that after i've added the tag, if i move to a bookmarks folder and i delete a bookmark, or tag a bookmark it takes a long time, like if history tree would be rebuilding even if it's not shown.
and ::Refresh() for "Tags" is called a bunch of times when adding a single tag
what i think is that when adding/removing a tag we generate a bunch of observers calls (onItemAdded/Removed/Changed) that are observed by open queries with a bookmarks observer, so for example the left pane and Tags. Being those queries, every call causes a Refresh() that requery history, being history really big we slowdown rebuilding the tree. The callstack at every call to nsNavHistoryQueryFolder::Refresh() seems to confirm that.
Attached patch patchSplinter Review
unless we kill a bunch of notifications or revise completely the observers system i think we could batch tagging and untagging so we will rebuild the History tree only onEndUpdateBatch. The main problem (as always) is that tags are bookmarks, every tag change fires up item notifications.
Attachment #352739 - Flags: review?(dietrich)
maybe i'm missing something, but notifications are still sent inside batches. so, while we should take this patch since it puts all the db activity in a transaction, it doesn't solve the problem you discuss above.

in order to suspend refreshing and live-updating inside a batch, we need the patch on bug 432706.

another problem with your previous comment is that the History tree shouldn't be paying attention to bookmark notifications, unless it's QUERYUPDATE_COMPLEX_WITH_BOOKMARKS (which it seems like the History tree wouldn't be, right?).
Hardware: PC → All
(In reply to comment #13)
> in order to suspend refreshing and live-updating inside a batch, we need the
> patch on bug 432706.

never mind this, i see you set it as blocking this bug.
(In reply to comment #13)
> maybe i'm missing something, but notifications are still sent inside batches.

the strange thing is that doing this i don't see all of them while i only see endupdatebatch calls... i'll go back to the debugger to see exactly what happens.
(In reply to comment #13)
> another problem with your previous comment is that the History tree shouldn't
> be paying attention to bookmark notifications, unless it's
> QUERYUPDATE_COMPLEX_WITH_BOOKMARKS (which it seems like the History tree
> wouldn't be, right?).

per irc, it's the Tags query that is causing this, not History.
do you want me to strip patch to a new bug?
Comment on attachment 352739 [details] [diff] [review]
patch

r=me
Attachment #352739 - Flags: review?(dietrich) → review+
http://hg.mozilla.org/mozilla-central/rev/c368f6264e50
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [patch in dependent bugs][re-triage after dependent bugs get fixed] → [re-triage after dependent bugs get fixed]
Target Milestone: Firefox 3.1 → Firefox 3.2a1
Whiteboard: [re-triage after dependent bugs get fixed]
Assignee: dietrich → mak77
verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090127 Shiretoko/3.1b3pre
Status: RESOLVED → VERIFIED
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.