If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in Firefox 3.6a1

Status

()

Firefox
Bookmarks & History
P2
normal
VERIFIED FIXED
10 years ago
a year ago

People

(Reporter: Damian Shaw [Quan], Assigned: mak)

Tracking

({perf, verified1.9.1})

Trunk
Firefox 3.6a1
perf, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 -
blocking-firefox3.5 +
wanted-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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?

Comment 1

10 years ago
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)
(Reporter)

Comment 3

10 years ago
(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. 
(Reporter)

Comment 4

10 years ago
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
(Assignee)

Comment 5

10 years ago
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
(Reporter)

Comment 6

9 years ago
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
(Assignee)

Comment 8

9 years ago
see also bug 460830, Tags has a wrong history observer.
Depends on: 432706, 460830
Whiteboard: [patch in dependent bugs]

Updated

9 years ago
Whiteboard: [patch in dependent bugs] → [patch in dependent bugs][re-triage after dependent bugs get fixed]
Assignee: nobody → dietrich
(Assignee)

Comment 9

9 years ago
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.
(Assignee)

Comment 10

9 years ago
and ::Refresh() for "Tags" is called a bunch of times when adding a single tag
(Assignee)

Comment 11

9 years ago
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.
(Assignee)

Comment 12

9 years ago
Created attachment 352739 [details] [diff] [review]
patch

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.
(Assignee)

Comment 15

9 years ago
(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.
(Assignee)

Comment 17

9 years ago
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+
(Assignee)

Comment 19

9 years ago
http://hg.mozilla.org/mozilla-central/rev/c368f6264e50
Status: NEW → RESOLVED
Last Resolved: 9 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
(Assignee)

Updated

9 years ago
Whiteboard: [re-triage after dependent bugs get fixed]
(Assignee)

Updated

9 years ago
Assignee: dietrich → mak77
(Assignee)

Comment 20

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/833dc6994cb0
Keywords: fixed1.9.1
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
Keywords: fixed1.9.1 → verified1.9.1
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.