Closed Bug 967799 Opened 6 years ago Closed 6 years ago

runreftestb2g.py uses 100% of the cpu during run

Categories

(Testing :: Reftest, defect)

x86
macOS
defect
Not set

Tracking

(firefox29 wontfix, firefox30 fixed, b2g18 fixed, b2g-v1.2 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 unaffected)

RESOLVED FIXED
mozilla30
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
b2g18 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- unaffected

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file, 2 obsolete files)

I'm not sure why yet. But this can slow down things quite a bit.
The time is spent in b2gautomation.py:waitForFinish
and it seems like waitForFinish is just busy waiting
May just want to put a time.sleep(0.1) into the waitForFinish to see if it helps.
This patch seems to fix the problem. I added a getStdout function that does the waiting. I left the stdout property because I wasn't sure if it's used elsewhere.

As an aside, I also found the fact that stdout was a property very confusing because reading from it has side-effects.
Attachment #8370357 - Flags: review?(ahalberstadt)
Blocks: 818968
This fixes an unhandled exception.
Attachment #8370357 - Attachment is obsolete: true
Attachment #8370357 - Flags: review?(ahalberstadt)
Attachment #8370979 - Flags: feedback?(ahalberstadt)
Comment on attachment 8370979 [details] [diff] [review]
wait on the queue instead of polling it v2

Review of attachment 8370979 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! This looks reasonable to me. I think reftests are the last harness using b2gautomation.py, so I don't think it'll have any affect anywhere else. Mozprocess should also handle waiting for output properly, so I don't think we'll run into this problem when we switch them over.
Attachment #8370979 - Flags: feedback?(ahalberstadt) → feedback+
Removes the stdout function and renames getStdout to getStdoutLines
Attachment #8370979 - Attachment is obsolete: true
Attachment #8373567 - Flags: review?(ahalberstadt)
Comment on attachment 8373567 [details] [diff] [review]
wait on the queue instead of polling it v3

Review of attachment 8373567 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this looks much better!
Attachment #8373567 - Flags: review?(ahalberstadt) → review+
https://hg.mozilla.org/mozilla-central/rev/34db3f21d953
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
My unscientific observation is that this is saving ~5 minutes per chunk :)
Comment on attachment 8373567 [details] [diff] [review]
wait on the queue instead of polling it v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): The AWS test machines are only running with one core. Without this change we spend a lot of our time in the test harness instead of just the emulator.
User impact if declined: The tests run slower and are more orange
Testing completed: has been on m-c for a month
Risk to taking this patch (and alternatives if risky): the test harness will be broken
Attachment #8373567 - Flags: approval-mozilla-b2g26?
Attachment #8373567 - Flags: approval-mozilla-b2g18?
Comment on attachment 8373567 [details] [diff] [review]
wait on the queue instead of polling it v3

Making an exception here to the same extent as commented in https://bugzilla.mozilla.org/show_bug.cgi?id=981856#c11
Attachment #8373567 - Flags: approval-mozilla-b2g26?
Attachment #8373567 - Flags: approval-mozilla-b2g26+
Attachment #8373567 - Flags: approval-mozilla-b2g18?
Attachment #8373567 - Flags: approval-mozilla-b2g18+
Thank you Bhavana!
You need to log in before you can comment on or make changes to this bug.