Mir

Merge lp:~vanvugt/mir/compositor-test into lp:mir

Proposed by Daniel van Vugt
Status: Work in progress
Proposed branch: lp:~vanvugt/mir/compositor-test
Merge into: lp:mir
Prerequisite: lp:~vanvugt/mir/generalise-performance-tests
Diff against target: 166 lines (+131/-1)
4 files modified
debian/mir-test-tools.install (+1/-0)
tests/performance-tests/CMakeLists.txt (+7/-0)
tests/performance-tests/compositor_test.sh (+121/-0)
tests/performance-tests/performance_tests.sh (+2/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/compositor-test
Reviewer Review Type Date Requested Status
Alan Griffiths Abstain
Mir CI Bot continuous-integration Approve
Chris Halse Rogers Abstain
Kevin DuBois (community) Needs Fixing
Cemil Azizoglu (community) Needs Fixing
Review via email: mp+292608@code.launchpad.net

Commit message

Add a compositor performance test, which is actually a regression test
for LP: #1563287.

Description of the change

And now you can see the compositor test running in CI. Just not easily...
 1. Click on the job that has 'arch=cross-armhf'
 2. Click 'Console Output'
 3. Scroll to bottom and click 'device-runtests-mir ...'
 4. Click 'krillin'
 5. Click 'Console Output'
 6. Click 'Full Log'
 7. Search in page 'Mir Performance Tests'

In future it might be nice to have separate GL/bypass tests. But the problem with doing that right now is that we have different server parameters per platform, so it would be messy. We really need to re-unite our bypass/overlay terminology and options.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3484
https://mir-jenkins.ubuntu.com/job/mir-ci/890/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/922
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/961
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/952
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/952
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/932
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/932
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/932/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/932
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/932
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/932
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/932/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Should be made a bit more flexible by allowing these params passed via the command line.

89 + mir_demo_client_egltriangle
90 + mir_demo_client_egltriangle
91 + mir_demo_client_egltriangle
92 + mir_demo_client_egltriangle
93 + mir_demo_client_egltriangle
94 + mir_demo_client_flicker
95 + mir_demo_client_progressbar
96 + mir_demo_client_scroll
97 + mir_demo_client_flicker

141 +verify '$frame_rate \>= 55'
142 +verify '$render_time \<= 17'

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

+$bin_dir/mir_demo_server -f $mir_sock --window-manager=tiling --compositor-report=log > $server_log &

Wow! A use of "--window-manager=tiling". Maybe I shouldn't be proposing to remove it?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

What is the motivation for writing this in bash?

It looks like something easy to do using the google test framework we use widely.

Alternatively, there's the python based mir_perf_framework.

Do we really need a third approach?

review: Needs Information
Revision history for this message
Kevin DuBois (kdub) wrote :

> Should be made a bit more flexible by allowing these params passed via the
> command line.

> 141 +verify '$frame_rate \>= 55'
> 142 +verify '$render_time \<= 17'

+1, esp the parameters. Not too unreasonable that we might see an android device that's got a goofy vsync (like krillin)

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I have considered those things already, so should explain...

Bourne shell: is much more appropriate for simple command-line driven tests. And there will be more in future that are simple external commands, not C/C+ code. More like system tests.

gtest:
https://code.launchpad.net/~vanvugt/mir/generalise-performance-tests/+merge/292600/comments/750962

Parameters: that's actually not important and even not sensible. Because a regression test should be difficult to break. Making the threshold for your regression test a parameter makes it too easy to pass the test by accident.

krillin: Actually when bug 1563287 is present, krillin drops to 20Hz (and the test fails), so a threshold of 55 is fine.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

mir_performance_tests: Is the only performance test using real graphics we already have active in CI already. Admittedly it's only run on krillin (see bug 1566743). And looking forward it is grammatically most appropriate that the command 'mir_performance_tests' should run multiple performance tests.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

BTW -- I did start out trying to use GTest and threw it away after realizing all of the above.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

tiling: I would happily use any old Mir shell, but to ensure there's no occlusion logic active (which would skew performance results) would like to see a shell that cascades windows by default instead. So a little bit of each client must always be visible.

lp:~vanvugt/mir/compositor-test updated
3485. By Daniel van Vugt

Merge latest trunk

3486. By Daniel van Vugt

Merge latest parent branch

3487. By Daniel van Vugt

Merge latest trunk

3488. By Daniel van Vugt

Merge latest trunk

3489. By Daniel van Vugt

Merge latest trunk

3490. By Daniel van Vugt

Merge latest trunk

3491. By Daniel van Vugt

Merge latest trunk

3492. By Daniel van Vugt

Merge latest trunk

3493. By Daniel van Vugt

Merge latest trunk

3494. By Daniel van Vugt

Remove the dependency on tiling and redesign the test to stress Mir's
compositing much harder using fewer clients.

krillin gets 66 FPS with the fix for LP: #1563287, and 9 FPS without it.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3494
https://mir-jenkins.ubuntu.com/job/mir-ci/933/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/994/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1040
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1031
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1031
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1004
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1004/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1004
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1004/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1004
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1004/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1004
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1004/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1004/console

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

review: Needs Fixing (continuous-integration)
lp:~vanvugt/mir/compositor-test updated
3495. By Daniel van Vugt

Try harder, Jenkins.

Revision history for this message
Chris Halse Rogers (raof) wrote :

Looks like the sort of thing that would be better in a gtest executable; the processing is at the limit of what I'm comfortable bashing.

That would also let you ensure that clients are laid out as expected, replace the “sleep x” with events, and make it easy to add extra checks, like variance and such.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Incorrect...

A gtest executable would not only be more complex (I have tried) but is also impossible right now without forking off the server process in the same way a shell script does. See bug 1546912.

Also, the sleep should not be replaced because it's a real-time delay you need to put the clients visually out of phase.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3495
https://mir-jenkins.ubuntu.com/job/mir-ci/937/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/999
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1045
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1036
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1036
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1009
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1009/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1009
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1009/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1009
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1009/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1009
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1009/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1009
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1009/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

To which sleep do you refer? The one between sending SIGTERM to the server and sending SIGKILL? Or the one between forking the server and trying to see if the socket is up? Or the one between killing the clients and killing the server? :)

That crash should be fairly easy to work around by pre-forking the children, right?

Revision history for this message
Chris Halse Rogers (raof) wrote :

But, to reiterate, I'm not blocking on doing that.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I was referring to the sleep between clients starting. Although now I've changed the sizes of the triangles that one is not necessary. But it does help visually inform the observer what's going on though... The fact remains this new test takes less time than the existing performance test. I could shave some seconds off, but don't think it matters that much.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I am still not convinced that a gtest executable would be more complex (I haven't tried), nor that e.g. using popen() to detect progress would be less reliable than sleeps.

I am also concerned that we created the mir_perf_framework for performance testing, but appear determined not to use it.

But we do need to test this.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

My other reason for avoiding gtest (other than it taking more time, more code, and having to work around crashes on android) is that gtest doesn't like you forking. But I might try and prototype it today anyway...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Unmerged revisions

3495. By Daniel van Vugt

Try harder, Jenkins.

3494. By Daniel van Vugt

Remove the dependency on tiling and redesign the test to stress Mir's
compositing much harder using fewer clients.

krillin gets 66 FPS with the fix for LP: #1563287, and 9 FPS without it.

3493. By Daniel van Vugt

Merge latest trunk

3492. By Daniel van Vugt

Merge latest trunk

3491. By Daniel van Vugt

Merge latest trunk

3490. By Daniel van Vugt

Merge latest trunk

3489. By Daniel van Vugt

Merge latest trunk

3488. By Daniel van Vugt

Merge latest trunk

3487. By Daniel van Vugt

Merge latest trunk

3486. By Daniel van Vugt

Merge latest parent branch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/mir-test-tools.install'
2--- debian/mir-test-tools.install 2016-05-03 04:36:33 +0000
3+++ debian/mir-test-tools.install 2016-05-04 06:22:13 +0000
4@@ -6,6 +6,7 @@
5 usr/bin/mir_integration_tests*
6 usr/bin/mir_performance_tests
7 usr/bin/mir_glmark2_performance_test
8+usr/bin/mir_compositor_performance_test
9 usr/bin/mir_privileged_tests
10 usr/bin/mir_test_reload_protobuf
11 usr/bin/mir_test_client_*
12
13=== modified file 'tests/performance-tests/CMakeLists.txt'
14--- tests/performance-tests/CMakeLists.txt 2016-04-27 09:21:20 +0000
15+++ tests/performance-tests/CMakeLists.txt 2016-05-04 06:22:13 +0000
16@@ -16,3 +16,10 @@
17 mirserver
18 ${GTEST_BOTH_LIBRARIES}
19 )
20+
21+add_custom_target(mir_compositor_performance_test ALL
22+ cp ${CMAKE_CURRENT_SOURCE_DIR}/compositor_test.sh ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/mir_compositor_performance_test
23+)
24+install(PROGRAMS ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/mir_compositor_performance_test
25+ DESTINATION ${CMAKE_INSTALL_BINDIR}
26+)
27
28=== added file 'tests/performance-tests/compositor_test.sh'
29--- tests/performance-tests/compositor_test.sh 1970-01-01 00:00:00 +0000
30+++ tests/performance-tests/compositor_test.sh 2016-05-04 06:22:13 +0000
31@@ -0,0 +1,121 @@
32+#!/bin/sh
33+#
34+# Copyright © 2016 Canonical Ltd.
35+#
36+# This program is free software: you can redistribute it and/or modify
37+# it under the terms of the GNU General Public License version 3 as
38+# published by the Free Software Foundation.
39+#
40+# This program is distributed in the hope that it will be useful,
41+# but WITHOUT ANY WARRANTY; without even the implied warranty of
42+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
43+# GNU General Public License for more details.
44+#
45+# You should have received a copy of the GNU General Public License
46+# along with this program. If not, see <http://www.gnu.org/licenses/>.
47+#
48+# Author: Daniel van Vugt <daniel.van.vugt@canonical.com>
49+#
50+
51+bin_dir=`dirname $0`
52+
53+mir_sock="/tmp/mir_compositor_test_socket_$$"
54+server_log="/tmp/mir_compositor_test_log_$$"
55+
56+$bin_dir/mir_demo_server -f $mir_sock --compositor-report=log > $server_log &
57+server_pid=$!
58+echo "Started server PID $server_pid. Log file: $server_log"
59+
60+kill_server()
61+{
62+ kill $server_pid
63+ sleep 3
64+ kill -9 $server_pid 2>&-
65+}
66+
67+cancel()
68+{
69+ echo "Test cancelled."
70+ kill_server
71+ exit 99
72+}
73+
74+tries=0
75+while [ ! -e $mir_sock -a $tries -lt 3 ]; do
76+ sleep 1
77+ tries=$(($tries + 1))
78+done
79+if [ ! -e $mir_sock ]; then
80+ echo "Socket '$mir_sock' never created. Giving up."
81+ exit 88
82+fi
83+
84+trap cancel INT
85+trap cancel TERM
86+
87+export MIR_SOCKET=$mir_sock
88+
89+# This list of clients is carefully chosen to push the limits of Mir
90+# without known regressions, and still composite smoothly. Please don't
91+# change this list unless you're adding to it as the result of further
92+# performance improvements.
93+for client in mir_demo_client_flicker \
94+ "mir_demo_client_egltriangle -b0.5 -f" \
95+ mir_demo_client_progressbar \
96+ mir_demo_client_scroll \
97+ "mir_demo_client_egltriangle -b0.5" \
98+ mir_demo_client_multiwin
99+do
100+ $bin_dir/$client 2>&- 1>&- &
101+ client_pids="$client_pids $!"
102+ echo "Started client '$client' PID $!"
103+ sleep 1 # Make the triangles out of phase (and separately visible)
104+done
105+
106+echo "Test is running. Please wait..."
107+sleep 10
108+echo "Test is done. Collecting results..."
109+
110+kill $client_pids
111+sleep 2
112+kill_server
113+
114+final_report=`grep 'compositor: Display' $server_log | grep FPS | tail -1`
115+echo ""
116+echo "Final report:"
117+echo "$final_report"
118+echo ""
119+
120+# Note we can only extract and test integers...
121+frame_rate=`echo $final_report | sed 's/.* \([0-9][0-9]*\)\.[0-9][0-9]* FPS.*/\1/'`
122+render_time=`echo $final_report | sed 's/.* \([0-9][0-9]*\\).[0-9][0-9]* ms\/frame.*/\1/'`
123+
124+total_tests=0
125+passed_tests=0
126+
127+verify()
128+{
129+ cond=$1
130+ total_tests=$(($total_tests + 1))
131+ pretty=`echo "$1" | sed 's/[\\$]//g'`
132+ echo -n "TEST: $pretty "
133+ result=`eval expr $cond`
134+ if [ $result -ne 0 ]; then
135+ echo "PASS"
136+ passed_tests=$(($passed_tests + 1))
137+ else
138+ echo "FAIL"
139+ fi
140+}
141+
142+# Without the fix for LP: #1563287 we see 9 FPS on krillin here...
143+verify '$frame_rate \>= 58'
144+
145+# A render time threshold >= 16 is really redundant with the above test,
146+# but a much lower threshold won't work on the Android platform, because it
147+# rounds up render times to whole frame intervals...
148+verify '$render_time \<= 17'
149+
150+percent=$(($passed_tests * 100 / $total_tests))
151+echo "Passed $passed_tests out of $total_tests ($percent%)"
152+return $(($total_tests - $passed_tests))
153
154=== modified file 'tests/performance-tests/performance_tests.sh'
155--- tests/performance-tests/performance_tests.sh 2016-04-22 05:09:40 +0000
156+++ tests/performance-tests/performance_tests.sh 2016-05-04 06:22:13 +0000
157@@ -30,7 +30,8 @@
158
159 tests_run=0
160 failures=0
161-for perf_test in mir_glmark2_performance_test
162+for perf_test in mir_glmark2_performance_test \
163+ mir_compositor_performance_test
164 do
165 title_banner "Mir Performance Tests now running: $perf_test"
166 tests_run=$(($tests_run + 1))

Subscribers

People subscribed via source and target branches