Merge lp:~vanvugt/mir/override-ipc-threads into lp:mir

[Daniel van Vugt]

Allow the default thread pool size to be overridden. This is required
for the unity-system-compositor specific fix for LP: #1233001.

Also remove redundant option documentation which will soon get out of
sync and be wrong.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1096
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/53/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2063
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/53
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/53/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Another approach would be to change the ProgramOption class to be able to set the default value for each option independently of the get() method. u-s-c, or any other mir server, would then be able to set the default values to match its needs.

"Needs Discussion"

review: Needs Information

[Alan Griffiths (alan-griffiths)]

I don't think this is the right way to support the overriding of option defaults.

As discussed elsewhere (e.g. bug 1226227) the strong coupling we have between DefaultServerConfiguration and the creation of the options object is wrong.

Clients should be free to implement the Option interface and supply it to DefaultServerConfiguration. This would allow then to both supply options via another mechanism or use different default values (which is the purpose here).

This MP makes the existing issues worse, not better. I'd prefer an approach that splits out the initialization of options and allows for the more general case. (And if we must address the specific case, a "virtual int default_ipc_threads()" method is clearer.)

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Less coupling...
Now allow the default number of IPC threads to be overridden in derived
ServerConfiguration constructors by simply assigning:
   default_ipc_threads = N;

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1098
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/61/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2187
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2072
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/61
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/61/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Ugly, but doesn't make bad things worse.

review: Abstain

[Robert Carr (robertcarr)]

Ok. Would be nice if it were int const though.

review: Approve

[Robert Carr (robertcarr)]

This should be part of some ServerOptions interface which also encapsulates our current arguments/options and is passed to ServerConfiguration at construct time.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2013-09-24 11:43:27 +0000
+++ include/server/mir/default_server_configuration.h	2013-10-01 04:09:42 +0000
@@ -301,6 +301,8 @@
     CachedPtr<shell::MediatingDisplayChanger> mediating_display_changer;
     CachedPtr<shell::BroadcastingSessionEventSink> broadcasting_session_event_sink;
 
+    int default_ipc_threads = 10;
+
 private:
     int const argc;
     char const** const argv;

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-09-24 17:25:47 +0000
+++ src/server/default_server_configuration.cpp	2013-10-01 04:09:42 +0000
@@ -289,7 +289,7 @@
             "directory instead of the default logging directory."
             " [string:default=\"\"]")
         ("ipc-thread-pool", po::value<int>(),
-            "threads in frontend thread pool. [int:default=10]")
+            "threads in frontend thread pool.")
         ("vt", po::value<int>(),
             "VT to run on or 0 to use current. [int:default=0]");
 }

=== modified file 'src/server/frontend/default_configuration.cpp'
--- src/server/frontend/default_configuration.cpp	2013-09-25 14:18:18 +0000
+++ src/server/frontend/default_configuration.cpp	2013-10-01 04:09:42 +0000
@@ -51,7 +51,8 @@
     return connector(
         [&,this]() -> std::shared_ptr<mf::Connector>
         {
-            auto const threads = the_options()->get("ipc-thread-pool", 10);
+            auto const threads = the_options()->get("ipc-thread-pool",
+                                                    default_ipc_threads);
             auto shell_sessions = the_shell_session_container();
             auto const& force_requests_to_complete = [shell_sessions]
             {