Closed
Bug 490035
Opened 16 years ago
Closed 14 years ago
"Script is busy" ("Unresponsive Script") warning from places flush script (nsPlacesDBFlush.js) on initial migration / import
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 4.0b9
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: jimm, Unassigned)
References
Details
(Keywords: helpwanted, Whiteboard: [TSnappiness][status is comment 21][see Bug 492410][fixed-in-places])
Attachments
(2 files, 1 obsolete file)
While testing bug 313879, I occasionally received this error (see screen shot) during initial migration.
This is also present in beta 4 as I just received it again while testing for bug 490009.
Reporter | ||
Updated•16 years ago
|
Flags: blocking-firefox3.5?
Reporter | ||
Comment 1•16 years ago
|
||
I should also add, clicking continue finishes up the import and everything continues on normally.
Updated•16 years ago
|
Whiteboard: [TSnappiness]
Comment 2•16 years ago
|
||
So, that's onItemChanged... Hrm - seems like stuff isn't in a batch update? Mak - any comments? I don't understand how _flushWithQueries could be taking much time at all though...
...that is, unless are not in a batch update.
Comment 3•16 years ago
|
||
looks crazy to me, have to look at the code. that point does not look like can cause a script busy, looks like we call a bunch of onItemChanged and that causes an high possibility the code ends up locking in that method.
Comment 4•16 years ago
|
||
Tentatively blocking, though this may be more due to a change in how we do runaway script detection.
Jim: can you run a comparison with Firefox 3.0.10 to see if we're much slower than we used to be on migration?
Flags: blocking-firefox3.5? → blocking-firefox3.5+
Reporter | ||
Comment 5•16 years ago
|
||
(In reply to comment #4)
> Tentatively blocking, though this may be more due to a change in how we do
> runaway script detection.
>
> Jim: can you run a comparison with Firefox 3.0.10 to see if we're much slower
> than we used to be on migration?
3.5 did the import in about half the time based on simple test using a sidebar stop watch.
I think one of the problems here is that the migration wizard doesn't have any user feedback while the import takes place. The windows message pump appears frozen - Vista reports the wizard as ("Not Responding)" in the titlebar after a few seconds. Maybe we need to add a progress to the import dialog and pump messages to it to free up events? That might give the script engine some time to breath as well.
Reporter | ||
Updated•16 years ago
|
Flags: in-litmus?
Comment 6•16 years ago
|
||
also bug 392193 could help here, there's a bitrotted patch there, we could try to strip part of it to speed up import, clearly not the interfaces changes.
Comment 7•16 years ago
|
||
Looks at these related bugs (dupes?):
Bug 489274
Bug 477070
Why does this one block but these older ones did not? FWIW, I've personally seen the unresponsive script when importing bookmarks on 3.0. Given a large enough import, hasn't this always happened?
Comment 8•16 years ago
|
||
this is happening more often in 3.5/3.6 due to changes in how the "slow script" warning works, that is probably the only difference with 3.0.x (so actually could probably happen with a not-so-large import file).
Comment 9•16 years ago
|
||
Looking at the code in nsIEPRofileMigrator, we use batch mode for history, but not for bookmarks.
Initially i was thinking to an import from html, but this is different, so nevermind comment 6.
We should at least import bookmarks in batch.
Comment 10•16 years ago
|
||
this is the first patch Dietrich posted in bug 392193 (Oct 2007!), that bug then changed to cover html import and the patch was still laying around. But the approach is still valid, i modified it to make it compile. He said it was crashing, but from the first test i did it didn't, so could be some crasher has gone.
Jim could you try with this patch and see if anything changes?
Reporter | ||
Comment 11•16 years ago
|
||
(In reply to comment #10)
> Created an attachment (id=376130) [details]
> unbitrot patch from bug 392193
>
> this is the first patch Dietrich posted in bug 392193 (Oct 2007!), that bug
> then changed to cover html import and the patch was still laying around. But
> the approach is still valid, i modified it to make it compile. He said it was
> crashing, but from the first test i did it didn't, so could be some crasher has
> gone.
> Jim could you try with this patch and see if anything changes?
Hmm, strange, this time it's in the event handler for the observer:
Script: chrome://browser/content/migration/migration.js:454
http://mxr.mozilla.org/mozilla-central/source/browser/components/migration/content/migration.js#430
Comment 12•16 years ago
|
||
we want that patch even if the slow script does not go away, and applied to all migrators. Maybe with an additional progressbar so we can get rid of that.
Probably there's always the same amount ot ticks before the warning fires, since we have removed the calls to dbflush using batch the warning has moved later in the code.
Does it take less time to complete the import?
Reporter | ||
Comment 13•16 years ago
|
||
(In reply to comment #12)
> Does it take less time to complete the import?
Yes, it was very quick!
Updated•16 years ago
|
Assignee: nobody → mak77
Comment 14•16 years ago
|
||
Jim, do you have an IE profile i can use to reproduce the hang?
Reporter | ||
Comment 15•16 years ago
|
||
Simplest way to reproduce it is to take a large folder of favorites with long titles (attached are some good msdn links) and duplicate that about 10 times in the favorites folder. I've used this to reproduce in an xp image.
Comment 19•16 years ago
|
||
this problem is quite common, we are hitting the disk with heavy load and the "Unresponsive script" dialog is a pain.
I've tried to enqueue import functions calls to the main thread to give some breath to the UI, potentially works between import steps, but as soon as we are importing a large history or large number of bookmarks, we are stuck inside a single import method, and i don't see clear ways to stop us doing work and dispatch to the main thread so that UI updates. unless we try to chunk the method so that it imports a maximum number of items per chunk.
This is a pain even when you remove a big number of bookmarks, i'd prefer that "Unresponsive script" warning being deactivable for code that is known taking time or for chrome code, or whatever (The dialog also says "a script ON THIS PAGE may be busy", but clearly it's not a script from the page we are on)
Updated•16 years ago
|
Attachment #376130 -
Attachment is obsolete: true
Comment 20•16 years ago
|
||
i will provide batch migration patch in bug 392193, that will make import faster.
On the other side even if import will be faster the "unresponsive script" warning will still show with a lot of bookmarks, so we need to find a way to fix that here. That could be hard if we are not going to allow idl changes for migration interface.
the low-bar workaround is to temporarly increase the timeout pref value.
I'm doing some experiment with events enqueueing on the main thread and a dedicated observer interface, but any suggestion is welcome.
Simply adding a progressbar won't help a lot since the ui message pump is completely frozen as Jim pointed out.
Depends on: 392193
Updated•16 years ago
|
Whiteboard: [TSnappiness] → [TSnappiness][status is comment 20]
Comment 21•16 years ago
|
||
With patch in bug 392193 importing bookmarks should take a few seconds on an optimized build (i tried with more than 10000 bookmarks), instead of a few minutes, so this could be no more an issue for a normal sized import (IIRC default chrome timeout is 20 seconds).
So i suggest taking that patch. Migration tests will require some more attention from QA (Migration components has sadly no automatic tests).
I tried other experiments to try breaking at better points the import without touching the interface, but still no luck so far, the maximum i could do is splitting between different import tasks (cookies, history, bookmarks, ...), but we will still lock during the execution of one of those.
I've no more ideas, so if anyone has any, feel free to steal the bug or suggest them.
Updated•16 years ago
|
Keywords: helpwanted
Updated•16 years ago
|
Whiteboard: [TSnappiness][status is comment 20] → [TSnappiness][status is comment 21]
Comment 22•16 years ago
|
||
jmathies, can you try your migration scenario again now that bug 392193 has landed?
Reporter | ||
Comment 23•16 years ago
|
||
(In reply to comment #22)
> jmathies, can you try your migration scenario again now that bug 392193 has
> landed?
Hmm, unfortunately I still receive the warning -
Script: chrome://browser/content/migration/migration.js:457
It always happens right before the import finishes up.
This is on a debug 191 build that was checked out about an hour ago.
Comment 24•16 years ago
|
||
So, debug builds are going to be a bit slower than opt builds, so that actually matters.
Comment 25•16 years ago
|
||
we can probably re-evaluate blocking status due to the fact:
- we are quite faster than 3.0 and now quite faster then before bug 392193
- bug 492410
and this should really be fixed for .next.
Whiteboard: [TSnappiness][status is comment 21] → [TSnappiness][status is comment 21][see Bug 492410]
Comment 26•16 years ago
|
||
(In reply to comment #25)
> we can probably re-evaluate blocking status due to the fact:
> - we are quite faster than 3.0 and now quite faster then before bug 392193
> - bug 492410
I agree - let's put this back in the nomination queue.
Flags: blocking-firefox3.5+ → blocking-firefox3.5?
Comment 27•16 years ago
|
||
Agreed; removed & renomed for 192
Flags: blocking-firefox3.6?
Flags: blocking-firefox3.5?
Flags: blocking-firefox3.5-
Comment 28•15 years ago
|
||
Is this still a problem?
Comment 29•15 years ago
|
||
potentially it is. I mean, we don't notify anymore for slow script, and migration is damn faster. in some case the warning can still appear because all migration work is done in a sequential way, all in the same method, all on the main thread.
Comment 30•15 years ago
|
||
(In reply to comment #29)
> I mean, we don't notify anymore for slow script, and
...
> in some case the warning can still appear because all
Which one is it? Pick a side, we're at war. :)
Comment 31•15 years ago
|
||
on really large migrations, for example heavy IE users having large profiles and tons of bookmarks.
Comment 32•15 years ago
|
||
Not blocking 3.6, though we'd take a fix. Is there a way to get this to work off the main thread such that it doesn't block? Or to at least show a progress indicator when it takes more than 2s?
blocking2.0: --- → ?
Flags: wanted-firefox3.6+
Flags: blocking-firefox3.6?
Flags: blocking-firefox3.6-
Comment 33•15 years ago
|
||
not actively working on this atm, we can mitigate the issue making Places more performant, still revising migration component to be more "modular" could help a lot.
Assignee: mak77 → nobody
Comment 34•15 years ago
|
||
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
Updated•15 years ago
|
Summary: "Script is busy" warning from places flush script on initial migration → "Script is busy" ("Unresponsive Script") warning from places flush script (nsPlacesDBFlush.js) on initial migration / import
Comment 35•14 years ago
|
||
This does not block Firefox 4.0, and will be fixed with bug 552023.
blocking2.0: ? → -
Depends on: 552023
Updated•14 years ago
|
Whiteboard: [TSnappiness][status is comment 21][see Bug 492410] → [TSnappiness][status is comment 21][see Bug 492410][fixed-in-places]
Comment 36•14 years ago
|
||
this specific bug cannot happen anymore since PlacesDBFlush was just removed
Status: NEW → RESOLVED
Closed: 14 years ago
OS: Windows Vista → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b9
Reporter | ||
Updated•12 years ago
|
Flags: in-litmus?
You need to log in
before you can comment on or make changes to this bug.
Description
•