Closed Bug 994560 Opened 10 years ago Closed 10 years ago

CPU Priority is computed according to previous priority value

Categories

(Core :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: kk1fff, Assigned: kk1fff)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Proposed patch (obsolete) — Splinter Review
CPU priority is computed before we set priority to a process[1], but it is computed according to current priority[2], instead of the new priority value that we are going to set.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/ipc/ProcessPriorityManager.cpp?from=processprioritymanager.cpp#985
[2] http://dxr.mozilla.org/mozilla-central/source/dom/ipc/ProcessPriorityManager.cpp?from=processprioritymanager.cpp#961
Attachment #8404523 - Flags: review?(gsvelto)
Comment on attachment 8404523 [details] [diff] [review]
Proposed patch

This looks good to me as it fixes some transactions we were doing incorrectly but before we land it we should really add some unit-tests to cover this particular issue. Bug 870181 had some tests which reside under dom/browser-element/mochitest/priority, see attachment 747243 [details] [diff] [review]. Those should be extended to cover this particular case.
Attachment #8404523 - Flags: review?(gsvelto) → feedback+
Assignee: nobody → pwang
Adding cpu priority check in test case of preallocated process can cover this case. I have run the test locally, although this test is not running on TBPL before bug 987164 has been fixed.
Attachment #8404523 - Attachment is obsolete: true
Attachment #8408044 - Flags: review?(gsvelto)
Comment on attachment 8408044 [details] [diff] [review]
Proposed Patch v2

Looks good to me; BTW I've tested a few different scenarios on my phone with an w/o your patch applied and this saves us quite a few transactions while also making certain high->low transactions faster which should improve responsiveness.
Attachment #8408044 - Flags: review?(gsvelto) → review+
Yeah, hope this can improve some in the transaction of turing preallocated to foreground as well.
https://hg.mozilla.org/mozilla-central/rev/24496a6b3fea
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.