I was looking for an screenshot image I put on grab.by recently, so I opened up the history window and dis a search for "grab.by". It takes 5-10 seconds for results to finish [with no feedback :(], at which point it says "454 items" at the bottom of the window. At this point, I ran into 2 problems, which are probably(?) different aspects of the same issue. (1) I started clicking on my search results (to find the image I was looking for), and quickly noticed that this was _very_ slow. About every other click caused the browser to freeze for a couple seconds. This made it extremely hard to actually use the search results. (2) I left the search window open, and went to go load a few unrelated sites. Sites were slow to load, and clicking in the URL bar seemed to freeze the browser for a couple of seconds. Not sure what's happening, but having the browser become this slow when a history window is open is a bad problem. :(

[To be clear, this is on a trunk nightly: Gecko/20100329]

this is due to history live-updating, since the query is synchronous, and at each visit we have to update it. i must notice in trunk we should be about 90% faster than in 3.5, this is obviously not enough though, we relly need being async, and, if possible, searching at db level.

the last part is most likely FTS.

How about making the results not be live (at least for now)?

it should just require a new option to get a "dead" result. I think we already thought to add it for other reasons, so would not be impossible. A better fix would be to detect the query params, check if the new entry satisfies them or not, and refresh only in that case, this could be possible for common cases like a simple search term. How big is your places.sqlite? 5-10s sounds like a lot, my DB (about 70MB) takes maximum 4 seconds to search in full history.

(In reply to comment #5) > How big is your places.sqlite? 5-10s sounds like a lot, my DB (about 70MB) > takes maximum 4 seconds to search in full history. 61MB.

drew's async query work might help here.

was this in the history sidebar or the library?

denying blocking on this, since neither piece is primary UI. i was able to reproduce this with a large history file on linux, and yeah it's annoying. there's not a wanted flag available yet, but consider it set.

This is currently much more visible and worse due to bug 595530 (that will be fixed apart).

Limi asked about this recently, cc-ing him. We could disable live-updates in searches before final, if the performance will still suck so much after places branch merges back.

(In reply to comment #12) > Limi asked about this recently, cc-ing him. > We could disable live-updates in searches before final, if the performance will > still suck so much after places branch merges back. If it can't be fixed easily, we should. It absolutely *destroys* browser performance when active/enabled, even with a modest browser history like my own.

OK, we just tested this after the Places merge. It still absolutely kills the browser. Multi-second beachballs while loading google.com front page is crazy. I suggest that we just turn off the live updating, and require people to hit enter again in the search field to update the search with the new history that has been added instead of trying to update it while you are browsing.

I think it is way too late int he release process to do anything here other than to limit the default amount of history saved to something that makes this issue be tolerable. (Just my opinion, I could be wrong)

(In reply to comment #16) > I think it is way too late int he release process to do anything here other > than to limit the default amount of history saved to something that makes this > issue be tolerable. (Just my opinion, I could be wrong) That's not true at all, FWIW. Comment 15 has a viable solution.

Nominating as soft-blocker, since this effectively renders our browser unusable if you have a search open (try it, I'm not exaggerating :( ), and should be allowed to land if we get a fix ready in time. (I know it's tagged as "wanted", but I think soft/hard blocker lists are what people will be looking at for this release cycle.)

I have a wip patch for this I worked in spare time, it is still failing some test though that I have to debug but it's promising, I hope to be able to give you a trybuild along this week, and that you can help me testing it. It's not high prio though, this is a wanted, just below a soft-blocker. Disabling liveupdate is a bit easier than properly fixing the bug at first glance, but there are lots of cases where it would require even worse hacks and I'd still have to handle test failures, so I'll keep that as a last resource since fixing the actual bug would be globally better. My patch should fix the live-update case, while the initial query will still be slow. This slowness is instead due to the fact we collect all results before filtering them (if a row conversion takes 100microsecs and you multiply it per 50 000 results, you easily get 5 seconds of slowdown), the simple solution here would be to limit the number of results (Also because a user can hardly manage more than a hundred results), unfortunately this requires some refactoring of the pipeline so that we can filter results one-by-one and stop when we have enough. Stopping at the required number of results would be hard though, because we allow to sort later than when we get results, so we'd not know which rows are good. The locationbar has easier life here because sorting is hardcoded, it is just frecency. Could probably be done in some case, but it's not something I'd feel comfortable fix for FX4.

(In reply to comment #19) > My patch should fix the live-update case, while the initial query will still be > slow. Yes, I think we should handle that in a follow-up bug post-FF4, I agree.

This is a bit larger than I expected, but ends up having lots of tests in Places is finally useful for something :) I'm going to land this on tryserver, get trybuilds and ask cooperation to test eventual live-update edge-cases that the tests could be missing, plus feedback on whether this still freezes in some situation. Will report links here once I have the builds.

Builds here: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mak77@bonardo.net-424e25d020d4/ How are you supposed to test this: - make a new clean profile - copy your huge places.sqlite from your main profile to the test profile - play with history/bookmarks sidebar and library What are you supposed to test: - when library is open or a search is open (in either library/sidebar) the browser UI should not freeze - live-update of queries when you add/remove visits, pages, bookmarks What is NOT fixed: - the initial search is still slow, see the second part of comment 19 for details.

This really helps in my case, the browser is usable again, even with a search open. The initial search is still very slow, but this fixes accidentally leaving the History window open with a search active, which would absolutely kill your performance before — we're talking 5-10s freezes/beachballs on OS X when opening google.com.

(In reply to comment #22) The to suggested test cases are totally fine with the try build. But searching in History still freezes the whole UI as expected, which is the biggest problem here.

the freeze on the initial query won't be fixed here, I have some idea to make it better, but I really don't want to add anything more to this patch, apart eventual fixes for eventual regressions, clearly.

(In reply to comment #24) > The to suggested test cases are totally fine with the try build. But searching > in History still freezes the whole UI as expected, which is the biggest problem > here. That is out of scope for this bug. Comment 0 and the summary are pretty clear that this bug is strictly about having the window open.

This passes all automated tests and is ideally ready for review. Now, after a brief talk with dietrich, even if we have a lot of queries tests, the patch is a bit scary due to its size. I'll try to figure out if I can apply a really simple (I hope) optimization to the search algorithm to get generally faster search queries (thus partially solving issue #2) allowing us to delay this patch to the next (minor?) release. I think this contains a lot of good fixes, but I absolutely understand the concern about size of the changes.

(In reply to comment #26) > (In reply to comment #24) > > The to suggested test cases are totally fine with the try build. But searching > > in History still freezes the whole UI as expected, which is the biggest problem > > here. > That is out of scope for this bug. Comment 0 and the summary are pretty clear > that this bug is strictly about having the window open. Ok, ok. Can you link me the related bug? (if exists)

I have attached a patch in Bug 365992 for the second issue (slow queries). With that patch applied this one loses some importance, but is still valuable to reduce the load. I hoped to be able to solve one of these 2 bugs with far less code changes, the patch in Bug 365992 is probably less scarying than this one, but still it's not my call at this point. Unless a driver tells me the opposite, I'm not going to spend any more time on this wanted perf gain till the next version.

Comment on attachment 507483 [details] [diff] [review] patch v1.1 This patch won't make FX4 due to the involved risk in the size of the changes, btw I want to split it to its own bug, since it solves only part of the issue, the most part is tracked by the meta bug on slow history searches. Bug 630225 has a not scary patch that will improve some most common cases, as a temporary solution.

(In reply to comment #31) > Comment on attachment 507483 [details] [diff] [review] > patch v1.1 > > This patch won't make FX4 due to the involved risk in the size of the changes, > btw I want to split it to its own bug, since it solves only part of the issue, > the most part is tracked by the meta bug on slow history searches. > Bug 630225 has a not scary patch that will improve some most common cases, as a > temporary solution. Not sure I understand the rationale. If it works and doesn't brake anything why not land it in minefield? I guess it could be backed out quickly if a serious issue pops up.

(In reply to comment #32) > Not sure I understand the rationale. If it works and doesn't brake anything why > not land it in minefield? I guess it could be backed out quickly if a serious > issue pops up. This late in the cycle also minimal changes can cause a week of delay in the release, thus we must pay more attention than usual.

Could you please tell me how will behave tomorrow (0305) nightly build regarding this issue? it will have some related fix.

Anyone have feedback here since comment #35? Sounds like some changes landed related to this.

Its been quite fine for quite while now. Didn't experience such things for several months when I forgot about the history window. One of Marco's bugs (forgot which) helped the issue a while back.

I think separate new bugs should be filed for specific issues, I didn't hear complains after the latest changes and this bug lacks new comments. Bug 365992 will stay open for discussion btw.