Closed
Bug 501485
Opened 15 years ago
Closed 13 years ago
investigate new NPAPI plugin unloading strategy
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla13
People
(Reporter: jaas, Assigned: jaas)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
Bug 500925 changed our default behavior to never unloading NPAPI plugins once they are loaded. We should investigate a new strategy for unloading plugins that does not regress the following scenario:
"1 window open, 1 tab: Open page X which has a single Flash instance. Go to page Y, which also has a single Flash instance."
In this scenario, if we unload plugins when their last instance is destroyed then we'll completely unload the Flash plugin when we unload page X and load the whole plugin again when we load page Y. We don't want to do that, we want to simply destroy the first instance and then load another instance, keeping the plugin alive the whole time.
Note that the bfcache does not keep plugin instances alive.
A potential solution is to see whether or not a plugin can be unloaded some number of minutes after its last instance is destroyed, killing that timer if another instance is started.
Comment 1•15 years ago
|
||
There's also a memory pressure notification that stuart added, which would be a good time to free any unused plugins.
Comment 2•15 years ago
|
||
Firefox 3.6 is never unloading the plugins anymore. While this is a very nice speech improvement for websites like Youtube and such, wouldn't people start reporting this as a memory leak (the behavior change is not publicly known yet)? Especially Flash can consume quite a lot of memory.
In the duplicate bug that I filed (see comment 3), I stated the following:
Results after watching a video:
Just opening youtube.com was around 6,000 K
Closing youtube.com without watching a video resulted in releasing 100 K
Watching the video memory went to 39,000 K
closing the youtube tab and memory went down to 13,000 K
This is going to appear as a memory leak to any end user. So users that complain of memory being held by plugins that might be used
again in same session should add 'plugins.unloadASAP'? I just foresee a lot of
complaints from users complaining about the memory being held.
I also didn't get an answer about this, "I couldn't find in the bug you linked to the actual time limit until the
process is killed. There was talk of 10 minutes but that doesn't seem like it
was the final answer." Bug 500925 is the mentioned bug
Comment 10•13 years ago
|
||
(In reply to comment #0)
> A potential solution is to see whether or not a plugin can be unloaded some
> number of minutes after its last instance is destroyed, killing that timer
> if another instance is started.
Here is a solution,
1.we differ the setting of each plug-in, and have a gerneral killing time keep on record the time since its last used, says the threshold 2mins.
2. when first time a plug-in meets the time, pop-up asking user "whether auto kill the instance when it's unused next time?" as we ask them whether keep a password.
So user can kill the less using plug-ins and keep the active ones (like Flash)
Updated•13 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 12•13 years ago
|
||
The processes after closed takes a long time to release memory.
Comment 13•13 years ago
|
||
Firefox should terminate the process when you disable the plugin in the AOM.
Comment 14•13 years ago
|
||
I mean to say that if I close any flash website, then the plugin-container still consumes memory and is not released immediately.
I don't want to disable the plugin.
Comment 15•13 years ago
|
||
I wasn't responding to you.
This bug has nothing to do with flash or flash plugin container memory usage.
Comment 16•13 years ago
|
||
Then why my bug Bug 673356 has been marked as a duplicate of this bug.
Comment 17•13 years ago
|
||
Comment 15 is just wrong, as far as I can tell. This bug is precisely about deciding when to kill plugin-container when there are no plugin instances around.
Note that "immediately" would actually suck a lot on page transitions, so is probably the wrong answer. Hence the "investigate" bit.
Comment 18•13 years ago
|
||
Its not about the container taking a long time to release memory, thats a flash bug itself.
Updated•13 years ago
|
Whiteboard: [MemShrink]
Updated•13 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 21•13 years ago
|
||
The workaround I am using is to close firefox when I see too many plugin-container.exe processes running, and open it again. This cannot be, naturally,
the final solution. The browser cannot start new processes and never terminate
them when they are no longer needed. So, if it is really time consuming to
start them when need occurs, let's keep them alive for some time when they are no longer needed (in case the user would open shortly another page requiring them).
Comment 25•13 years ago
|
||
I'm going to take this, but I'll want plugin-as-content to land first otherwise the merge will be difficult.
Assignee: nobody → benjamin
Comment 26•13 years ago
|
||
i can't seem to find a bug for that, is there one?
Comment 27•13 years ago
|
||
Comment 30•13 years ago
|
||
I have opened a ticket too but it was marked as a duplicate (I read the first post of this ticket before but it hasn't described the closing of plugin-container (was there already plugin-container at this date?)). I would like to have something like dom.ipc.plugins.idleTimeoutSecs which defines the time in seconds after plugin-container will be closed if no plugin used it.
Comment 31•13 years ago
|
||
the plugin container loads the plugin, not the other way around ;p
for npapi unloading to work, content and the container need to notify each other of whats going on is the gist i get from this, or else you could terminate things like buffered video's simply because theres no activity after the video buffers completely.
Comment 32•13 years ago
|
||
>the plugin container loads the plugin, not the other way around ;p
I know this but my post seems to look a little confusing about this :P
>for npapi unloading to work, content and the container need to notify each other of whats going on is the gist i get from this, or else you could terminate things like buffered video's simply because theres no activity after the video buffers completely.
Normally after a video is buffered it is playing and the flash plugin is processing it. Even if I pause a video there is a moderate cpu usage. If I kill the plugin-container the video says that the flash plugin has crashed.
Doesn't Firefox already know with this way that the flash plugin is already working? Otherwise how does Firefox know that the flash plugin has crashed? Just by watching if plugin-container is running?
Assignee | ||
Comment 33•13 years ago
|
||
Benjamin - do you mind if I take this bug or are you planning to work on it soon?
Assignee | ||
Comment 34•13 years ago
|
||
I wrote this pretty quickly but it seems to work alright. I'll need to think about this and do some more testing to be confident, but it'd be good to get review and feedback now.
Attachment #596059 -
Flags: review?(benjamin)
Assignee | ||
Comment 35•13 years ago
|
||
Fixes a minor bug that probably had no real consequences (incorrect return value from timer Notify callback).
Attachment #596059 -
Attachment is obsolete: true
Attachment #596140 -
Flags: review?(benjamin)
Attachment #596059 -
Flags: review?(benjamin)
Comment 36•13 years ago
|
||
Have you looked at the existing bugs related to unloading plugins? My main worry for this bug was that we'd cause those other bugs to appear by default. See bug 661260, especially comment 1, and also some of the commentary in bug 520639. I suspect that we should attempt to unload only OOPP plugins, and never in-process plugins.
Assignee | ||
Comment 37•13 years ago
|
||
I think we should entirely remove the ability to unload any plugin from a non-plugin process. I don't even want to offer that option with a pref any more. I'll do that in an updated patch, and the unloadASAP pref will only apply to OOPP.
Assignee | ||
Comment 38•13 years ago
|
||
Updated to never try to unload in-process plugin libraries. Only tested on Mac OS X so far.
Attachment #596140 -
Attachment is obsolete: true
Attachment #596181 -
Flags: review?(benjamin)
Attachment #596140 -
Flags: review?(benjamin)
Assignee | ||
Comment 39•13 years ago
|
||
Cancel the timer (if there is one) when a new instance is created.
Attachment #596181 -
Attachment is obsolete: true
Attachment #596181 -
Flags: review?(benjamin)
Assignee | ||
Comment 40•13 years ago
|
||
We should probably fix bug 661260 before making plugin unloading much more likely for the average user.
Depends on: 661260
Assignee | ||
Comment 41•13 years ago
|
||
try server run:
https://tbpl.mozilla.org/?tree=Try&rev=bfb36b156244
Attachment #596522 -
Flags: review?(benjamin)
Assignee | ||
Comment 42•13 years ago
|
||
Updated to current trunk.
Attachment #596522 -
Attachment is obsolete: true
Attachment #597036 -
Flags: review?(benjamin)
Attachment #596522 -
Flags: review?(benjamin)
Updated•13 years ago
|
Attachment #597036 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 43•13 years ago
|
||
pushed to mozilla-inbound
http://hg.mozilla.org/integration/mozilla-inbound/rev/27a6f4156f0a
Comment 44•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Comment 45•13 years ago
|
||
I don't see 'dom.ipc.plugins.unloadASAP' anywhere in about:config. Is it something that has to be added manually or did something get missed?
Comment 46•13 years ago
|
||
(In reply to IU from comment #45)
> I don't see 'dom.ipc.plugins.unloadASAP' anywhere in about:config. Is it
> something that has to be added manually or did something get missed?
I believe you have to create it by yourself, like old plugins.unloadASAP.
Firefox will unload plugin after it's unused 3 minute by default now.
Comment 47•13 years ago
|
||
Does this respond to memory pressure events (i.e. immediately unload on memory pressure)?
Comment 48•13 years ago
|
||
No.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•