Closed Bug 499852 Opened 10 years ago Closed 10 years ago
up network thread priority on Windows CE
To avoid network slowdowns, it helps to up the network thread priority on Windows CE; here's a simple patch that does this. The suggested 116 priority came from various contacts for CE6. (This is the right thread to set, right? We don't do IO on any other threads?)
10 years ago
This is the only thread on which we do scoket I/O, but we do some file I/O from a threadpool: http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsStreamTransportService.cpp#476
Comment on attachment 384541 [details] [diff] [review] set network thread priority I assume we can create a new bug/patch if we want to alter the priority of the File I/O thread, too. Vlad: does this fix bug 496849?
Comment on attachment 384541 [details] [diff] [review] set network thread priority could you diff with more context & the -p flag next time?
Attachment #384541 - Flags: superreview?(cbiesinger) → superreview+
10 years ago
Priority: -- → P2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Is there a downside to up'ing the priority? We have had some complaints and bugs for Fennec that networking activity was blocking the UI. See bug 497470 for example.
Hmm, given when this was pushed and when bug 497470 was filed, it probably wasn't the cause.
Vlad - I think you should be using CeSetThreadPriorityLevel here instead of SetThreadPriorityLevel. The MSDN docs say that SetThreadPriorityLevel is only good for setting levels 248 and higher: "Applications and device drivers should use the CeGetThreadPriority and CeSetThreadPriority functions, instead of the legacy functions GetThreadPriority and SetThreadPriority. The legacy functions are still available with the same interfaces but the functions have access only to the original 8 priority levels." http://msdn.microsoft.com/en-us/library/aa450596.aspx
This patch does the same thing but uses the CeSetThreadPriority function
Attachment #387000 - Flags: review?(vladimir)
10 years ago
Attachment #387000 - Flags: superreview+
Comment on attachment 387000 [details] [diff] [review] patch using CeSetThreadPriority Whoops -- I recreated this patch from memory and forgot that I had to use Ce*. Looks fine, but there's a lot of other cruft in this patch (whitespace changes). May want to land those separately to avoid obscuring the actual change in the patch (or just not include them at all, could be your editor stripping trailing whitespace automatically).
Attachment #387000 - Flags: review?(vladimir) → review+
FWIW, the 116 value seems to be the default priority of the NDIS driver/kernel thread -- so essentially we're saying to prioritize our processing roughly the same as getting the data in the first place.
removed the whitespace changes: http://hg.mozilla.org/mozilla-central/rev/0471fa063e43
You need to log in before you can comment on or make changes to this bug.