Closed
Bug 879529
Opened 11 years ago
Closed 11 years ago
Non-main thread priorities in child processes are completely wrong
Categories
(Firefox OS Graveyard :: General, defect)
Firefox OS Graveyard
General
Tracking
(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)
People
(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)
References
Details
(Keywords: perf, Whiteboard: c= , [YouTubeCertBlocker+])
Attachments
(1 file)
|
1014 bytes,
patch
|
gsvelto
:
review+
|
Details | Diff | Splinter Review |
> + int newtaskpriority =
> + std::max(origtaskpriority + aNice - origProcPriority, origProcPriority);
The second argument of the max should be the new process priority, not the original process priority.
As it is, this prevents us from increasing the priority of threads. For example, if we're changing a process from bg to fg, the origProcPriority might be 18. This means that the new thread priority can't be greater than 18!
|
Assignee | |
Updated•11 years ago
|
blocking-b2g: --- → leo?
|
|
Assignee | |
Comment 1•11 years ago
|
||
See bug 874165 comment 27 for an example of this happening in the wild and having user-visible effects.
|
|
Assignee | |
Comment 3•11 years ago
|
||
We need to fix b2g-procrank so that it can show this data, so we're not totally in the dark anymore.
|
||
Updated•11 years ago
|
Whiteboard: [YouTubeCertBlocker] → [YouTubeCertBlocker+]
|
||
Comment 4•11 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #2) > Ouch. My fault, I was the reviewer, I should have spotted this :) > We need to fix b2g-procrank so that it can show this data, so we're not > totally in the dark anymore. Can you open a bug for this and CC me?
|
|
||
Updated•11 years ago
|
Attachment #758256 -
Flags: review?(gsvelto) → review+
|
|
Assignee | |
Comment 5•11 years ago
|
||
> Can you open a bug for this and CC me? Basically, it's bug 864597. > My fault, I was the reviewer, I should have spotted this :) As I recall, you wrote the original code, then I reviewed it, then you reviewed it, then I pushed it. So we both share some blame here. :)
|
|
Assignee | |
Comment 6•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/de1f999797ab
|
|
||
Comment 7•11 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #5) > Basically, it's bug 864597. Thanks, I've cc'd that bug. > As I recall, you wrote the original code, then I reviewed it, then you > reviewed it, then I pushed it. So we both share some blame here. :) Fair enough :) I haven't looked at your priority unit-tests but I will soon enough because it would be good to cover this stuff (and possibly things like bug 841563). BTW I had a chat with dhylands about I/O priorities which is something we might want to add to the ProcessPriorityManager (to prevent BG applications from freezing the phone with I/O and stuff). That might give me more chances to introduce obscure bugs ;)
|
|
||
Comment 8•11 years ago
|
||
(In reply to Gabriele Svelto [:gsvelto] from comment #7) > (and possibly things like bug 841563). Wrong bug, I meant bug 873284.
|
||
Comment 9•11 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #3) > We need to fix b2g-procrank so that it can show this data, so we're not > totally in the dark anymore. Hi Justin, Maybe you can try b2g-ps from gonk-misc "master" branch. I think "b2g-ps -t -P -p" may give you some of the data you want.
|
|
Assignee | |
Comment 10•11 years ago
|
||
>$ adb shell b2g-ps -t -P -p
>APPLICATION USER PID PPID VSIZE RSS PRIO NICE RTPRI SCHED PCY WCHAN PC NAME
>b2g root 144 112 175172 67316 20 0 0 0 fg ffffffff 40086430 S /system/b2g/b2g
>b2g root 249 144 175172 67316 20 0 0 0 fg c00f89ec 400857ec S b2g
>b2g root 258 144 175172 67316 20 0 0 0 fg c0176438 ffff0520 S b2g
>b2g root 274 144 175172 67316 20 0 0 0 fg c0104d84 40084678 S b2g
>b2g root 322 144 175172 67316 20 0 0 0 fg c03bdd38 400848b0 S b2g
>b2g root 492 144 175172 67316 20 0 0 0 fg c00f89ec 400857ec S b2g
>Usage app_376 376 144 65692 26820 38 18 0 0 fg ffffffff 400c7430 S /system/b2g/plugin-container
>Homescreen app_377 377 144 69380 27676 38 18 0 0 fg ffffffff 40045430 S /system/b2g/plugin-container
>Gallery app_406 406 144 74036 29144 21 1 0 0 fg ffffffff 400f5430 S /system/b2g/plugin-container
>E-Mail app_540 540 144 77668 27684 38 18 0 0 fg ffffffff 40018430 S /system/b2g/plugin-container
>(Preallocated a root 566 144 60020 20460 38 18 0 0 fg ffffffff 400c9430 S /system/b2g/plugin-container
This doesn't seem to list threads for anything other than the root b2g process.
This is probably because the script only shows entries which for
/system/b2g/b2g, or whose PPID is 144.
You're welcome to write a patch to fix this; doing so by only using the shell
commands available in the Android toolbox is too hard for me. :)|
|
||
Comment 11•11 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #10) > > This doesn't seem to list threads for anything other than the root b2g > process. > This is probably because the script only shows entries which for > /system/b2g/b2g, or whose PPID is 144. > > You're welcome to write a patch to fix this; doing so by only using the shell > commands available in the Android toolbox is too hard for me. :) Hmm, that's weird. I remember Dave fixed "-t" option in master branch (only) of gonk-misc. It should be below commit: https://github.com/mozilla-b2g/gonk-misc/commit/c94522947c4362fd3bb55123d0dd1f9887c26c2e I have no device in hand now, so I will check it again tomorrow.
|
|
Assignee | |
Comment 12•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/de1f999797ab
|
||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/de1f999797ab
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
|
||
Comment 14•11 years ago
|
||
This patch seems to provide a ~7% improvement on sunspider, and 3.5% on kraken test when ran in b2g browser: http://www.arewefastyet.com/#machine=14
|
|
||
Comment 15•11 years ago
|
||
(In reply to Alan Huang [:ahuang] from comment #11) > (In reply to Justin Lebar [:jlebar] from comment #10) > > > > This doesn't seem to list threads for anything other than the root b2g > > process. > > This is probably because the script only shows entries which for > > /system/b2g/b2g, or whose PPID is 144. > > > > You're welcome to write a patch to fix this; doing so by only using the shell > > commands available in the Android toolbox is too hard for me. :) I use master branch gonk-misc. I got below output: $ adb shell b2g-ps -t -P -p APPLICATION USER PID PPID VSIZE RSS PRIO NICE RTPRI SCHED PCY WCHAN PC NAME b2g root 110 1 185504 60600 20 0 0 0 fg ffffffff 400d5330 S /system/b2g/b2g Binder Thread # root 180 110 185504 60600 20 0 0 0 fg c03dcb4c 400d37b0 S Binder Thread # Binder Thread # root 182 110 185504 60600 20 0 0 0 fg c03dcb4c 400d37b0 S Binder Thread # b2g root 234 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S b2g Gecko_IOThread root 238 110 185504 60600 20 0 0 0 fg c0166e80 400d5330 S Gecko_IOThread Socket Thread root 243 110 185504 60600 20 0 0 0 fg c0142bc4 400d4558 S Socket Thread JS GC Helper root 244 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S JS GC Helper JS Sour~ Thread root 245 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S JS Sour~ Thread JS Watchdog root 246 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S JS Watchdog Hang Monitor root 247 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S Hang Monitor b2g root 248 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S b2g DOM Worker root 249 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S DOM Worker Timer root 250 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S Timer DOM Worker root 251 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S DOM Worker DOM Worker root 252 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S DOM Worker DOM Worker root 253 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S DOM Worker b2g root 254 110 185504 60600 20 0 0 0 fg c00e88c8 400d3578 S b2g InputReader root 255 110 185504 60600 12 -8 0 0 fg c0166e80 400d5330 S InputReader GonkSensors root 256 110 185504 60600 20 0 0 0 fg c0142bc4 400d4558 S GonkSensors Compositor root 257 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Compositor ImageBridgeChil root 259 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S ImageBridgeChil Storage I/O root 297 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Storage I/O HTML5 Parser root 317 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S HTML5 Parser Indexed~rans #1 root 318 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Indexed~rans #1 Indexed~rans #2 root 319 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Indexed~rans #2 ImageDecoder #1 root 320 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S ImageDecoder #1 b2g root 321 110 185504 60600 20 0 0 0 fg c036ed18 400d37b0 S b2g Cache I/O root 322 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Cache I/O mozStorage #1 root 331 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S mozStorage #1 Indexed~rans #3 root 332 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Indexed~rans #3 Indexed~rans #4 root 333 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Indexed~rans #4 Cert Verify root 352 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S Cert Verify localStorage DB root 374 110 185504 60600 21 1 0 0 fg c00ddcdc 400d46ec S localStorage DB GL updater root 386 110 185504 60600 10 -10 0 0 fg c00ddcdc 400d46ec S GL updater DeviceS~che I/O root 387 110 185504 60600 20 0 0 0 fg c00ddcdc 400d46ec S DeviceS~che I/O Communications app_336 336 110 107256 34304 21 1 0 0 fg ffffffff 4001d330 S /system/b2g/plugin-container Binder Thread # app_336 349 336 107256 34304 21 1 0 0 fg c03dcb4c 4001b7b0 S Binder Thread # Chrome_ChildThr app_336 350 336 107256 34304 21 1 0 0 fg c0166e80 4001d330 S Chrome_ChildThr Binder Thread # app_336 351 336 107256 34304 21 1 0 0 fg c03dcb4c 4001b7b0 S Binder Thread # JS GC Helper app_336 371 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S JS GC Helper JS Sour~ Thread app_336 372 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S JS Sour~ Thread JS Watchdog app_336 373 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S JS Watchdog Socket Thread app_336 375 336 107256 34304 21 1 0 0 fg c0142bc4 4001c558 S Socket Thread (Preallocated a app_336 388 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S (Preallocated a ImageBridgeChil app_336 389 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S ImageBridgeChil Timer app_336 392 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S Timer ImageDecoder #1 app_336 395 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S ImageDecoder #1 HTML5 Parser app_336 396 336 107256 34304 21 1 0 0 fg c00ddcdc 4001c6ec S HTML5 Parser Homescreen app_391 391 110 121024 34948 38 18 0 0 fg ffffffff 4005b330 S /system/b2g/plugin-container Binder Thread # app_391 399 391 121024 34948 38 18 0 0 fg c03dcb4c 400597b0 S Binder Thread # Binder Thread # app_391 400 391 121024 34948 38 18 0 0 fg c03dcb4c 400597b0 S Binder Thread # Chrome_ChildThr app_391 401 391 121024 34948 38 18 0 0 fg c0166e80 4005b330 S Chrome_ChildThr JS GC Helper app_391 414 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S JS GC Helper JS Sour~ Thread app_391 415 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S JS Sour~ Thread JS Watchdog app_391 416 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S JS Watchdog Socket Thread app_391 435 391 121024 34948 38 18 0 0 fg c0142bc4 4005a558 S Socket Thread (Preallocated a app_391 436 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S (Preallocated a ImageBridgeChil app_391 437 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S ImageBridgeChil Timer app_391 438 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S Timer ImageDecoder #1 app_391 441 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S ImageDecoder #1 HTML5 Parser app_391 442 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S HTML5 Parser StreamTrans #18 app_391 466 391 121024 34948 38 18 0 0 fg c00ddcdc 4005a6ec S StreamTrans #18 (Preallocated a root 450 110 68820 20604 38 18 0 0 fg ffffffff 400f9330 S /system/b2g/plugin-container Binder Thread # root 453 450 68820 20604 38 18 0 0 fg c03dcb4c 400f77b0 S Binder Thread # Chrome_ChildThr root 454 450 68820 20604 38 18 0 0 fg c0166e80 400f9330 S Chrome_ChildThr Binder Thread # root 455 450 68820 20604 38 18 0 0 fg c03dcb4c 400f77b0 S Binder Thread # JS GC Helper root 458 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S JS GC Helper JS Sour~ Thread root 459 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S JS Sour~ Thread JS Watchdog root 460 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S JS Watchdog Socket Thread root 462 450 68820 20604 38 18 0 0 fg c0142bc4 400f8558 S Socket Thread (Preallocated a root 467 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S (Preallocated a ImageBridgeChil root 468 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S ImageBridgeChil Timer root 474 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S Timer StreamTrans #1 root 475 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S StreamTrans #1 ImageDecoder #1 root 477 450 68820 20604 38 18 0 0 fg c00ddcdc 400f86ec S ImageDecoder #1 Maybe we could do more to improve the APPLICATION column.
|
||
Updated•11 years ago
|
blocking-b2g: leo? → leo+
Keywords: perf
Whiteboard: [YouTubeCertBlocker+] → c= , [YouTubeCertBlocker+]
|
|
Assignee | |
Comment 16•11 years ago
|
||
> I use master branch gonk-misc. I got below output: This may have been because I had busybox installed. Anyway, see bug 864597 for an alternative tool that I like a lot more.
|
|
||
Comment 17•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/331a13fa0bfd
Assignee: nobody → justin.lebar+bug
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-b2g-v1.1hd:
--- → affected
status-firefox22:
--- → wontfix
status-firefox23:
--- → wontfix
status-firefox24:
--- → fixed
Target Milestone: --- → 1.1 QE3
|
|
||
Updated•11 years ago
|
Flags: in-moztrap-
You need to log in
before you can comment on or make changes to this bug.
Description
•