Mir

Merge lp:~vanvugt/mir/fix-1373809 into lp:mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Kevin DuBois
Approved revision: 1938
Merged at revision: 1939
Proposed branch: lp:~vanvugt/mir/fix-1373809
Merge into: lp:mir
Diff against target: 41 lines (+13/-2)
2 files modified
src/common/input/android/android_input_receiver.cpp (+9/-1)
tests/unit-tests/client/input/test_android_input_receiver.cpp (+4/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1373809
Reviewer Review Type Date Requested Status
Kevin DuBois (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Review via email: mp+235917@code.launchpad.net

Commit message

Avoid polling in a loop with timeout zero most of the time. That
will only waste CPU. (LP: #1373809)

This also fixes test failure LP: #1373826.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks sensible.

Does it make sense to experiment with other timeout values, in order to find an even better trade-off between CPU consumption and event latency?

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

looks good to me

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/common/input/android/android_input_receiver.cpp'
2--- src/common/input/android/android_input_receiver.cpp 2014-09-25 07:57:28 +0000
3+++ src/common/input/android/android_input_receiver.cpp 2014-09-25 09:02:15 +0000
4@@ -142,11 +142,19 @@
5 }
6
7 auto reduced_timeout = timeout;
8- if (input_consumer->hasDeferredEvent() || input_consumer->hasPendingBatch())
9+ if (input_consumer->hasDeferredEvent())
10 {
11 // consume() didn't finish last time. Retry it immediately.
12 reduced_timeout = std::chrono::milliseconds::zero();
13 }
14+ else if (input_consumer->hasPendingBatch())
15+ {
16+ // When in constant motion we will usually "hasPendingBatch".
17+ // But the batch won't get flushed until the next frame interval,
18+ // so be sure to use a non-zero sleep time to avoid spinning the CPU
19+ // for the whole interval...
20+ reduced_timeout = std::chrono::milliseconds(1);
21+ }
22
23 auto result = looper->pollOnce(reduced_timeout.count());
24 if (result == ALOOPER_POLL_WAKE)
25
26=== modified file 'tests/unit-tests/client/input/test_android_input_receiver.cpp'
27--- tests/unit-tests/client/input/test_android_input_receiver.cpp 2014-09-25 07:57:28 +0000
28+++ tests/unit-tests/client/input/test_android_input_receiver.cpp 2014-09-25 09:02:15 +0000
29@@ -243,8 +243,11 @@
30 // Sadly using real time as droidinput::Looper doesn't use a mocked clock.
31 ASSERT_LT(duration_cast<nanoseconds>(duration).count(), one_frame);
32
33+ // Verify we don't use all the CPU by not sleeping (LP: #1373809)
34+ EXPECT_GT(duration_cast<nanoseconds>(duration).count(), one_millisecond);
35+
36 // But later in a frame or so, the motion will be reported:
37- t += one_frame;
38+ t += 2 * one_frame; // Account for the new slower 55Hz event rate
39 ASSERT_TRUE(receiver.next_event(max_timeout, ev));
40 ASSERT_EQ(mir_event_type_motion, ev.type);
41 }

Subscribers

People subscribed via source and target branches