Merge lp:~josharenson/mir/install_glmark2 into lp:mir

[Josh Arenson]

Creates a performance test that runs glmak2 and saves the score to a json file.

[Robert Carr (robertcarr)]

We don't need to link mir_performance_test against so much right?

review: Needs Fixing

[Josh Arenson (josharenson)]

> We don't need to link mir_performance_test against so much right?
Absolutely.

[Alan Griffiths (alan-griffiths)]

Where should we source glmark2-es2-mir?

$ apt-cache search glmark2-es2-mir
$

review: Needs Information

[Josh Arenson (josharenson)]

> 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-team/staging. I have also successfully built it from source. Currently, the Jenkins job adds the ppa, and installs the package as part of the environment set up.

[Robert Carr (robertcarr)]

Why doesnt jenkins even say hello to this branch?

review: Needs Information

[Josh Arenson (josharenson)]

> Why doesnt jenkins even say hello to this branch?
According to fginther, there is an outage.

[PS Jenkins bot (ps-jenkins)]

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://code.launchpad.net/~josharenson/mir/install_glmark2/+merge/216504/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1388/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1689/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1687
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1262
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1120
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1120/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1125
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1125/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1265
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1265/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1165
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6335

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1388/rebuild

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

46 +find_program(GLMARK2_ES2_MIR_FOUND glmark2-es2-mir)
47 +
48 +if(NOT GLMARK2_ES2_MIR_FOUND)
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://unity.ubuntu.com/mir/cppguide/index.html):

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::regex_match(line, matches, re_glmark2_score))

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_output{output_filename};" after the ASSERT.

123 + glmark2_output.close();

No need to manually close the file, it's closed on std::ofstream destruction.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

I have no opinion, but Jenkins won't ever land this till you add a commit message up the top.

review: Needs Fixing

[Josh Arenson (josharenson)]

> 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)]

> > 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)]

review: Abstain

[PS Jenkins bot (ps-jenkins)]

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://code.launchpad.net/~josharenson/mir/install_glmark2/+merge/216504/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1439/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1758
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1756
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1328
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1171
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1171/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1176
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1176/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1331
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1331/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1225
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6538

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1439/rebuild

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

119 + /*HACK to wait for the server to start*/

You can override the mir::DefaultServerConfiguration::the_server_status_listener() and provide you own implementation to wait for server start.

/*HACK to stop the mir server*/

You can get the main loop object from mir::DefaultServerConfiguration::the_main_loop() and call the stop() method.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

> 119 + /*HACK to wait for the server to start*/
>
> You can override the
> mir::DefaultServerConfiguration::the_server_status_listener() and provide you
> own implementation to wait for server start.
>
> /*HACK to stop the mir server*/
>
> You can get the main loop object from
> mir::DefaultServerConfiguration::the_main_loop() and call the stop() method.

Even simpler: just use the pre-existing InProcessServer test fixture to start the server before entering the test body and close it afterwards.

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

> > 119 + /*HACK to wait for the server to start*/
> >
> > You can override the
> > mir::DefaultServerConfiguration::the_server_status_listener() and provide
> you
> > own implementation to wait for server start.
> >
> > /*HACK to stop the mir server*/
> >
> > You can get the main loop object from
> > mir::DefaultServerConfiguration::the_main_loop() and call the stop() method.
>
> 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)]

~~~
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://unity.ubuntu.com/mir/cppguide/index.html#Function_Names

~~~
106 + enum ResultFileType {raw, json};
~~~

Use enum class.

review: Needs Fixing

[Alberto Aguirre (albaguirre)]

Maybe this will be useful:
https://code.launchpad.net/~mir-team/mir/popen-cpp-wrapper/+merge/217831

Popen p{"glmark2-es2-mir --fullscreen"};
std::string line;
while(p.get_line(line))
{
  <do something with line>
}

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1579
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1457/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1783
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1781
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1350/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1189
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1189/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1194
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1194/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1353
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1353/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1246/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6602/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1457/rebuild

review: Needs Fixing (continuous-integration)

=== modified file 'debian/mir-test-tools.install'
--- debian/mir-test-tools.install	2014-03-06 06:05:17 +0000
+++ debian/mir-test-tools.install	2014-04-30 23:22:39 +0000
@@ -2,5 +2,6 @@
 usr/bin/mir_unit_tests
 usr/bin/mir_acceptance_tests
 usr/bin/mir_integration_tests
+usr/bin/mir_performance_tests
 usr/lib/*/mir/tools/libmirclientlttng.so
 usr/lib/*/mir/tools/libmirserverlttng.so

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-03-26 05:48:59 +0000
+++ tests/CMakeLists.txt	2014-04-30 23:22:39 +0000
@@ -26,6 +26,7 @@
 
 option(MIR_BUILD_ACCEPTANCE_TESTS "Build acceptance tests" ON)
 option(MIR_BUILD_INTEGRATION_TESTS "Build integration tests" ON)
+option(MIR_BUILD_PERFORMANCE_TESTS "Build performance tests" ON)
 option(MIR_BUILD_UNIT_TESTS "Build unit tests" ON)
 
 if (MIR_BUILD_ACCEPTANCE_TESTS)
@@ -36,6 +37,10 @@
   add_subdirectory(integration-tests/)
 endif (MIR_BUILD_INTEGRATION_TESTS)
 
+if (MIR_BUILD_PERFORMANCE_TESTS)
+    add_subdirectory(performance-tests/)
+endif(MIR_BUILD_PERFORMANCE_TESTS)
+
 if (MIR_BUILD_UNIT_TESTS)
   add_subdirectory(unit-tests/)
 endif (MIR_BUILD_UNIT_TESTS)

=== added directory 'tests/performance-tests'
=== added file 'tests/performance-tests/CMakeLists.txt'
--- tests/performance-tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/performance-tests/CMakeLists.txt	2014-04-30 23:22:39 +0000
@@ -0,0 +1,26 @@
+set(
+  PERFORMANCE_TESTS_SOURCES
+
+  test_glmark2-es2-mir.cpp
+)
+
+add_executable(
+  mir_performance_tests
+
+  ${PERFORMANCE_TESTS_SOURCES}
+)
+
+target_link_libraries(
+  mir_performance_tests
+  mirserver
+
+  ${Boost_LIBRARIES}
+  ${GTEST_BOTH_LIBRARIES}
+  ${GMOCK_LIBRARY}
+  ${GMOCK_MAIN_LIBRARY}
+)
+
+install(
+  TARGETS mir_performance_tests
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
+)

=== added file 'tests/performance-tests/test_glmark2-es2-mir.cpp'
--- tests/performance-tests/test_glmark2-es2-mir.cpp	1970-01-01 00:00:00 +0000
+++ tests/performance-tests/test_glmark2-es2-mir.cpp	2014-04-30 23:22:39 +0000
@@ -0,0 +1,111 @@
+#include "mir/default_server_configuration.h"
+#include "mir/display_server.h"
+#include "mir/run_mir.h"
+
+#include <gtest/gtest.h>
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include <boost/date_time/posix_time/posix_time.hpp>
+#include <boost/regex.hpp>
+#include <boost/thread/thread.hpp> 
+#include <condition_variable>
+#include <iostream>
+#include <mutex>
+#include <fstream>
+#include <string>
+#include <thread>
+
+class GLMark2Test : public ::testing::Test
+{
+protected:
+    enum ResultFileType {raw, json};
+    mir::DisplayServer* result{nullptr};
+    std::thread server_thread;
+
+    mir::DisplayServer* launch_mir_server()
+    {
+        char const* argv[] = {""};
+        int argc = sizeof(argv) / sizeof(argv[0]);
+        mir::DefaultServerConfiguration config(argc, argv);
+
+        std::condition_variable cv;
+        std::mutex mutex;
+        server_thread = std::thread([&]
+        {
+            try
+            {
+                run_mir(config, [&](mir::DisplayServer& ds) {
+                    std::unique_lock<std::mutex> lock(mutex);
+                    result = &ds;
+                    cv.notify_one();
+                });
+            }
+            catch(...)
+            {
+                mir::report_exception(std::cerr);
+            }
+        });
+       
+        using namespace std::chrono; 
+        auto const time_limit = system_clock::now() + seconds(2);
+        std::unique_lock<std::mutex> lock(mutex);
+        while(!result)
+        {
+            cv.wait_until(lock, time_limit);
+        }
+ 
+        return result;
+    }
+    
+    virtual void RunGLMark2(char const* output_filename, ResultFileType file_type)
+    {
+        mir::DisplayServer* ds = launch_mir_server();
+        ASSERT_TRUE(ds);
+
+        boost::cmatch matches;
+        boost::regex re_glmark2_score(".*glmark2\\s+Score:\\s+(\\d+).*");
+        FILE* in;
+        char* line = NULL;
+        size_t len = 0;
+        ssize_t read;
+        std::ofstream glmark2_output;
+        glmark2_output.open(output_filename);
+
+        char const* cmd = "glmark2-es2-mir -b texture --fullscreen";
+        ASSERT_TRUE((in = popen(cmd, "r")));
+        
+        while ((read = getline(&line, &len, in)) != -1)
+        {
+            if (file_type == raw)
+            {
+                glmark2_output << line;
+            }
+            else if (file_type == json)
+            {
+                std::cout << line;
+                if (boost::regex_match(line, matches, re_glmark2_score))
+                {
+                    std::string json =  "{";
+                        json += "'benchmark_name':'glmark2-es2-mir'";
+                        json += ",";
+                        json += "'score':'" + matches[1] + "'";
+                        json += "}";
+                    glmark2_output << json;
+                    break;
+                }
+            }
+        }
+        
+        ds->stop();
+        if (server_thread.joinable()) server_thread.join();
+        free(line);
+        fclose(in);
+    }
+};
+
+TEST_F(GLMark2Test, benchmark_fullscreen_default)
+{
+    RunGLMark2("glmark2_fullscreen_default.json", json);
+}