Created attachment 264666 [details] [diff] [review] Patch against 1.8 branch This is the result of the work from Intel to get better battery life on laptops under linux. Here is a copy of what the original patch taken from linuxpowertop.com says: Firefox had 2 condition variables that were waited on with a timeout of 250 miliseconds in a loop. It appears that the author of the code uses 250 milliseconds as "inifite", however, the PR_WaitCondVar() API has an actual value for inifite, PR_INTERVAL_NO_TIMEOUT. Use that instead. I'm attaching here a slightly modified patch and adding its original author to the Cc list (Edit: I would have, if Bugzilla would allow to Cc to adresses not associated to a bugzilla account). The difference with the original patch (http://www.linuxpowertop.org/patches/firefox-2.0.patch ) is that I removed the wait_time variables that are useless by the usage of PR_INTERVAL_NO_TIMEOUT.
Created attachment 264719 [details] [diff] [review] Patch against 1.8 branch Same patch, with correct paths.
Comment on attachment 264719 [details] [diff] [review] Patch against 1.8 branch I'm the original author of this code. Thanks for working on this and trying to optimize it. Regarding nsCertVerificationThread, I tend to agree this patch is safe. The condition variable gets set in both cases where we care: When exit is requested and when a new job gets pushed into the queue. Regarding nsSSLThread, life is more complicated. In addition to mExitRequested we have 2 additional exit conditions that we must watch out for. Regarding "pending work", we must ensure mCond will get signaled each time work is passed to us by assigning a socket to mBusySocket. It appears this is done already. And when "mSocketScheduledToBeDestroyed" gets set, the condition variable will currently not get signaled. Can you please fix that? Finally, we must land this patch on the "trunk" first, in the latest development version. This might be a risky change and it should bake there for a while before we dare to land this on 1.8 branch. It should get sufficient testing with all sorts of SSL access.
Created attachment 264740 [details] [diff] [review] New patch (Note the previous patch applied as-is perfectly fine on trunk, only with a hunk beeing offsetted by 62 lines) Here is a modified patch following what I understood from your hints. I only saw one place where mSocketScheduledToBeDestroyed should be notified. This diff has been done against the trunk with cvs diff -u8.
Comment on attachment 264740 [details] [diff] [review] New patch I think this should be sufficient. r=kengert for trunk Have you tested this patch yourself and tried to connect to some SSL sites?
not yet for the latest patch
I tested it on a couple of SSL sites. No problem encountered. Could this be landed ?
Patch checked in to trunk.
Do we have a bug open to track this for Branch?
(In reply to comment #8) > Do we have a bug open to track this for Branch? No separate bug needed. I requested approval for the patch for the stable branch. (Removing request to block 1.9, as 1.9 already has it)
Comment on attachment 264740 [details] [diff] [review] New patch approved for 22.214.171.124, a=dveditz for release-drivers
Fixed for 126.96.36.199 I'd like to encourage people on the cc list to test tomorrow's nightly 1.8 branch build and confirm that SSL is still working as expected, just to have this tested some more. Thanks.
With the last 3 releases of Firefox being Fire Drill releases was this actually fixed in 188.8.131.52 or has it been pushed back?
This was added as of Firefox 184.108.40.206 == 220.127.116.11
hey mike, cool tool! there is another interesting test case over in 407325 that might be interesting to figure out a solution to...
i've been following the case in bug 407325, which is still happening even though firefox is minimized, or a non ajax page is in the selected tab. the wakeups jump to over 150 with firefox being responsible for 1/3 of the wakeups on the whole system.