Merge lp:~josharenson/mir/install_glmark2 into lp:mir
- install_glmark2
- Merge into development-branch
Status: | Superseded |
---|---|
Proposed branch: | lp:~josharenson/mir/install_glmark2 |
Merge into: | lp:mir |
Diff against target: |
335 lines (+230/-4) 10 files modified
CMakeLists.txt (+1/-1) debian/control (+1/-0) debian/mir-test-tools.install (+1/-0) include/test/mir_test/popen.h (+59/-0) tests/CMakeLists.txt (+5/-0) tests/mir_test/CMakeLists.txt (+2/-1) tests/mir_test/popen.cpp (+65/-0) tests/mir_test_framework/in_process_server.cpp (+6/-2) tests/performance-tests/CMakeLists.txt (+28/-0) tests/performance-tests/test_glmark2-es2-mir.cpp (+62/-0) |
To merge this branch: | bzr merge lp:~josharenson/mir/install_glmark2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alan Griffiths | Needs Fixing | ||
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Robert Carr | Pending | ||
Alberto Aguirre | Pending | ||
Daniel van Vugt | Pending | ||
Alexandros Frantzis | Pending | ||
Review via email: mp+217849@code.launchpad.net |
This proposal supersedes a proposal from 2014-04-18.
This proposal has been superseded by a proposal from 2014-05-05.
Commit message
* Adds "performance tests" as a test category.
* Implements one test that runs glmark2, saving results to a local file.
Description of the change
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal | # |
Josh Arenson (josharenson) wrote : Posted in a previous version of this proposal | # |
> We don't need to link mir_performance
Absolutely.
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal | # |
Where should we source glmark2-es2-mir?
$ apt-cache search glmark2-es2-mir
$
Josh Arenson (josharenson) wrote : Posted in a previous version of this proposal | # |
> Where should we source glmark2-es2-mir?
>
> $ apt-cache search glmark2-es2-mir
> $
glmark2-es2-mir (the version that actually runs) is currently located in ppa:mir-
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal | # |
Why doesnt jenkins even say hello to this branch?
Josh Arenson (josharenson) wrote : Posted in a previous version of this proposal | # |
> Why doesnt jenkins even say hello to this branch?
According to fginther, there is an outage.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:1572
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
46 +find_program(
47 +
48 +if(NOT GLMARK2_
49 + message("warning: glmark2-es2-mir not found, performance tests _may_ fail.")
50 +endif()
Since we are not running the program automatically when building, I am not sure how useful it is to print this message at build time.
77 +#include <stdlib.h>
We also need <stdio.h> for popen, getline etc (we shouldn't depend on other headers bringing it in).
Some style issues (see http://
85 +class GLMark2Test : public ::testing::Test
86 +{
...
Mir uses 4 space indentations.
88 + enum ResultFileType {RAW, JSON};
Lowercase identifiers for enum values.
103 + while((read = getline(&line, &len, in)) != -1)
105 + if(file_type == RAW)
107 + else if(file_type == JSON)
109 + if(boost:
Spaces between keyword (if,while) and opening parenthesis.
105 + if(file_type == RAW)
106 + glmark2_output << line;
Braces around statement, since the 'else' clause also has braces.
94 + const char *cmd
93 + FILE *in;
...
'*' goes with type in our C++ code and const goes after the type. So: "FILE* in" and "char const* cmd"
98 + std::ofstream glmark2_output;
No need to define all variables in the beginning of the code block if it is more convenient (or necessary for RAII purposes) not to do so. So, for example it is allowed to have "std::ofstream glmark2_
123 + glmark2_
No need to manually close the file, it's closed on std::ofstream destruction.
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal | # |
I have no opinion, but Jenkins won't ever land this till you add a commit message up the top.
Josh Arenson (josharenson) wrote : Posted in a previous version of this proposal | # |
> I have no opinion, but Jenkins won't ever land this till you add a commit
> message up the top.
Thanks for the heads up. I'm working on a refactor and will add the message when done.
Alberto Aguirre (albaguirre) wrote : Posted in a previous version of this proposal | # |
> > I have no opinion, but Jenkins won't ever land this till you add a commit
> > message up the top.
> Thanks for the heads up. I'm working on a refactor and will add the message
> when done.
You should mark this as "work in progress" so that others don't try to review code that is about to change.
Daniel van Vugt (vanvugt) : Posted in a previous version of this proposal | # |
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:1577
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
119 + /*HACK to wait for the server to start*/
You can override the mir::DefaultSer
/*HACK to stop the mir server*/
You can get the main loop object from mir::DefaultSer
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal | # |
> 119 + /*HACK to wait for the server to start*/
>
> You can override the
> mir::DefaultSer
> own implementation to wait for server start.
>
> /*HACK to stop the mir server*/
>
> You can get the main loop object from
> mir::DefaultSer
Even simpler: just use the pre-existing InProcessServer test fixture to start the server before entering the test body and close it afterwards.
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
> > 119 + /*HACK to wait for the server to start*/
> >
> > You can override the
> > mir::DefaultSer
> you
> > own implementation to wait for server start.
> >
> > /*HACK to stop the mir server*/
> >
> > You can get the main loop object from
> > mir::DefaultSer
>
> Even simpler: just use the pre-existing InProcessServer test fixture to start
> the server before entering the test body and close it afterwards.
Even better, +1
Alberto Aguirre (albaguirre) wrote : Posted in a previous version of this proposal | # |
~~~
111 + FILE* in;
112 + char* line = NULL;
113 + size_t len = 0;
114 + ssize_t read;
123 + ASSERT_TRUE((in = popen(cmd, "r")));
126 + while ((read = getline(&line, &len, in)) != -1)
151 + free(line);
152 + fclose(in);
~~~
I'd like to see that in a separate class and put under mir_test as it could be useful if we need to run other
benchmark programs OR use pstreams library which already does that for you.
~~~
virtual void RunGLMark2(char const* output_filename, ResultFileType file_type)
~~~
RunGLMark2 not in the style described at http://
~~~
106 + enum ResultFileType {raw, json};
~~~
Use enum class.
Alberto Aguirre (albaguirre) wrote : Posted in a previous version of this proposal | # |
Maybe this will be useful:
https:/
Popen p{"glmark2-es2-mir --fullscreen"};
std::string line;
while(p.
{
<do something with line>
}
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:1579
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1580
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
My earlier comment hasn't been addressed:
Even simpler: just use the pre-existing InProcessServer test fixture to start the server before entering the test body and close it afterwards.
Preview Diff
1 | === modified file 'CMakeLists.txt' |
2 | --- CMakeLists.txt 2014-05-01 08:44:30 +0000 |
3 | +++ CMakeLists.txt 2014-05-05 21:25:21 +0000 |
4 | @@ -76,7 +76,7 @@ |
5 | include_directories(include/shared) |
6 | |
7 | # Check for boost |
8 | -find_package(Boost 1.48.0 COMPONENTS chrono date_time filesystem system thread program_options regex REQUIRED) |
9 | +find_package(Boost 1.48.0 COMPONENTS chrono date_time filesystem system thread program_options regex iostreams REQUIRED) |
10 | include_directories ( |
11 | ${Boost_INCLUDE_DIRS} |
12 | ) |
13 | |
14 | === modified file 'debian/control' |
15 | --- debian/control 2014-05-01 08:49:17 +0000 |
16 | +++ debian/control 2014-05-05 21:25:21 +0000 |
17 | @@ -18,6 +18,7 @@ |
18 | libboost-thread-dev, |
19 | libboost-regex-dev, |
20 | libboost-filesystem-dev, |
21 | + libboost-iostreams-dev, |
22 | protobuf-compiler, |
23 | libdrm-dev, |
24 | libegl1-mesa-dev, |
25 | |
26 | === modified file 'debian/mir-test-tools.install' |
27 | --- debian/mir-test-tools.install 2014-03-06 06:05:17 +0000 |
28 | +++ debian/mir-test-tools.install 2014-05-05 21:25:21 +0000 |
29 | @@ -2,5 +2,6 @@ |
30 | usr/bin/mir_unit_tests |
31 | usr/bin/mir_acceptance_tests |
32 | usr/bin/mir_integration_tests |
33 | +usr/bin/mir_performance_tests |
34 | usr/lib/*/mir/tools/libmirclientlttng.so |
35 | usr/lib/*/mir/tools/libmirserverlttng.so |
36 | |
37 | === added file 'include/test/mir_test/popen.h' |
38 | --- include/test/mir_test/popen.h 1970-01-01 00:00:00 +0000 |
39 | +++ include/test/mir_test/popen.h 2014-05-05 21:25:21 +0000 |
40 | @@ -0,0 +1,59 @@ |
41 | +/* |
42 | + * Copyright © 2014 Canonical Ltd. |
43 | + * |
44 | + * This program is free software: you can redistribute it and/or modify it |
45 | + * under the terms of the GNU General Public License version 3, |
46 | + * as published by the Free Software Foundation. |
47 | + * |
48 | + * This program is distributed in the hope that it will be useful, |
49 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
50 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
51 | + * GNU General Public License for more details. |
52 | + * |
53 | + * You should have received a copy of the GNU General Public License |
54 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
55 | + * |
56 | + * Authored by: Alberto Aguirre <alberto.aguirre@canonical.com> |
57 | + */ |
58 | + |
59 | + |
60 | +#ifndef MIR_TEST_POPEN_H_ |
61 | +#define MIR_TEST_POPEN_H_ |
62 | + |
63 | +#include <cstdio> |
64 | +#include <memory> |
65 | +#include <string> |
66 | +#include <streambuf> |
67 | +#include <istream> |
68 | + |
69 | +namespace mir |
70 | +{ |
71 | +namespace test |
72 | +{ |
73 | +/** |
74 | + * Popen - A popen c++ wrapper |
75 | + */ |
76 | +class Popen |
77 | +{ |
78 | +public: |
79 | + Popen(std::string const& cmd); |
80 | + |
81 | + /** |
82 | + * Read a line from the output of the executed command |
83 | + * returns false if there is nothing more to read |
84 | + */ |
85 | + bool get_line(std::string& line); |
86 | + |
87 | +private: |
88 | + Popen() = delete; |
89 | + Popen(Popen const&) = delete; |
90 | + Popen& operator=(Popen const&) = delete; |
91 | + std::unique_ptr<std::FILE, void(*)(FILE* f)> raw_stream; |
92 | + std::unique_ptr<std::streambuf> stream_buffer; |
93 | + std::istream stream; |
94 | +}; |
95 | + |
96 | +} |
97 | +} |
98 | + |
99 | +#endif |
100 | |
101 | === modified file 'tests/CMakeLists.txt' |
102 | --- tests/CMakeLists.txt 2014-03-26 05:48:59 +0000 |
103 | +++ tests/CMakeLists.txt 2014-05-05 21:25:21 +0000 |
104 | @@ -26,6 +26,7 @@ |
105 | |
106 | option(MIR_BUILD_ACCEPTANCE_TESTS "Build acceptance tests" ON) |
107 | option(MIR_BUILD_INTEGRATION_TESTS "Build integration tests" ON) |
108 | +option(MIR_BUILD_PERFORMANCE_TESTS "Build performance tests" ON) |
109 | option(MIR_BUILD_UNIT_TESTS "Build unit tests" ON) |
110 | |
111 | if (MIR_BUILD_ACCEPTANCE_TESTS) |
112 | @@ -36,6 +37,10 @@ |
113 | add_subdirectory(integration-tests/) |
114 | endif (MIR_BUILD_INTEGRATION_TESTS) |
115 | |
116 | +if (MIR_BUILD_PERFORMANCE_TESTS) |
117 | + add_subdirectory(performance-tests/) |
118 | +endif(MIR_BUILD_PERFORMANCE_TESTS) |
119 | + |
120 | if (MIR_BUILD_UNIT_TESTS) |
121 | add_subdirectory(unit-tests/) |
122 | endif (MIR_BUILD_UNIT_TESTS) |
123 | |
124 | === modified file 'tests/mir_test/CMakeLists.txt' |
125 | --- tests/mir_test/CMakeLists.txt 2014-04-15 05:31:19 +0000 |
126 | +++ tests/mir_test/CMakeLists.txt 2014-05-05 21:25:21 +0000 |
127 | @@ -5,4 +5,5 @@ |
128 | display_config_matchers.cpp |
129 | cross_process_action.cpp |
130 | spin_wait.cpp |
131 | -) |
132 | + popen.cpp |
133 | +) |
134 | \ No newline at end of file |
135 | |
136 | === added file 'tests/mir_test/popen.cpp' |
137 | --- tests/mir_test/popen.cpp 1970-01-01 00:00:00 +0000 |
138 | +++ tests/mir_test/popen.cpp 2014-05-05 21:25:21 +0000 |
139 | @@ -0,0 +1,65 @@ |
140 | +/* |
141 | + * Copyright © 2014 Canonical Ltd. |
142 | + * |
143 | + * This program is free software: you can redistribute it and/or modify it |
144 | + * under the terms of the GNU General Public License version 3, |
145 | + * as published by the Free Software Foundation. |
146 | + * |
147 | + * This program is distributed in the hope that it will be useful, |
148 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
149 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
150 | + * GNU General Public License for more details. |
151 | + * |
152 | + * You should have received a copy of the GNU General Public License |
153 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
154 | + * |
155 | + * Authored by: Alberto Aguirre <alberto.aguirre@canonical.com> |
156 | + */ |
157 | + |
158 | +#include <mir_test/popen.h> |
159 | + |
160 | +#include <boost/throw_exception.hpp> |
161 | +#include <boost/exception/errinfo_errno.hpp> |
162 | +#include <boost/iostreams/device/file_descriptor.hpp> |
163 | +#include <boost/iostreams/stream_buffer.hpp> |
164 | + |
165 | +#include <stdexcept> |
166 | + |
167 | +namespace mt = mir::test; |
168 | +namespace io = boost::iostreams; |
169 | + |
170 | +namespace |
171 | +{ |
172 | +std::unique_ptr<std::streambuf> create_stream_buffer(FILE* raw_stream) |
173 | +{ |
174 | + if (raw_stream == nullptr) |
175 | + { |
176 | + BOOST_THROW_EXCEPTION( |
177 | + boost::enable_error_info(std::runtime_error("popen failed")) |
178 | + << boost::errinfo_errno(errno)); |
179 | + } |
180 | + |
181 | + int fd = fileno(raw_stream); |
182 | + if (fd == -1) |
183 | + { |
184 | + BOOST_THROW_EXCEPTION( |
185 | + boost::enable_error_info(std::runtime_error("invalid file stream")) |
186 | + << boost::errinfo_errno(errno)); |
187 | + } |
188 | + auto raw = new io::stream_buffer<io::file_descriptor_source>{ |
189 | + fd, io::never_close_handle}; |
190 | + return std::unique_ptr<std::streambuf>(raw); |
191 | +} |
192 | +} |
193 | + |
194 | +mt::Popen::Popen(std::string const& cmd) |
195 | + : raw_stream{popen(cmd.c_str(), "r"), [](FILE* f){ pclose(f); }}, |
196 | + stream_buffer{create_stream_buffer(raw_stream.get())}, |
197 | + stream{stream_buffer.get()} |
198 | +{ |
199 | +} |
200 | + |
201 | +bool mt::Popen::get_line(std::string& line) |
202 | +{ |
203 | + return std::getline(stream, line); |
204 | +} |
205 | |
206 | === modified file 'tests/mir_test_framework/in_process_server.cpp' |
207 | --- tests/mir_test_framework/in_process_server.cpp 2014-03-06 06:05:17 +0000 |
208 | +++ tests/mir_test_framework/in_process_server.cpp 2014-05-05 21:25:21 +0000 |
209 | @@ -25,6 +25,7 @@ |
210 | |
211 | #include <gtest/gtest.h> |
212 | |
213 | +#include <boost/algorithm/string.hpp> |
214 | #include <condition_variable> |
215 | #include <mutex> |
216 | |
217 | @@ -37,12 +38,15 @@ |
218 | |
219 | mtf::InProcessServer::InProcessServer() : |
220 | old_env(getenv(env_no_file)) |
221 | -{ |
222 | - if (!old_env) setenv(env_no_file, "", true); |
223 | +{ |
224 | } |
225 | |
226 | void mtf::InProcessServer::SetUp() |
227 | { |
228 | + if (!old_env || boost::iequals(old_env, "false")) |
229 | + { |
230 | + unsetenv(env_no_file); |
231 | + } |
232 | display_server = start_mir_server(); |
233 | ASSERT_TRUE(display_server); |
234 | } |
235 | |
236 | === added directory 'tests/performance-tests' |
237 | === added file 'tests/performance-tests/CMakeLists.txt' |
238 | --- tests/performance-tests/CMakeLists.txt 1970-01-01 00:00:00 +0000 |
239 | +++ tests/performance-tests/CMakeLists.txt 2014-05-05 21:25:21 +0000 |
240 | @@ -0,0 +1,28 @@ |
241 | +set( |
242 | + PERFORMANCE_TESTS_SOURCES |
243 | + |
244 | + test_glmark2-es2-mir.cpp |
245 | +) |
246 | + |
247 | +add_executable( |
248 | + mir_performance_tests |
249 | + |
250 | + ${PERFORMANCE_TESTS_SOURCES} |
251 | +) |
252 | + |
253 | +target_link_libraries( |
254 | + mir_performance_tests |
255 | + |
256 | + mir-test |
257 | + mir-test-framework |
258 | + |
259 | + ${Boost_LIBRARIES} |
260 | + ${GTEST_BOTH_LIBRARIES} |
261 | + ${GMOCK_LIBRARY} |
262 | + ${GMOCK_MAIN_LIBRARY} |
263 | +) |
264 | + |
265 | +install( |
266 | + TARGETS mir_performance_tests |
267 | + RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} |
268 | +) |
269 | |
270 | === added file 'tests/performance-tests/test_glmark2-es2-mir.cpp' |
271 | --- tests/performance-tests/test_glmark2-es2-mir.cpp 1970-01-01 00:00:00 +0000 |
272 | +++ tests/performance-tests/test_glmark2-es2-mir.cpp 2014-05-05 21:25:21 +0000 |
273 | @@ -0,0 +1,62 @@ |
274 | +#include "mir/default_server_configuration.h" |
275 | +#include "mir_test/popen.h" |
276 | +#include "mir_test_framework/in_process_server.h" |
277 | + |
278 | +#include <gtest/gtest.h> |
279 | + |
280 | +#include <stdlib.h> |
281 | +#include <stdio.h> |
282 | + |
283 | +#include <boost/regex.hpp> |
284 | +#include <iostream> |
285 | +#include <fstream> |
286 | +#include <string> |
287 | + |
288 | +class GLMark2Test : public mir_test_framework::InProcessServer |
289 | +{ |
290 | +public: |
291 | + char const* argv[1] = {""}; |
292 | + int argc = sizeof(argv) / sizeof(argv[0]); |
293 | + mir::DefaultServerConfiguration config{argc, argv}; |
294 | + |
295 | + mir::DefaultServerConfiguration& server_config() override {return config;}; |
296 | +protected: |
297 | + enum ResultFileType {raw, json}; |
298 | + virtual void RunGLMark2(char const* output_filename, ResultFileType file_type) |
299 | + { |
300 | + std::string const cmd = "glmark2-es2-mir --fullscreen"; |
301 | + mir::test::Popen p(cmd); |
302 | + |
303 | + boost::cmatch matches; |
304 | + boost::regex re_glmark2_score(".*glmark2\\s+Score:\\s+(\\d+).*"); |
305 | + std::string line; |
306 | + std::ofstream glmark2_output; |
307 | + glmark2_output.open(output_filename); |
308 | + while (p.get_line(line)) |
309 | + { |
310 | + if (file_type == raw) |
311 | + { |
312 | + glmark2_output << line; |
313 | + } |
314 | + else if (file_type == json) |
315 | + { |
316 | + if (boost::regex_match(line.c_str(), matches, re_glmark2_score)) |
317 | + { |
318 | + std::string json = "{"; |
319 | + json += "'benchmark_name':'glmark2-es2-mir'"; |
320 | + json += ","; |
321 | + json += "'score':'" + matches[1] + "'"; |
322 | + json += "}"; |
323 | + glmark2_output << json; |
324 | + break; |
325 | + } |
326 | + } |
327 | + } |
328 | + } |
329 | +}; |
330 | + |
331 | +TEST_F(GLMark2Test, benchmark_fullscreen_default) |
332 | +{ |
333 | + unsetenv("MIR_SERVER_NO_FILE"); |
334 | + RunGLMark2("glmark2_fullscreen_default.json", json); |
335 | +} |
We don't need to link mir_performance _test against so much right?