Merge lp:~vanvugt/mir/fix-ClientLatency-of-5-alt into lp:mir

[Daniel van Vugt]

ClientLatency acceptance tests: Fix mismatching of stats that was causing
a spurious latency of 5 to be recorded for one of the buffer IDs when it's
really precisely 2.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3302
https://mir-jenkins.ubuntu.com/job/mir-ci/269/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build/57
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/63
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/59
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/59
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=amd64,compiler=gcc,platform=mesa,release=xenial/59
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=amd64,compiler=gcc,platform=mesa,release=xenial/59/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/59
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/59/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/59
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/59/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=mesa,release=xenial/59
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=mesa,release=xenial/59/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/269/rebuild

review: Approve (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

+ // The server is skipping a frame we gave it, which may or
17 + // may not be a bug. TODO: investigate.

Shouldn't we investigate first, instead of accommodating a potential bug in the test?

review: Needs Information

[Daniel van Vugt (vanvugt)]

There are two issues and neither is strictly a bug in the test really.

1. The server seems to drop our first frame (unless we sleep first). TODO: investigate. This may turn out to be a "feature" as we apparently intentionally drop frames on server restarts already.

2. When #1 happens the latency stats are wrong. That's all I'm fixing here.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3302
http://jenkins.qa.ubuntu.com/job/mir-ci/6252/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5855
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4762
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5811
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/414
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/576
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/576/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/576
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/576/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5808
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5808/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8211
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27434
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/410
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/410/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/263
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27437

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6252/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

+ // EXPECT_TRUE(i == submissions.begin());

Not useful

review: Approve

[Daniel van Vugt (vanvugt)]

That's arguably a useful future expectation.

If it turns out the server missing a frame is actually a bug, that's the kind of expectation we will need to introduce as a regression test. Although it may move.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2405/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5887
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4794
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5843/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/237
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/237/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/237
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/237/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5840
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5840/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8237/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27515

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3304
https://mir-jenkins.ubuntu.com/job/mir-ci/288/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build/76
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/82
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/78
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/78
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=amd64,compiler=gcc,platform=mesa,release=xenial/78
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=amd64,compiler=gcc,platform=mesa,release=xenial/78/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/78
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/78/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/78
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/78/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=mesa,release=xenial/78
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-advanced/arch=i386,compiler=gcc,platform=mesa,release=xenial/78/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/288/rebuild

review: Approve (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Ok

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3304
http://jenkins.qa.ubuntu.com/job/mir-ci/6275/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5890
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4797
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5846
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/599
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/599/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/599
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/599/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5843
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5843/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8240
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27528
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/422
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/422/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/275
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27529

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6275/rebuild

review: Approve (continuous-integration)

=== modified file 'tests/acceptance-tests/test_latency.cpp'
--- tests/acceptance-tests/test_latency.cpp	2016-01-29 08:18:22 +0000
+++ tests/acceptance-tests/test_latency.cpp	2016-02-12 04:13:44 +0000
@@ -40,6 +40,7 @@
 namespace
 {
 
+unsigned int const expected_client_buffers = 3;
 int const refresh_rate = 60;
 std::chrono::microseconds const vblank_interval(1000000/refresh_rate);
 
@@ -69,6 +70,13 @@
         {
             if (i->buffer_id == submission_id)
             {
+                // The server is skipping a frame we gave it, which may or
+                // may not be a bug. TODO: investigate.
+                // EXPECT_TRUE(i == submissions.begin());
+                // Fix it up so our stats aren't skewed by the miss:
+                if (i != submissions.begin())
+                    i = submissions.erase(submissions.begin(), i);
+
                 latency = post_count - i->time;
                 submissions.erase(i);
                 break;
@@ -151,6 +159,8 @@
         {
             std::lock_guard<std::mutex> lock{mutex};
             latency_list.push_back(latency.value());
+            if (latency.value() > max)
+                max = latency.value();
         }
 
         stats.post();
@@ -175,9 +185,15 @@
         return static_cast<float>(sum) / latency_list.size();
     }
 
+    unsigned int max_latency() const
+    {
+        return max;
+    }
+
 private:
     std::mutex mutex;
-    std::vector<int> latency_list;
+    std::vector<unsigned int> latency_list;
+    unsigned int max = 0;
     Stats& stats;
     IdCollectingDB db;
 };
@@ -230,8 +246,6 @@
     ASSERT_TRUE(stats.wait_for_posts(test_submissions,
                                      std::chrono::seconds(60)));
 
-    unsigned int const expected_client_buffers = 3;
-
     // Note: Using the "early release" optimization without dynamic queue
     //       scaling enabled makes the expected latency possibly up to
     //       nbuffers instead of nbuffers-1. After dynamic queue scaling is
@@ -245,6 +259,24 @@
     EXPECT_THAT(observed_latency, Lt(expected_max_latency+error_margin));
 }
 
+TEST_F(ClientLatency, latency_is_limited_to_nbuffers)
+{
+    using namespace testing;
+
+    auto stream = mir_surface_get_buffer_stream(surface);
+    for(auto i = 0u; i < test_submissions; i++) {
+        auto submission_id = mir_debug_surface_current_buffer_id(surface);
+        stats.record_submission(submission_id);
+        mir_buffer_stream_swap_buffers_sync(stream);
+    }
+
+    ASSERT_TRUE(stats.wait_for_posts(test_submissions,
+                                     std::chrono::seconds(60)));
+
+    auto max_latency = display.group.max_latency();
+    EXPECT_THAT(max_latency, Le(expected_client_buffers));
+}
+
 TEST_F(ClientLatency, throttled_input_rate_yields_lower_latency)
 {
     using namespace testing;