Mir

Merge lp:~vanvugt/mir/workaround-1665802 into lp:mir

Proposed by Daniel van Vugt on 2017-03-10
Status: Work in progress
Proposed branch: lp:~vanvugt/mir/workaround-1665802
Merge into: lp:mir
Diff against target: 142 lines (+71/-6)
3 files modified
src/client/buffer_stream.cpp (+20/-5)
src/client/buffer_stream.h (+1/-0)
tests/unit-tests/client/test_client_buffer_stream.cpp (+50/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/workaround-1665802
Reviewer Review Type Date Requested Status
Mir development team 2017-03-10 Pending
Review via email: mp+319528@code.launchpad.net

Commit message

Provide an environment setting to work around LP: #1665802:

MIR_CLIENT_VSYNC_METHOD={client,server,both}

client = Reliably low latency, but suffers from the Freedreno driver bug.
server = High latency, but works around the bug.
both = Medium latency (varies randomly between client and server levels),
         and also works around the bug.

To post a comment you must log in.

Unmerged revisions

4078. By Daniel van Vugt on 2017-03-09

Better comments

4077. By Daniel van Vugt on 2017-03-09

Merge latest trunk

4076. By Daniel van Vugt on 2017-03-08

Merge latest parent branch

4075. By Daniel van Vugt on 2017-03-08

Cleaner

4074. By Daniel van Vugt on 2017-03-08

Merge branch 'test-client-BufferStream-vsync-switching' which includes
the latest trunk

4073. By Daniel van Vugt on 2017-03-07

Works now

4072. By Daniel van Vugt on 2017-03-07

Prototyped, not working

4071. By Daniel van Vugt on 2017-03-07

Add a (failing) regression test for the new code to come

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/client/buffer_stream.cpp'
2--- src/client/buffer_stream.cpp 2017-02-15 07:38:33 +0000
3+++ src/client/buffer_stream.cpp 2017-03-10 04:06:27 +0000
4@@ -290,6 +290,15 @@
5 */
6 current_swap_interval = interval_config.swap_interval();
7
8+ vsync_method = VSyncMethod::client;
9+ if (char const *env = getenv("MIR_CLIENT_VSYNC_METHOD"))
10+ {
11+ if (!strcmp(env, "server"))
12+ vsync_method = VSyncMethod::server;
13+ else if (!strcmp(env, "both"))
14+ vsync_method = VSyncMethod::both;
15+ }
16+
17 if (!protobuf_bs->has_id())
18 {
19 if (!protobuf_bs->has_error())
20@@ -476,9 +485,12 @@
21
22 void mcl::BufferStream::swap_buffers_sync()
23 {
24+ VSyncMethod method;
25+
26 {
27 std::lock_guard<decltype(mutex)> lock(mutex);
28- using_client_side_vsync = true;
29+ using_client_side_vsync = vsync_method != VSyncMethod::server;
30+ method = vsync_method; // An unlocked copy
31 }
32
33 /*
34@@ -492,7 +504,7 @@
35 * interval 0 when using client-side vsync. This guarantees that random
36 * scheduling imperfections won't create queuing lag.
37 */
38- if (interval_config.swap_interval() != 0)
39+ if (interval_config.swap_interval() != 0 && method == VSyncMethod::client)
40 set_server_swap_interval(0);
41
42 swap_buffers([](){})->wait_for_all();
43@@ -502,9 +514,12 @@
44 using_client_side_vsync = false;
45 }
46
47- int interval = swap_interval();
48- for (int i = 0; i < interval; ++i)
49- wait_for_vsync();
50+ if (method != VSyncMethod::server)
51+ {
52+ int const waits = swap_interval();
53+ for (int i = 0; i < waits; ++i)
54+ wait_for_vsync();
55+ }
56 }
57
58 void mcl::BufferStream::request_and_wait_for_configure(MirWindowAttrib attrib, int interval)
59
60=== modified file 'src/client/buffer_stream.h'
61--- src/client/buffer_stream.h 2017-02-15 07:38:33 +0000
62+++ src/client/buffer_stream.h 2017-03-10 04:06:27 +0000
63@@ -181,6 +181,7 @@
64 std::unordered_set<MirWindow*> users;
65 std::shared_ptr<FrameClock> frame_clock;
66 mir::time::PosixTimestamp last_vsync;
67+ enum class VSyncMethod {client, server, both} vsync_method;
68 };
69
70 }
71
72=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
73--- tests/unit-tests/client/test_client_buffer_stream.cpp 2017-03-08 09:49:38 +0000
74+++ tests/unit-tests/client/test_client_buffer_stream.cpp 2017-03-10 04:06:27 +0000
75@@ -32,6 +32,7 @@
76 #include "mir/test/doubles/mock_protobuf_server.h"
77 #include "mir/test/doubles/mock_client_buffer.h"
78 #include "mir/test/fake_shared.h"
79+#include "mir_test_framework/temporary_environment_value.h"
80
81 #include "mir_toolkit/mir_client_library.h"
82
83@@ -709,10 +710,58 @@
84 EXPECT_FALSE(conf.has_swapinterval());
85 bs.set_swap_interval(1);
86 bs.swap_buffers_sync();
87- EXPECT_TRUE(conf.has_swapinterval());
88+ ASSERT_TRUE(conf.has_swapinterval());
89 EXPECT_EQ(0, conf.swapinterval());
90 }
91
92+TEST_F(ClientBufferStream, can_force_vsync_method_both)
93+{
94+ mir_test_framework::TemporaryEnvironmentValue
95+ method("MIR_CLIENT_VSYNC_METHOD", "both");
96+ mcl::BufferStream bs{
97+ nullptr, nullptr, wait_handle, mock_protobuf_server,
98+ std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
99+ map, factory,
100+ response, perf_report, "", size, nbuffers};
101+ service_requests_for(mock_protobuf_server.alloc_count);
102+
103+ EXPECT_CALL(mock_protobuf_server, configure_buffer_stream(_,_,_))
104+ .Times(0);
105+ bs.set_swap_interval(1);
106+ bs.swap_buffers_sync();
107+ /*
108+ * TODO: Should also somehow test that the sleep happens here. Although
109+ * that's presently pointless because a non-adopted BufferStream
110+ * with no MirWindow won't have a frame clock and so won't sleep.
111+ * We'll need to either change adopted_by to take the frame clock
112+ * pointer (or some other interface) or mock MirWindow for it.
113+ */
114+}
115+
116+TEST_F(ClientBufferStream, can_force_vsync_method_server)
117+{
118+ mir_test_framework::TemporaryEnvironmentValue
119+ method("MIR_CLIENT_VSYNC_METHOD", "server");
120+ mcl::BufferStream bs{
121+ nullptr, nullptr, wait_handle, mock_protobuf_server,
122+ std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
123+ map, factory,
124+ response, perf_report, "", size, nbuffers};
125+ service_requests_for(mock_protobuf_server.alloc_count);
126+
127+ EXPECT_CALL(mock_protobuf_server, configure_buffer_stream(_,_,_))
128+ .Times(0);
129+ bs.set_swap_interval(1);
130+ bs.swap_buffers_sync();
131+ /*
132+ * TODO: Should also somehow test that NO sleep happens here. Although
133+ * that's presently pointless because a non-adopted BufferStream
134+ * with no MirWindow won't have a frame clock and so won't sleep.
135+ * We'll need to either change adopted_by to take the frame clock
136+ * pointer (or some other interface) or mock MirWindow for it.
137+ */
138+}
139+
140 TEST_F(ClientBufferStream, sets_swap_interval_requested)
141 {
142 mcl::BufferStream bs{

Subscribers

People subscribed via source and target branches