Live bookmarks load way too aggressively (lock up/hang/freeze browser)

RESOLVED FIXED in Firefox 3.1b1

Status

()

Firefox
Bookmarks & History
P2
major
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: Peter6, Assigned: Takeshi Ichimaru(a.k.a. Ayakawa))

Tracking

({hang, perf, regression})

Trunk
Firefox 3.1b1
hang, perf, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3.5 -
wanted-firefox3.5 +
blocking1.9.0.4 -
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: intermittent test failure, workaround pushed)

Attachments

(5 attachments, 10 obsolete attachments)

1.99 KB, patch
Details | Diff | Splinter Review
183.03 KB, image/png
Details
5.89 KB, application/octet-stream
Details
12.42 KB, patch
mak
: review+
Details | Diff | Splinter Review
2.31 KB, patch
dietrich
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060304 Firefox/1.6a1 ID:2006030418

I have a set of 20-ish live bookmarks and they take long to load and lock up the browser.

repro:
1.download bookmarks_history.sqlite.zip
2.rename your current bookmarks_history.sqlite (profile IMPORTANT)
3.copy bookmarks_history.sqlite from the zip into your profile
4.open taskmanager
5.start Firefox
6.wait till the processorload hits 100% , this will start after about 10 seconds after the Firefox window has appeared on screen
7.try to do anything in Firefox

result:
- processorload 100% for 15-20seconds
- firefox is dead and responds to nothing untill the loading has finished

we had something similar during the run up to FF1.0 , but Vlad somehow solved that.

att. coming
(Reporter)

Comment 1

11 years ago
Created attachment 214211 [details]
bookmarks_history.sqlite zipped
Assignee: nobody → annie.sullivan
Priority: -- → P2
Target Milestone: --- → Firefox 2 alpha2
*** Bug 330916 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Whiteboard: swag:2 days
I think the following is related but I don't know if I shall file a new bug for it. It only happens in a places enabled browser.
Problem: folder with loading feed closes automatically.

Steps to reproduce:
- Start Firefox with the default set of bookmarks
- On the toolbar, make a new folder and drag the "Latest Headlines" feed into it
- Restart the browser and click on the new folder. Keep it open and wait.
- Within 10 seconds the folder closes again. 
I assume that's because the feed loads right after a restart. I can't reproduce this when I wait a minute before clicking on the new folder.

When I do the same in a non-places trunk browser, I see not much difference in processor behaviour. There are two or three peaks of about 30% processor usage. 
*** Bug 340062 has been marked as a duplicate of this bug. ***

Comment 5

11 years ago
Perhaps we should take this in a different direction.  How about putting something in the Options menu that looks something like this:

Automatically reload Live Bookmarks:

[X] On Firefox startup

then every [x] minutes

"Let's solve our bugs by having preferences that users have no idea how to set" is pretty much the exact opposite of Firefox. About the only way you could get farther away from Firefox's style would be to suggest a modal dialog explaining it the first time a live bookmark loads.

Comment 7

11 years ago
OK, that *might* not be the best way to solve this bug, but IMHO it's a good enhancement anyway.
This P2 has a target of Firefox 2a2. We are now up to Firefox 2rc1. Please update the target milestone or nominate this bug for blocking-firefox2.

Comment 9

11 years ago
William, this bug only existed in Places, which got pulled from Firefox 2 sometime in May. So the bug you're seeing is most certainly a different bug.
(In reply to comment #9)
> William, this bug only existed in Places, which got pulled from Firefox 2
> sometime in May. So the bug you're seeing is most certainly a different bug.
> 

Then what, may I ask, is this bug report concerning? If Places got pulled months ago, will it be reinstated in the trunk builds after Firefox 2?

I actually got to this bug from bug 330916 as I was going through verifying blockers resolved as duplicates, not by experiencing this problem personally.

Comment 11

10 years ago
It's been a year since I've submitted the same bug (330672), but nothing changed yet. We live in the RSS era and many users subsribe to numerous feeds. The majority of the users, don't have access to broadband yet; not to mention that it is a waste of bandwidth (some users have bandwidth limits). Travelers and others using mobile connections, are also another group affected.

There should be an option to change this. I like Jackie's solution. For a visual helping hand, the rss icon could be greyed out when the option to autoload feeds are selected. This is not confusing at all. Sometimes developers think that they help the majority with their solutions ("keep it simple"), but this is not the case.
Assignee: annie.sullivan → nobody
Whiteboard: swag:2 days
Target Milestone: Firefox 2 alpha2 → ---

Comment 12

10 years ago
Is the Firefox polling strategy described somewhere on the wiki? I believe this is possible to poll in a reasonable way without having to ask the user. Just by yielding to the UI after refreshing each feed.
Blocks: 380307
Target Milestone: --- → Firefox 3 alpha5
Assignee: nobody → sspitzer
Depends on: 379591

Updated

10 years ago
Target Milestone: Firefox 3 alpha5 → Firefox 3 alpha6
for B1, let's confirm that this is not the same as bug 379729, and that it still occurs in contemporary builds.
Keywords: qawanted
Target Milestone: Firefox 3 alpha6 → Firefox 3 beta1
moving off to m8
Keywords: perf
Target Milestone: Firefox 3 M7 → Firefox 3 M8
-> m9
Target Milestone: Firefox 3 M8 → Firefox 3 M9
dietrich wrote:

> let's confirm that this is not the same as bug 379729

I just ran into this again, with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082222 Minefield/3.0a8pre

at 8:05 pm, 5 feeds appears to get refreshed:  4 copies of planet (http://planet.mozilla.org/atom.xml) and one copy of asa's blog (http://weblogs.mozillazine.org/asa/index.rdf), which result in 325 total items (75 * 4 + 25), so I think that means I removed and re-added 325 items.

note, i got a ton of assertions on my console.

Updated

10 years ago
Duplicate of this bug: 330672
When I refresh the Live Bookmarks, which takes about a minute and completely locks up firefox, I get the following exceptions in the error console, maybe it helps to fix the bug.

Error: [Exception... "'Failure' when calling method: [nsINavHistoryBatchCallback::runBatched]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js :: LLL_handleResult :: line 876"  data: no]
Source File: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js
Line: 876
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.runInBatchMode]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js :: LLL_handleResult :: line 876"  data: no]
Source File: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js
Line: 876
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.runInBatchMode]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js :: LLL_handleResult :: line 876"  data: no]
Source File: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js
Line: 876
Error: not well-formed
Source File: http://feedhouse.mozillazine.org/rss20.xml
Line: 932, Column: 48
Source Code:
 <title>Mitchell Baker: Activities:  Mid July & Aug 2007</title>
Error: this._processor has no properties
Source File: file:///C:/Programme/Mozilla%20Firefox/components/nsLivemarkService.js
Line: 903
FWIW, the "XML" icons on http://www.syndic8.com/ seems to trigger most of the symptoms outlined in this bug quite frequently.
moving to m10
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Target Milestone: Firefox 3 beta3 → ---
(Assignee)

Comment 21

9 years ago
Created attachment 308333 [details] [diff] [review]
simple patch for distribute the load of the livemark update

This patch is simple concept.
3 Livemarks load -> wait 10sec -> next 3Livemarks load -> wait 10sec...
(Assignee)

Updated

9 years ago
Attachment #308333 - Attachment is obsolete: true
(Assignee)

Comment 22

9 years ago
Created attachment 308338 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 2

I find a little mistake, re-uploaded.
mmh is this confirmed as an issue still on current trunk? it's a quite old report

would not be easier using a setTimeout for every iteration of the loop (or every X iterations) in _checkAllLivemarks?
(Assignee)

Comment 24

9 years ago
Let's try to have 50 or more livemarks, and start fx3. This bug still alive.

I don't know JavaScript well. If somebody know smart method, please teach to me,
or write patch :)


maybe you could simply add a setTimeout (or SetInterval), separating each updateLivemarkChildren by 1s (or less)... for example (pseudocode):
for (i) {setTimeout(myUpdateFunc(i), i*1000);}
you should also increase the minimum expire time, that actually is 1m, that should be limited to a minimum of 15m, so we have a check every 225s (15m / 4 = about 3,5m), that should be enough time to update 200 livemarks (having the default time of 1m would bring to a check every 15s, with overlapping updates).
(Assignee)

Updated

9 years ago
Attachment #308338 - Flags: review?(dietrich)
(Assignee)

Comment 26

9 years ago
setTimeout and setInterval aren't used this patch. These cause error :(
only way to use timer is to use G_Alarm.

Comment 27

9 years ago
(In reply to comment #24)
> Let's try to have 50 or more livemarks, and start fx3. This bug still alive.
> 
> I don't know JavaScript well. If somebody know smart method, please teach to
> me,
> or write patch :)
> 

Wouldn't the best way to solve this bug (irrespective of number of subscribed feeds) to simply only make sure one live bookmark is processed at a given time?

In other words, I envision a worker being called every PREF milliseconds, and processing only one feed (unlike the current case of multiple.)  It would always process the least recently updated one ideally.

In other words, using let's say a smaller number like 2000 milliseconds for the cycle time on the G_Alarm (the main one already being used currently), but then add/rename "checkAllLivemarks" to "checkFirstLivemark" and make it stop as soon as it hits one.

A better solution would be to change that logic, to search the children and find the one that is most in need of an update, and update that one each time.  Most in need would just be the maximal value of |Date.now() - expireTime|.

That would elimate the need for a second G_Alarm, no?  Although you'd probably still want the locking, theoretically, so you're still only ever updating one at any given time.  And you'd probably still want a different pref (separate from gExpiration) to say how often to check each one (and this would be something lower like current default / expected high-case feeds e.g. 36000 by default or something.)

Does that make sense/seem like the right way?  I'd be happy to write it as a patch if it seems like a good idea, or I'm sure you'd have no trouble doing so.

-[Unknown]
(Assignee)

Updated

9 years ago
Attachment #308338 - Attachment is obsolete: true
Attachment #308338 - Flags: review?(dietrich)
(Assignee)

Comment 28

9 years ago
Created attachment 310366 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 3

rewrite with using 1 timer.
(Assignee)

Updated

9 years ago
Attachment #310366 - Flags: review?(dietrich)
(Assignee)

Comment 29

9 years ago
If we have many many livemarks(50, 100 or more), Fx3 lock up when it start, and it hangs up regularly when updating livemarks.
This patch reduces the reading load of a live mark to extent in which the inspection of web is not at least disturbed. 
Flags: blocking-firefox3?

Comment 30

9 years ago
Isn't it better to first figure out why a single reload takes 2-3 seconds? With plenty of CPU and disk activity?

Distributing is a good thing, but looks like a workaround.
This will not block the final release of Firefox 3. Any patch will need unit tests in order to be approved.
Flags: blocking-firefox3? → blocking-firefox3-
(Assignee)

Comment 32

9 years ago
I don't know how to make unit test.

perhaps, test story is:

1. make bookmark with 50 or more livemarks
2. check to be lock up or not when Fx started

Can a series of operations be mounted ? Please teach someone.
*ping*
OS: Windows 2000 → All

Updated

9 years ago
Assignee: moco → nobody

Comment 34

9 years ago
duplicate of bug 295758?

Comment 35

9 years ago
This bug should be added to the bugs blocking ff3 list.

I consider this very serious, especially when you have a lot of live bookmarks and you start ff, it takes a long time to start (since it checks live bookmarks at startup). This alone makes ff3 unusable.

Comment 36

9 years ago
This bug is very frustating, because i erased my biggest Livemarks, thus reduced the load, but this not eliminate the problem. If you have 6~10 medium sized livemarks (like BBC News RSS), you'll suffer from the same problem.

Ppl at mozilla development don't use RSS?

The people claims that RSS function in IE7 sucks, but in FF3 will sucks more.

This bug does NOT occur in FF2. In FF2 i had even more livemarks, like 10 big (30+ items), 15 medium and antoher 40 small, without any performance hit.
Flags: blocking-firefox3- → blocking-firefox3?
Taking the liberty of removing the blocking 3.0 request. nightly0, please read comment 31. This bug does not block Firefox 3.
Flags: blocking-firefox3?

Comment 38

9 years ago
(In reply to comment #36)
> This bug is very frustating, because i erased my biggest Livemarks, thus
> reduced the load, but this not eliminate the problem. If you have 6~10 medium
> sized livemarks (like BBC News RSS), you'll suffer from the same problem.
> 
> Ppl at mozilla development don't use RSS?
> 
> The people claims that RSS function in IE7 sucks, but in FF3 will sucks more.
> 
> This bug does NOT occur in FF2. In FF2 i had even more livemarks, like 10 big
> (30+ items), 15 medium and antoher 40 small, without any performance hit.
> 

Agreed, This bug did not happen for me either in FF2.
Duplicate of this bug: 295758

Comment 40

9 years ago
From duped bug 295758:

It's caused by the *number of total items* in feeds (e.g., news articles in a news feed), and not the *number of feeds*:

Bug occurred in FF3 RC1 with *only one* livemark, which had ~2,000 items in its feed. On my Pentium M 2GHz with 1.5GB RAM, FF consumed 90-99% CPU for approximately 40 seconds every time it refreshed the feed.

The typical end user will have a very hard time diagnosing this problem.  There is no indication of what's happening. I didn't figure it out until I logged the hangs and noted the interval. I'm an IT professional and knew enough to suspect livemarks. I'm no genius, but most users I support would have no idea.

Because of the likelihood of the problem -- it isn't astronomical but I suspect it will any user with many feed items -- and the difficulty for users to solve the problem themselves, I'm nominating it to block FF3.1.


SOLUTIONS

If there are no resources to fix the problem, then just adding feedback in the statusbar, such as 'Refreshing live bookmarks ..,.', would be a big help to end users.

At least they could identify the source of the problem and take action.  However, if they have more bookmarks then they can manually read through, then they encounter the problem that you can't search for live bookmarks (bug 437163) and you need an addon to generate a list of them.
Flags: blocking-firefox3.1?
Summary: Live bookmarks load way too agressive (lock up browser) → Live bookmarks load way too agressive (lock up/hang/freeze browser)

Comment 41

9 years ago
I should add, I did see the problem in FF2. I had hoped the FF3 upgrade and a fresh profile would fix it, but no luck ...

Comment 42

9 years ago
(In reply to comment #40)
> It's caused by the *number of total items* in feeds (e.g., news articles in a
> news feed), and not the *number of feeds*:

Given that, attachment 310366 [details] [diff] [review] is unlikely to make any difference.  But, I didn't think this was true in 2.0.

Could this be a problem of SQLite performance?  I know that (for me) deleting items from my history/bookmarks is surprisingly slow, and if a livemark has 20 items in it, deleting those and adding new ones might be the problem (since for me, deleting 20 history items can take 20 seconds on better hardware than yours.)

Is there any way you could check if your problem occurs with your bookmarks in Firefox 2?  If they have different problems I'd suggest it should be a different bug.

Thanks,
-[Unknown]
IMHO this is a serious showstopper. I think it's incorrect to leave users with the browser practically hanged for many many seconds on start up, just for the sake that live bookmarks are updated before the user starts using the browser. It's a very bad user experience IMHO.

I have many live bookmarks which I frequently use, and I still think it's an incorrect priority decision.

I'm suggesting the following (some items may be obvious, each suggestion is independent):

1. IIRC, currently it's a theoretically background job. I.e. update while the user is using the browser. Understand that it isn't. It makes the browser practically unresponsive.

2. Understand what exactly makes the UI unresponsive and consumes CPU.

3. Make it work better in the background. Maybe download all the data and manipulate it in the background, and only when it's all done integrate it into the UI? Maybe it becomes unresponsive due to constantly modifying the menu system for each item?

4. Give some feedback to the user. I.e. status bar message (maybe with percentage/count?), progress bar etc.

5. Let the user control when live bookmarks are updated. I.e. Boolean preference to update bookmarks automatically periodically, and a Bookmarks menu item: "Update live bookmarks now". It's OK if auto-check is on by default, but at least let users control it if it degrades the experience for them.

I really hope this bug gets some attention, as it currently makes the Firefox experience very annoying for people who have many live bookmark items. myself included.
One more suggestion in addition to comment #43 :

6. Lazy evaluation of live bookmarks? i.e. update it automatically when the user hoovers this item (with proper feedback). maybe as an extension of sugg. 5: preference to auto-check/only-when-hovering-item/only-manually ? this will surely solve the issue and the perceived penalty would be minimal: I'm looking for items, it checks, it shows. The duration it would take to update would be acceptable since the user is actually interested in this data right now. Next hoovers will not trigger automatic reload unless the timeout period has passed since last update. 
Created attachment 325782 [details] [diff] [review]
Inserts a 1000ms "sleep" between live bookmarks items update. POC only, NOT production quality.

Added a patch to insert a 1000ms "sleep" *between* each item check (it isn't triggered every 1000ms such that it doesn't flood the checks if a single item takes more than 1s to complete). The patch is for nsLivemarkService.js from Firefox 3.0

This is proof of concept only and not production quality. I don't claim that it covers error cases, shutdowns, etc. It also forces a full live bookmarks update on browser start-up regardless of expiration, just to make sure it can be tested easily.

IMHO, this simple patch improves the experience dramatically by allowing the UI to "breath" between live bookmarks updates.

Comment 46

9 years ago
Correct me if I'm wrong, but I always thought the most obvious solution is to wrap each livemark load into a transaction (covering both deletion of old items as well as creation of all new items). This way there's a single database update for each livemark, which is well enough to solve this problem.

Comment 47

9 years ago
Sorry, there would be two operations per each livemark - insertion of "live bookmark loading" item (and that would be fixed by bug 407468), and then the full clear+update transaction. Still much better than current implementation, which I presume doesn't use transaction at all (and I only HOPE it's capable of deleting old items in one go, not using separate deletions per item).
I have about 50+ feeds. I've counted the number of times insertLivemarkChild is called (it inserts a single new bookmark into the system, I guess it includes updating the menu). It's called almost 900(!!) times for an average of about 20 items per feed. In a loop! without any sleep in between anything, and it both consumes 100% CPU and modifying the UI about 1000 times (including the deletion of older items)

Is it any wonder it slows the UI to a crawl?

I re-suggest allowing some sleep between updates, or build the whole new updated live bookmarks structure "offline" and then update the system in one transaction and/or let the user choose when the update occurs.

Updated

9 years ago
Duplicate of this bug: 439383

Updated

9 years ago
Keywords: hang

Comment 50

9 years ago
I am also encountering the problem with 100+ Live Bookmarks on Firefox 3/WinXP Home SP2 (high CPU and memory usage at start and during automatic reloading), but not on Firefox 2.

Here are my suggestions:
 - load the Live Bookmarks (automatic reloading only when Firefox is idle?) by groups of 'n' bookmarks, with a sleep of 's' seconds between each group (where 'n' and 's' can be tweaked in about:config)
 - allow the user to set the maximum number of items to be stored/displayed per feed (globally, not for each and every feed)

Also if we consider RSS feeds for example, only the <title> and <link> sub-elements of each <item> element are useful when Live Bookmarks are used solely with Firefox's built-in mechanism. Other sub-elements such as <description> are useless and do not need to be stored in memory (outside of feeds-related extensions such as Sage).

If we take Google News RSS feed: http://news.google.com/nwshp?hl=en&tab=wn&output=rss, removing all <description> sub-elements reduce the size from +120 kB to +14 kB.

So for people using Firefox's built-in Live Bookmarks mechanism only, having an option to store sub-elements minimally might be welcome to reduce memory usage.

Comment 51

9 years ago
I'm attaching a commented procexp (sysinternals) screenshot showing how the bug looks on my system (ff 3.0, Windows XP).

I've about a hundred live bookmarks.

I didn't have any problem with ff 2.x.

Comment 52

9 years ago
Created attachment 326087 [details]
commented procexp screenshot shows the bug in action

The screenshot shows the memory load log of the ff 3.0 process for a couple of minutes, just after the process is started

Comment 53

9 years ago
Could Bug 384474 be related? or duplicate?

This bug is really annoying and freezes up the browser for a while before you can browse to any web page every time Firefox 3 starts. I have a large number of live bookmarks.

In fact while reading this page I was unable to scroll down for a while as the live bookmarks were updating.

In my opinion this should be fixed asap and be released in 3.0.x as many users will be put off Firefox if they cant use the browser due to it stopping responding every time it is opened.

I'm not sure if this is the right place but a suggestion for improving live bookmarks is when hovering over a title in a live bookmark that is longer than the box, to automatically scroll the text in the live bookmark.

Comment 54

9 years ago
i hope i'm not in the wrong place, i just want to share my suggested solution to this challenge.

i have about 60 RSS feeds and i have manually set them to update every 15 minutes as i like to be as realtime with the rest of the universe as possible:)
i believe you will agree this makes me a very good example of an "worse case scenario" for this problem.

having a quad core processor PC with oodles of ram and 20Mb ADSL is no match for a firefox browser that wants to update 60 RSS feeds simultaneously.
                   **it still locks the PC !**

SUGGESTED SOLUTION 
===================
update all feeds asynchronously as opposed to synchronously.

update feeds asynchronously based on a staggered "time to next feed" frequency derived from the total number of feeds divided by update period set for the browser (or user set as in my case).

so for me it would be:
update period 15 minutes (900 seconds) divided by number off RSS feeds (60)=15 seconds between each updated RSS feed.

all my feeds are still updated every 15 mins and i suffer no lock ups. 

which is the priority?
========================
1)updating feeds SYNCHRONOUSLY according to the update period at the expense of breaking the browsing experience due to lock ups.

2)updating feeds ASYNCHRONOUSLY according to a better algorithm that maintains update period WITHOUT compromising browser experience.

**will the user realize more the feeds are not all updated in parallel, or their PC locking up ?**


i'm sure you can see that even in my extreme case, having ONE feed update every 15 seconds to maintain an update frequency of 15 minutes is MUCH more tolerable than freezing users PC's for seconds each time.

HOW?
=====
i believe the best way is have firefox MONITOR whenever the user creates/deletes RSS feeds. 

as the user changes the feed total (by creating or deleting feeds),or changes the update frequency,  these are the same values for calculating the "time to next feed" is adjusted.

i hope this gives insight into a solution for this problem that causes such inconvenience from such a small symptom.

Comment 55

9 years ago
I suggest people stop asking that the feeds be monitored in the background/async.  This would likely require threading things that would take a whole lot more work than this bug probably requires.

I'm sure it can be solved by simply taking more care in updating things.  As for SQLite, if this really is a bottle-neck, I'd assume using a transaction on the delete would solve it (I doubt one is already being used?)

Just my cent and a half-piece.

-[Unknown]

Comment 56

9 years ago
Same as bug 435072 ?
Duplicate of this bug: 435072
Created attachment 326686 [details]
Heavy Testcase: bookmark.html full of Live Bookmarks

Testcase from Bug 435072
Attachment #214211 - Attachment is obsolete: true
There's no text in whiteboard, I remove qawanted keyword. Added request for wanted-fx3.1
Flags: wanted-firefox3.1?
Keywords: qawanted

Comment 60

9 years ago
UNRESOLVED from 2006-03-06 WTF!

Come on guys this is really a bug that needs fixing what is the hold up?

I think the only reason its not reported more is the difficulty of diagnosing the real issue.

I am on 64 Ubuntu firefox 3 and confirm this is still causing slow startup and internment high cpu spikes (25% on a quad) for 30secs.

This shows FF3 in a very bad light.

Comment 61

9 years ago
(In reply to comment #60)
> Come on guys this is really a bug that needs fixing what is the hold up?

https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
For what I wrote in Bug 435072 Comment #18, Ria Klassen in Bug 397993 Comment #27 and Stan in Bug 397993 Comment #32, I think this bug happens because bookmarks are updated and written one-by-one to the hard disk, instead of written to HD one time at the end of the updating process. It think also bookmark updating is not an asynchronous operation.

Regression of Bug 379729? I add the keyword, remove it if you think it's not.

Can someone confirm this bug on Mac?
Keywords: regression
Whiteboard: [Related: 397993]

Comment 63

9 years ago
re comment 62: This is quite visible when you reload a live bookmark inside Library. I have one that is a live bookmark of a bugzilla search and it returns circa 1000 items. What I see is the "old items" slowly disappearing, being deleted, one by one. It is incredibly slow, and I am sure better transaction logic would help immensely.

Comment 64

9 years ago
(In reply to comment #61)
> (In reply to comment #60)
> > Come on guys this is really a bug that needs fixing what is the hold up?
> 
> https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
> 

Yeah apologies for the inappropriate rant :(

I should have instead settled for expressing my surprise and dismay that this was not considered a blocker for FF3.

I promote Firefox at every opportunity and this problem seriously degrades the user browsing experience.


(Reporter)

Comment 65

9 years ago
Everyone is aware of the problem, just adding "me too" comments won't fix this bug.
Please stop spamming, thanks
Whiteboard: [Related: 397993] → [Related: 397993, 391836, 393497, 432706]
Blocks: 397993
Pretty major usability problem for users with even 20 RSS feeds monitored through live bookmarks, and it doesn't seem like B1 made it go away.  Would be good to get a comment from someone on the Places team about the approaches suggested here, the prospect of a 3.0.x-appropriate fix, and what should happen in our test suite to better cover the performance aspects of different places-store scales.  Nay, would be great!
Flags: wanted1.9.0.x?
Comment on attachment 310366 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 3

(In reply to comment #42)
> (In reply to comment #40)
> > It's caused by the *number of total items* in feeds (e.g., news articles in a
> > news feed), and not the *number of feeds*:
> 
> Given that, attachment 310366 [details] [diff] [review] is unlikely to make any difference.  But, I
> didn't think this was true in 2.0.

I'll file a separate bug for that. Let's keep this one focused on distributing the load. If deleting/adding bookmarks is a pain point, 

This grouped-distribution approach should be sufficient to:

1. mitigate the long periods of high CPU usage when processing feeds
2. provide a level of customization that allows the commenters present here to experiment to find the right defaults.

Thanks for the initial patch. Comments below.

>+// We read gLimitCount Livemarks / gDelayTime sec
>+var gLimitCount = 1;
>+var gDelayTime  = 5;
>+

please document each of these.

also, i think that the default for gLimitCount is too conservative. users complaining about this problem have many livemarks, for the most part. let's start with 3, and see how it goes. i'd like to avoid having new bugs filed for livemarks not being updated promptly enough, due to this patch.

>@@ -136,16 +141,32 @@
>     var livemarkRefresh =
>       prefs.getIntPref("browser.bookmarks.livemark_refresh_seconds");
>     // Reset global expiration variable to reflect hidden pref (in ms)
>     // with a lower limit of 1 minute (60000 ms)
>     gExpiration = Math.max(livemarkRefresh * 1000, 60000);
>   }
>   catch (ex) { }
> 
>+  try {
>+    gLimitCount = prefs.getIntPref("browser.bookmarks.livemark_refresh_limit_count", 1);
>+    if ( gLimitCount < 1 ) {
>+      gLimitCount = 1;
>+    }

current style for this file is no brackets when the contents are single-line

>@@ -221,20 +238,39 @@
> 
>     // kill timer
>     if (this._updateTimer) {
>       this._updateTimer.cancel();
>       this._updateTimer = null;
>     }
>   },
> 
>+  // bug 329534 : we try to distribute the load of the livemark update.

please add a short description of the distribution approach
 
>+  _checkNo : -1,

please rename this such that it more accurately describes it's use, eg: _nextUpdateStartIndex or something like that.

also, why not set it to 0 here (and at the end of a cycle)? that removes the need for the +1 logic you have in a few places below.

>   _checkAllLivemarks: function LS__checkAllLivemarks() {
>-    // check if livemarks are expired, update if needed
>-    for (var i = 0; i < this._livemarks.length; ++i) {
>-      this._updateLivemarkChildren(i, false);
>+    var startNo = this._checkNo+1;
>+    var count = 0;
>+    for (var i = startNo; (i<this._livemarks.length) && (count<gLimitCount); ++i ) {
>+      // check if livemarks are expired, update if needed
>+      if (this._updateLivemarkChildren(i, false)) {
>+        count++;
>+      }

remove unnecessary brackets

>+      this._checkNo = i;
>+    }
>+    // 
>+    if ( (this._checkNo + 1) >= this._livemarks.length ) {
>+      // all livemarks are checked, sleeping until next priod
>+      this._checkNo = -1;
>+      var refresh_time = Math.min(Math.floor(gExpiration / 4), MAX_REFRESH_TIME);
>+      this._updateTimer = new G_Alarm(BindToObject(this._checkAllLivemarks, this),
>+                                      refresh_time); /* no repeat */

removing the repeating alarm means that if something above throws, there'll be no more update checks for the rest of the app lifetime. please wrap the updateLivemarkChildren() calls in a try/catch block.
Attachment #310366 - Flags: review?(dietrich) → review-
Target Milestone: --- → Firefox 3.1a1
(Assignee)

Comment 68

9 years ago
Created attachment 328445 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 4

> also, i think that the default for gLimitCount is too conservative. users
> complaining about this problem have many livemarks, for the most part. let's
> start with 3, and see how it goes. i'd like to avoid having new bugs filed for
> livemarks not being updated promptly enough, due to this patch.

My PC is Athlon 64 X2 4800+ and 8GB mem, using VISTA 64.
If this setting is 3 Livemarks / 5sec, or 3Livemarks / 10sec, Fx becomes heavy occasionally. Therefore, default setting was 1 Livemark / 5sec.
But, changed to '1 Livemark / 3 sec' as a compromise proposal.
Attachment #310366 - Attachment is obsolete: true
Attachment #328445 - Flags: review?(dietrich)
Summary: Live bookmarks load way too agressive (lock up/hang/freeze browser) → Live bookmarks load way too aggressively (lock up/hang/freeze browser)
Comment on attachment 328445 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 4


>@@ -80,16 +81,22 @@
> const SEC_CONTRACTID = "@mozilla.org/scriptsecuritymanager;1";
> const IS_CONTRACTID = "@mozilla.org/widget/idleservice;1";
> const SEC_FLAGS = Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL;
> const NS_BINDING_ABORTED = 0x804b0002;
> 
> // Expire livemarks after 1 hour by default
> var gExpiration = 3600000;
> 
>+// Number of livemarks that read one time

s/read one/are read at one/

>+  try {
>+    gLimitCount = prefs.getIntPref("browser.bookmarks.livemark_refresh_limit_count", 1);

nsIPrefBranch.getIntPref() doesn't take a default value as a second argument.

>+    if ( gLimitCount < 1 ) gLimitCount = 1;
>+  }
>+  catch (ex) { }
>+
>+  try {
>+    gDelayTime = prefs.getIntPref("browser.bookmarks.livemark_refresh_delay_time", 3);

ditto



>   _checkAllLivemarks: function LS__checkAllLivemarks() {
>-    // check if livemarks are expired, update if needed
>-    for (var i = 0; i < this._livemarks.length; ++i) {
>-      this._updateLivemarkChildren(i, false);
>+    var startNo = this._nextUpdateStartIndex;
>+    var count = 0;
>+    for (var i = startNo; (i < this._livemarks.length) && (count < gLimitCount); ++i ) {
>+      // check if livemarks are expired, update if needed
>+      try {
>+        if (this._updateLivemarkChildren(i, false)) count++;
>+      }
>+      catch (ex) { }
>+      this._nextUpdateStartIndex = i+1;
>+    }
>+    // 

remove extraneous empty comment

>+    if ( (this._nextUpdateStartIndex) >= this._livemarks.length ) {
>+      // all livemarks are checked, sleeping until next priod

s/priod/period/

other than these small fixes, the change looks good, r=me. however, before checking-in we need to add a test for this change. see this for example livemark tests:

http://mxr.mozilla.org/seamonkey/source/toolkit/components/places/tests/chrome/

test could do something like this:
- add two feeds
- create a bookmarks observer
- call livemarkservice.start() // starts the update timer, calls checkAllLivemarks()
- confirm via the observer that only the first feed was checked within the first period
- confirm via the observer that the second feed was checked within the second period
Attachment #328445 - Flags: review?(dietrich) → review+
(Assignee)

Comment 70

9 years ago
Created attachment 329386 [details] [diff] [review]
 simple patch for distribute the load of the livemark update rev 5
Attachment #328445 - Attachment is obsolete: true
Attachment #329386 - Flags: review?(dietrich)
(Assignee)

Updated

9 years ago
Attachment #329386 - Attachment is obsolete: true
Attachment #329386 - Flags: review?(dietrich)
(Assignee)

Comment 71

9 years ago
Created attachment 330339 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 5 with test
Attachment #330339 - Flags: review?(dietrich)
some thoughts...

first: personally i would not add two new prefs for this, i mean: this is an internal work redistribution and we should find a good compromise without having to rely on user tweaks.

second: having a default delay time of 3s appear too much to me, we must find a compromise clearly, but if i have 100 livemarks i could potentially have to wait 5 minutes before reading a livemark in my toolbar, and that's not cooler that having the ui hang for a couple seconds. 

So we should also try to optimize update in other ways to be able to reduce the delay time between feeds to at least 1s.
To optimize i think we should reduce the sql traffic generated during the update, a first minor tweak is fixing bug 407468, so we don't need to add anymore bookmarks for feed status.

A major tweak could be instead optimizing nsNavBookmarks::RemoveFolderChildren, we are using it to clear feed items on every update. The problem with it's approach is that it's a recursive function that for every item calls RemoveItem or RemoveFolder, and for every call we get:

- a query to get item properties
- a query to remove annotations
- a single itemId delete query
- a query to adjust indexes
- a query to adjust lastModified
- a query to update frecency
- call observers

if a feed contains 10 items and you have 100 feeds you end up with more than 6000 queries, many of wich are mostly useless (why updating indexes and lastmodified for every delete if we are removing all folder contents?).

That could be reduced to
- Build recursively an hash of items to be removed (for each item we get properties we will need later), 1 recursion/query for each folder (could be only 1 query if in future we could use a preordered nested tree table)
- 1 global query to remove all annotations
- 1 global query to remove all items/folders
- don't adjust indexes since we don't need it (removing all childs)
- 1 global query to update parent lastmodified when finished
- 1 query to update frecency (? tbd)
- call observers (correctly, since the hash has still infos for each item)

In the same case as above we could be reduced to 4 global queries, maybe slower queries but definetely less disk hog. That would probably require some code duplication from removeItem and removeFolder, but could also give a perf gain in places generally.
(In reply to comment #72)
errata:
> In the same case as above we could be reduced to 4 global queries
In the same case as above we could be reduced to 40 global queries (since we will call RemoveFolderChildren once for every feed)

agh! forgive me, 400! 100 feeds = 100 calls to removeFolderChildren, still better than 6000 though :)

Comment 75

9 years ago
has anybody actually run the code trough a profiler?
about comment #72 i found it has been reported by Ondrej some time ago, it's bug 418643 and i've attached a wip patch there. (adding as a dependancy)

ayakawa could you test your timings with that patch in and see if they can be reduced?
Depends on: 418643

Updated

9 years ago
Depends on: 407468
(Assignee)

Comment 77

9 years ago
I made Fx with my patch & bug 418643's patch, and used it(loading 3 livemarks / 5 sec). It is good than before, However, locking up somewhat.

i think that another lock up point is where we add bookmarks, doing 3 livemarks could potentially end up in in 50-60 *almost* consecutive inserts and even if we  are in batched mode could be an heavy disk usage, i suggest hardcoding 1 livemark update per pass and only tweaking delay time between them (so trying with 1lm and 2s delay) or try with 2lms in 3s
What about doing the updating on a background thread?  You won't have to worry about the main UI thread locking up then.
(In reply to comment #79)
> What about doing the updating on a background thread?  You won't have to worry
> about the main UI thread locking up then.

I agree, still if we doo too much concurrent IO access the UI will not hang but the system will still become slow. So imho better would be to do both, distributing work in a background thread.

Comment 81

9 years ago
any news if a fix for this bug made it into 3.1a1?
Comment on attachment 330339 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 5 with test

r=me, thanks very much, and sorry for the delay.

per the comments about alternate solutions: this patch allows us to easily change the defaults to increase or decrease load. this means we get some perf win, while also a framework to allow heavy users to modify their environment. please file separate bugs for further improvements.
Attachment #330339 - Flags: review?(dietrich) → review+
Keywords: checkin-needed
Hardware: PC → All
Target Milestone: Firefox 3.1a1 → Firefox 3.1a2
pushed with changeset fb56f835e52d
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
and test pushed in changeset 3323c9df20fc
I backed out fb56f835e52d because the test was failing on tinderbox, and probably causing leaks as well.  (I left the test in, though ... just not part of the build.)
http://hg.mozilla.org/mozilla-central/index.cgi/rev/10af545c0511
http://hg.mozilla.org/mozilla-central/index.cgi/rev/59569994531e
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Error from log:

*** 440 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/components/places/tests/chrome/test_329534.xul | Test timed out.

Leak data in log:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1218046424.1218050232.7424.gz
(Assignee)

Comment 87

9 years ago
livemarkservice.start() has called when mochitest start.
When test_329534.xul runs and livemarkservice.start() in this test has called,
this function do nothing. because this function is called already...
And, It is necessary for the end of test_329534.xul for one hour. 

livemarkservice don't have 'open API' for stop updating livemark,
therefore, we don't 'restart' livemark updating. Can we add new API ?

  stopUpdateLivemarks: function LS__stopUpdateLivemarks() {
    for (var livemark in this._livemarks) {
      if (livemark.loadGroup)
        livemark.loadGroup.cancel(NS_BINDING_ABORTED);
    }
    if (this._updateTimer) {
      this._updateTimer.cancel();
      this._updateTimer = null;
    }
  }

  and, test is

  // start updating livemarks
  lmsvc.stopUpdateLivemarks();
  lmsvc.start();

or livemarkservice.start() function change

  start: function LS_start() {
    if (this._updateTimer) {
      // cancel all updating when restart
      for (var livemark in this._livemarks) {
        if (livemark.loadGroup)
          livemark.loadGroup.cancel(NS_BINDING_ABORTED);
      }
      if (this._updateTimer) {
        this._updateTimer.cancel();
        this._updateTimer = null;
      }
    };
    // start is called in delayed startup, 5s after browser startup
    // we do a first check of the livemarks here, next checks will be on timer
    // browser start => 5s => this.start() => check => refresh_time => check
    this._checkAllLivemarks();
  },

Besides, isn't there good idea?
Depends on: 432706
No longer depends on: 418643
(Assignee)

Comment 88

9 years ago
Created attachment 333721 [details] [diff] [review]
 simple patch for distribute the load of the livemark update rev 6 with test

I change nsILivemarkService.idl for test. I think better than changing start() func.
Attachment #330339 - Attachment is obsolete: true
Attachment #333721 - Flags: review?(dietrich)
Flags: wanted1.9.0.x? → wanted1.9.0.x+
We really want this for 3.1.  Dietrich, what is left to do here?
Flags: wanted-firefox3.1?
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
Comment on attachment 333721 [details] [diff] [review]
 simple patch for distribute the load of the livemark update rev 6 with test


>+
>+  /**
>+   * Stop the livemark refresh timer.
>+   */
>+  void stopUpdateLivemarks();

this api will also be useful for turning off livemark updates when offline, etc, which is something planned, so that sounds fine.

>+// Number of livemarks that are read at one

typo in comment s/one/once/

>   _shutdown: function LS__shutdown() {
>     // remove bookmarks observer
>     this._bms.removeObserver(this);
> 
>-    for (var livemark in this._livemarks) {
>-      if (livemark.loadGroup)
>-        livemark.loadGroup.cancel(NS_BINDING_ABORTED);
>-    }
>-
>-    // kill timer
>-    if (this._updateTimer) {
>-      this._updateTimer.cancel();
>-      this._updateTimer = null;
>-    }
>+    // stop to update livemarks
>+    stopUpdateLivemarks();
>   },

should that not be this.stopUpdateLivemarks()?

>--- /dev/null	Wed Aug 13 11:25:56 2008
>+++ toolkit/components/places/tests/chrome/test_329534.xul	Wed Aug 13 11:15:30 2008

note that the test file doesn't need to be re-added, just updated, since dbaron didn't back out the file.

r=me with these changes
Attachment #333721 - Flags: review?(dietrich) → review+
(Assignee)

Comment 91

9 years ago
Created attachment 334649 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 6.1 with test

I change testcase a little bit.
Attachment #333721 - Attachment is obsolete: true
Attachment #334649 - Flags: review?(dietrich)
Attachment #334649 - Flags: review?(dietrich) → review+

Updated

9 years ago
Flags: blocking1.9.0.3?
(Assignee)

Comment 92

9 years ago
Created attachment 338234 [details] [diff] [review]
simple patch for distribute the load of the livemark update rev 6.1.1 with test

rewrite patch because bug 454360's patch cause to error for my patch.
Attachment #334649 - Attachment is obsolete: true
Attachment #338234 - Flags: review?(dietrich)
Attachment #338234 - Flags: review?(dietrich) → review+
Keywords: checkin-needed
two problems:
1. this is going to bitrot again due to bug 407468 (that should land before the string freeze, so it should land before this one)
2. bug 453530, when finished, will practically made the gLimitCount useless, that's why i still think that having 2 prefs is too much. Hwv we will still need both stopUpdateLivemarks and some sort of distribution, so i'm ok to take this after bug 407468 (sorry ayakawa you'll need to unbitrot again :() and remove the pref later.
Removinc checkin request for now, this can land after the string freeze (please)
Keywords: checkin-needed
bug 407468 has landed.
Ayakawa, do you have some time to unbitrot this? sorry for having to unbitrot again, i don't have any more patches requiring string changes, so this will be the next one :)

Updated

9 years ago
Blocks: 453530
(Assignee)

Comment 96

9 years ago
Created attachment 340715 [details] [diff] [review]
 simple patch for distribute the load of the livemark update rev 6.1.2 with test
Attachment #338234 - Attachment is obsolete: true
Attachment #340715 - Flags: review?(dietrich)

Updated

9 years ago
Attachment #340715 - Flags: review?(dietrich) → review+
Comment on attachment 340715 [details] [diff] [review]
 simple patch for distribute the load of the livemark update rev 6.1.2 with test

thank you, i've tested patch and i'm going to push, since dietrich has already r+ it before the unbitrot
http://hg.mozilla.org/mozilla-central/rev/c33829c5965d
Assignee: nobody → ayakawa.m
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3.1a2 → Firefox 3.1b1

Comment 99

9 years ago
I've noticed that if I have multiple instances of ff3 open, say ff3_A and ff3_B, and ff3_A triggers this bug (updating the livebookmarks), ff3_A will steal the windows focus from ff3_B.

Am I the only one having noticed that (I'm under win xp)?
Is this already addressed in the patch?
(In reply to comment #99)
> I've noticed that if I have multiple instances of ff3 open, say ff3_A and
> ff3_B, and ff3_A triggers this bug (updating the livebookmarks), ff3_A will
> steal the windows focus from ff3_B.
> 
> Am I the only one having noticed that (I'm under win xp)?
> Is this already addressed in the patch?

not addressed, it will be when we will move livemark updates to the background thread. if you don't find a filed bug for that please file it
on MacOSX Darwin 9.2.2 moz2-darwin8-slave01:

TEST-UNEXPECTED-FAIL |
chrome://mochikit/content/chrome/toolkit/components/places/tests/chrome/test_329534.xul
| not passed 3sec when second
livemark loading

will keep an eye on next build. seems fine on the other unit test boxes.
Same thing, moz2-darwin8-slave01 dep unit test on 2008/09/27 18:12:19
is it failing intermittently? maybe we should relax it a bit to accomplish time due to loading
mh if that is a VM (the slave makes me think that) could also be due to VM timers problems, so calculating the diff between the 2 dates could return wrong time delayed. Checking again the test this appear more probable since it's looking for >=, so nvm comment 103. We could use now() and check that the 2 times are different before performing the delayed test, and maybe relax its requirement to >= 2s.
Created attachment 340978 [details] [diff] [review]
followup patch

Try to fix intermittent test failures, supposing they are due to VM timers bugs.
Attachment #340978 - Flags: review?(dietrich)
Comment on attachment 340978 [details] [diff] [review]
followup patch

r=me, fine for now. we can tighten it up later if this fixes it.
Attachment #340978 - Flags: review?(dietrich) → review+
follow-up pushed:
http://hg.mozilla.org/mozilla-central/rev/04e3942b6d65

let's wait to see if the test is still failing intermittently

Updated

9 years ago
Whiteboard: [Related: 397993, 391836, 393497, 432706] → intermittent test failure, workaround pushed
Flags: blocking1.9.0.4? → blocking1.9.0.4-

Comment 108

9 years ago
Is this patch branch material? It is very frustrating to have Firefox freeze (not reacting to any interaction at all) for almost half a minute when started.
Can people that have experienced this bug please comment on how effective this fix is? I'd like to confirm solvency prior to requesting branch approval.

Comment 110

9 years ago
Tested with Gecko/20081014 Minefield/3.1b2pre. Seems fine.

But it's only a fast-visual-test. I will start using minefield for daily demands, to make 100% sure it's fixed.

Comment 111

9 years ago
Testing with Gecko/20081014 Minefield/3.1b2pre (Windows)
The situation looks greatly improved, still:
1. the very first run right after installation has a rather long delay while (I presume) importing (and updating?) the live bookmarks from Firefox 3
2. I noticed some frame dropping (used to freeze) while watching a long youtube video (fully buffered, no other applications running)

p.s.
just to encourage other testers: you can get the latest test build from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
and be assured that it won't mess up your current Firefox installation
(In reply to comment #109)
> Can people that have experienced this bug please comment on how effective this
> fix is? I'd like to confirm solvency prior to requesting branch approval.

Dietrich, you're not going to be able to land this as is on the branch due to the interface change.

Updated

9 years ago
Duplicate of this bug: 462987

Comment 114

9 years ago
Hi there,

I tried to rebuild Firefox 3.0.5 with this patch but I don't see any improvements on this? Did I do anything wrong or I missed something? Would anyone make a patch for Firefox 3.0.5 please :( I can't stand my Firefox is frozen endlessly each time it gets live bookmarks updated.

BTW, I did replace my whole Firefox 3.0.5 folder with new build, it's a kind of stupidity, so could anyone tell me which files should be updated plz?

Thanks in advance.
Duplicate of this bug: 384474
yep, mossop's right. need to find a way to get this fix w/o the interface change for branch. maybe should change to observe a shutdown notification instead?
The usual method would be to create a new nsILivemarkService_MOZILLA_1_9_0_BRANCH interface that inherits from the existing one and put the new method on the new interface. Old code can continue to use the old class without breaking.

Updated

8 years ago
Duplicate of this bug: 456973

Updated

8 years ago
Duplicate of this bug: 499832

Comment 120

8 years ago
(In reply to comment #119)
> *** Bug 499832 has been marked as a duplicate of this bug. ***

This problem STILL EXISTS in the latest 3.5 RC2!  It is NOT fixed.

Comment 121

8 years ago
agreed the problem still exists, this problem was most visible when starting firefox as it caused it to hang immediately for a few seconds.

It seems that instead of fixing the problem, firefox just doesn't check live bookmarks on startup anymore, hiding the problem but not fixing it.

Problem can still be reproduced when all live bookmarks are refreshed on live bookmark timeout. It can also be reproduced with the reliby extension.
live bookmarks are still checked on startup, just not all toghether, instead the updates are distributed, having a really high number of feeds could still show some issue, but the browser is not a feed reader, it provides basic support for users, without the need to have all the features and whisltes of a feed reader.
That said, we plan on further improvements, of course.

Comment 123

8 years ago
whether or not it's classified as a "feed reader" is irrelevant.  it doesn't function for those minimal built-in feed reader functions (which are more than sufficient for me) nearly as well under 3.x as it did under 2.x.  Period.  And there is sufficient evidence for it being solely due to the radical departure from the old bookmarking/history system to a mini database (that doesn't know how to stay "mini", apparently).

if the places.sqlite remains resident in memory while it's being dynamically written and re-written, the unbounded growth in file size should be considered a critical error because it will be a continuously increasing resource hog.  the other reports that it has gotten up to and exceed 170MB in size should be alarming in that case.  That unbounded growth has not changed in FF 3.5 RC2.  Just since doing a fresh install a few days ago, and with minimal usage of it as a test case, I've seen the places.sqlite db grow from a little over 5MB to 7.1MB.  this is consistent with the growth i saw as a normal installation over the course of several weeks, in which the final file size at uninstall was 66MB.
(In reply to comment #123)
> whether or not it's classified as a "feed reader" is irrelevant.  it doesn't
> function for those minimal built-in feed reader functions.

having 350 live bookmarks can't be considered " a minimal function". And no way the old database system could have been faster than the new one (indeed history moved from 9 days to 180days).
It is not clear if your issue is completely due to livemarks, or to safebrowsing updates, especially with 3.5RC2 (could you try disabling safebrowsing? Under security, uncheck "Block reported attack site" and "Block reported web forgery", then restart browser).
That said, could you check in about:config if you have any perf containing the word "livemark"?

finally i would suggest you to send your bookmarks.html file to me or to another developer by mail, that would be useful for debugging.

Comment 125

8 years ago
(In reply to comment #124)
> having 350 live bookmarks can't be considered " a minimal function". And no way

Remembering that I dont need 350 livemarks to cause the issue.
Only 10 is sufficient.
(In reply to comment #125)
> (In reply to comment #124)
> > having 350 live bookmarks can't be considered " a minimal function". And no way
> Only 10 is sufficient.

i have more than 10, as i suspect most of the users and never seen any problem, so in this case i can suspect a profile configuration problem. also you should always point which version are you referring to, we are talking about 3.5.

Comment 127

8 years ago
I can confirm that this problem still exists, at least for me.

Out of curiosity, I created a fresh profile on Fx 3.5 RC2 (Win 7 RC). It doesn't seem like there's any problem updating a single Live Bookmark (Firefox comes with "Latest Headlines" feed pre-installed), but I began noticing a delay once I added 5 feeds or so. With ten feeds, a good ten seconds passed before the UI became responsive again after updating the Live Bookmarks.

Note: I also installed Reliby, which is the easiest way to manually force Live Bookmarks to update. I also disabled Safe Browsing, as Marco suggested above.
What happens with Reliby only becomes interesting when its developer, or the developer of another extension doing the same thing, asks (in a new bug) for a different API. Reliby is just calling reloadAllLivemarks, which is intended to be a brutal "check even if the livemark itself knows it shouldn't be checked again yet" API, which the patch here didn't touch because that wasn't what this bug was about, and because that's not what Fx itself does to reload. So what you see with a Reliby reload has absolutely no relationship at all to this bug, or to Firefox's startup or periodic checks.

Comment 129

8 years ago
(In reply to comment #126)
> (In reply to comment #125)
> > (In reply to comment #124)
> > > having 350 live bookmarks can't be considered " a minimal function". And no way
> > Only 10 is sufficient.
> 
> i have more than 10, as i suspect most of the users and never seen any problem,
> so in this case i can suspect a profile configuration problem. also you should
> always point which version are you referring to, we are talking about 3.5.
 As I posted ages ago, in mozillazine forums, i used some live bookmarks with a lot of items, the issue is not only caused by # of feeds, but the # of feeds versus # of items of each feed.

Not it's is not a profile issue, i'm consider myself very experient with Firefox (since 0.x versions, before the name "Firefox"), and I tested in various scenes. This includes new machines, recently formatted  Windows XP and Vista. I reproduced in Ubuntu and Windows in the same machine.

The feeds I used to test are from mainly newspapers with 50+ items.

I simply tired of test, and test again, and started to use Chrome + Netvibes to do this task. 

I dislike the interface of the traditional RSS Readers, they appear a mail program, that don't help to read fast all I want to.

Can I choose Firefox someday again?
PLEASE READ:
If you are using Reliby, a bug in that extension may be causing your problem.  Please verify that the issue occurs with Reliby uninstalled before adding comments here.

For support information about Reliby, please contact its author:
http://gemal.dk/mozilla/reliby.html

PLEASE NOTE: This is not the proper bug for problems with Reliby.

-[Unknown]

Comment 131

8 years ago
(In reply to comment #128)
> Reliby is just calling reloadAllLivemarks, which is intended to
> be a brutal "check even if the livemark itself knows it shouldn't be checked
> again yet" API

Fair enough. But the problem exists for me whether or not Reliby is installed. My normal profile does NOT use Reliby and I _always_ know when Live Bookmarks are being checked in the background because the UI is far less responsive* for a good thirty seconds.

Once again, I created another fresh profile on Fx 3.5RC2 to test, this time without Reliby. I added ten random Live Bookmarks of varying sizes. I left Firefox closed for a couple of hours, then restarted, knowing the Live Bookmarks should be expired and updated at startup (technically, on delayed startup). The problem is slightly different than when I brute forced the updates. Instead of ten seconds with a completely unresponsive UI, I experience twenty seconds with a half-responsive UI. So... the staggered updates help, but don't really solve the problem for me.

(I also left Firefox open for a couple of hours and let the Live Bookmarks expire and update in the background as normal. Every hour or so, the problem occurs, consistently and predictably.)

* When I say "responsive," I mean as responsive as expected. Everything I attempt still occurs, just not at the rate it should. It's most obvious when I try to type during this period, because half the letters I type appear as I type them, the other half appear seconds later, all at once.

Comment 132

8 years ago
As I wrote in comment 63, the items in a live bookmarks seems to be written one by one, and deleted one by one, as you can see sometimes when you manually refresh a livemark.

My conclusion is that the patch that "fixed" this bug is based on the faulty assumption that the bug summary is correct in its assumption, that firefox is too aggressive in loading livemarks, rather it is the operation of altering the database in which the items are stored that are not scalable enough, and that the updates doesn't seem to be batched together in a transaction makes the matter just worse. This is my layman's guess of course. This is why the patch doesn't fix the whole problem, because it just mitigates the problem a bit, but it fails to solve the underlying problem because it doesn't address it at all.

This worked fine in 2.x as far as I can remember. I have this problem every day when I start Firefox and in the middle of browsing session, and it is the single biggest annoyance of the Firefox browser. To not be able to have 10 or 20 or even 50 or 60 live bookmarks, makes it a feature more or less useless, and the capacity limit is too easy for a relatively "normal" user to hit. One a bit older computers the problem is even more dismaying.

Comment 133

8 years ago
(In reply to comment #131 & #132)

> Fair enough. But the problem exists for me whether or not Reliby is installed.
> My normal profile does NOT use Reliby and I _always_ know when Live Bookmarks
> are being checked in the background because the UI is far less responsive* for
> a good thirty seconds.
> My conclusion is that the patch that "fixed" this bug is based on the faulty
> assumption that the bug summary is correct in its assumption


Concur with pretty much all of this.  I see the exact same things.  It's better in 3.5 RC2 (or RC3), but still not usable.  In FF3.0.x, after startup, the UI would lock up and even the window would not redraw when other windows were moved in front of it.  In 3.5, the stuttering is just distributed over a longer period of time.  Case in point:  I was just trying to type in a problem response for work related items after bringing up 3.5 for the purpose of testing it, and it was unusable due to the staccato typing response.  In fact, it was started 20 minutes ago, and is still stuttering in response to the same degree as when I opened it up.  This stuttering responsiveness also occurs when scrolling up/down through a web page or clicking on browser or page features.

I do not, nor have I ever had Reliby installed, so that precludes that extension as being at the heart of this matter.  As I think I've stated before, this occurs even with NO extensions installed.

I agree that the problem probably lies somewhere in the sql implementation of the bookmarking/history files, rather than the actual feed fetching.

Comment 134

8 years ago
is there any work being done to address the reports on this bug from the past month?  I made the last entry here, and that was three weeks ago.
all of the things reported in the release candidates of FF 3.5 are present in the official 3.5 version, and have not seen any bugfix releases.
This bug addressed the problem summarized in this bug's summary.

The problem of too much database churn and disk i/o is over in bug 453530. Please CC yourself there.
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.