Implemented new fence syncs which replaces the old sync points.

gpu/command_buffer/service/sync_point_manager_unittest.cc

Index: gpu/command_buffer/service/sync_point_manager_unittest.cc
diff --git a/gpu/command_buffer/service/sync_point_manager_unittest.cc b/gpu/command_buffer/service/sync_point_manager_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..43212b07a58449294a58e5eebdd3187153c01f14
--- /dev/null
+++ b/gpu/command_buffer/service/sync_point_manager_unittest.cc
@@ -0,0 +1,384 @@
+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <queue>
+
+#include "base/bind.h"
+#include "gpu/command_buffer/service/sync_point_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace gpu {
+
+class SyncPointManagerTest : public testing::Test {
+ public:
+  SyncPointManagerTest() {
+  }
+
+  ~SyncPointManagerTest() override {}
+
+ protected:
+  void SetUp() override {
+    sync_point_manager_.reset(new SyncPointManager(false));
+  }
+
+  void TearDown() override {
+    sync_point_manager_.reset();
+  }
+
+  // Simple static function which can be used to test callbacks.
+  static void SetIntegerFunction(int* test, int value) {
+    *test = value;
+  }
+
+  scoped_ptr<SyncPointManager> sync_point_manager_;
+};
+
+struct SyncPointStream {
+  scoped_refptr<SyncPointOrderData> order_data;
+  scoped_ptr<SyncPointClient> client;
+  std::queue<uint32_t> order_numbers;
+
+  SyncPointStream(SyncPointManager* sync_point_manager,
+                  CommandBufferNamespace namespace_id,
+                  uint64_t command_buffer_id)
+    : order_data(SyncPointOrderData::Create()),
+      client(sync_point_manager->CreateSyncPointClient(order_data,
+                                                       namespace_id,
+                                                       command_buffer_id)) {}
+
+  ~SyncPointStream() {
+    order_data->Destroy();
+    order_data = nullptr;
+  }
+
+  void AllocateOrderNum(SyncPointManager* sync_point_manager) {
+    order_numbers.push(
+        order_data->GenerateUnprocessedOrderNumber(sync_point_manager));
+  }
+
+  void BeginProcessing() {
+    ASSERT_FALSE(order_numbers.empty());
+    order_data->BeginProcessingOrderNumber(order_numbers.front());
+  }
+
+  void EndProcessing() {
+    ASSERT_FALSE(order_numbers.empty());
+    order_data->FinishProcessingOrderNumber(order_numbers.front());
+    order_numbers.pop();
+  }
+};
+
+TEST_F(SyncPointManagerTest, BasicSyncPointOrderDataTest) {
+  scoped_refptr<SyncPointOrderData> order_data = SyncPointOrderData::Create();
+
+  EXPECT_EQ(0u, order_data->current_order_num());
+  EXPECT_EQ(0u, order_data->processed_order_num());
+  EXPECT_EQ(0u, order_data->unprocessed_order_num());
+
+  const uint32_t order_num =
+      order_data->GenerateUnprocessedOrderNumber(sync_point_manager_.get());
+  EXPECT_EQ(1u, order_num);
+
+  EXPECT_EQ(0u, order_data->current_order_num());
+  EXPECT_EQ(0u, order_data->processed_order_num());
+  EXPECT_EQ(order_num, order_data->unprocessed_order_num());
+
+  order_data->BeginProcessingOrderNumber(order_num);
+  EXPECT_EQ(order_num, order_data->current_order_num());
+  EXPECT_EQ(0u, order_data->processed_order_num());
+  EXPECT_EQ(order_num, order_data->unprocessed_order_num());
+
+  order_data->FinishProcessingOrderNumber(order_num);
+  EXPECT_EQ(order_num, order_data->current_order_num());
+  EXPECT_EQ(order_num, order_data->processed_order_num());
+  EXPECT_EQ(order_num, order_data->unprocessed_order_num());
+}
+
+TEST_F(SyncPointManagerTest, SyncPointClientRegistration) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId = 0x123;
+
+  scoped_refptr<SyncPointClientState> empty_state =
+      sync_point_manager_->GetSyncPointClientState(kNamespaceId, kBufferId);
+  EXPECT_FALSE(empty_state);
+
+  scoped_refptr<SyncPointOrderData> order_data = SyncPointOrderData::Create();
+
+  scoped_ptr<SyncPointClient> client =
+      sync_point_manager_->CreateSyncPointClient(order_data,
+                                                 kNamespaceId,
+                                                 kBufferId);
+
+  EXPECT_EQ(order_data, client->client_state()->order_data());
+  EXPECT_EQ(client->client_state(),
+            sync_point_manager_->GetSyncPointClientState(kNamespaceId,
+                                                         kBufferId));
+}
+
+TEST_F(SyncPointManagerTest, BasicFenceSyncRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId = 0x123;
+
+  scoped_refptr<SyncPointOrderData> order_data = SyncPointOrderData::Create();
+  scoped_ptr<SyncPointClient> client =
+      sync_point_manager_->CreateSyncPointClient(order_data,
+                                                 kNamespaceId,
+                                                 kBufferId);
+  scoped_refptr<SyncPointClientState> client_state = client->client_state();
+
+  EXPECT_EQ(0u, client_state->fence_sync_release());
+  EXPECT_FALSE(client_state->IsFenceSyncReleased(1));
+
+  client->ReleaseFenceSync(1);
+
+  EXPECT_EQ(1u, client_state->fence_sync_release());
+  EXPECT_TRUE(client_state->IsFenceSyncReleased(1));
+}
+
+TEST_F(SyncPointManagerTest, MultipleClientsPerOrderData) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  scoped_refptr<SyncPointOrderData> order_data = SyncPointOrderData::Create();
+  scoped_ptr<SyncPointClient> client1 =
+      sync_point_manager_->CreateSyncPointClient(order_data,
+                                                 kNamespaceId,
+                                                 kBufferId1);
+  scoped_ptr<SyncPointClient> client2 =
+      sync_point_manager_->CreateSyncPointClient(order_data,
+                                                 kNamespaceId,
+                                                 kBufferId2);
+
+  scoped_refptr<SyncPointClientState> client_state1 = client1->client_state();
+  scoped_refptr<SyncPointClientState> client_state2 = client2->client_state();
+
+  client1->ReleaseFenceSync(1);
+
+  EXPECT_TRUE(client_state1->IsFenceSyncReleased(1));
+  EXPECT_FALSE(client_state2->IsFenceSyncReleased(1));
+}
+
+TEST_F(SyncPointManagerTest, BasicFenceSyncWaitRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  wait_stream.BeginProcessing();
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  ASSERT_TRUE(valid_wait);
+  EXPECT_EQ(10, test_num);
+
+  release_stream.BeginProcessing();
+  release_stream.client->ReleaseFenceSync(1);
+  EXPECT_EQ(123, test_num);
+}
+
+TEST_F(SyncPointManagerTest, OutOfOrderRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  // Generate wait order number first.
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  wait_stream.BeginProcessing();
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  EXPECT_FALSE(valid_wait);
+  EXPECT_EQ(10, test_num);
+}
+
+TEST_F(SyncPointManagerTest, HigherOrderNumberRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  // Generate wait order number first.
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  // Order number was higher but it was actually released.
+  release_stream.BeginProcessing();
+  release_stream.client->ReleaseFenceSync(1);
+  release_stream.EndProcessing();
+
+  wait_stream.BeginProcessing();
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  EXPECT_TRUE(valid_wait);
+  EXPECT_EQ(123, test_num);
+}
+
+TEST_F(SyncPointManagerTest, DestroyedClientRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  wait_stream.BeginProcessing();
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  EXPECT_TRUE(valid_wait);
+  EXPECT_EQ(10, test_num);
+
+  // Destroying the client should release the wait.
+  release_stream.client.reset();
+  EXPECT_EQ(123, test_num);
+}
+
+TEST_F(SyncPointManagerTest, NonExistentRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  wait_stream.BeginProcessing();
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  EXPECT_TRUE(valid_wait);
+  EXPECT_EQ(10, test_num);
+
+  // No release but finishing the order number should automatically release.
+  release_stream.BeginProcessing();
+  EXPECT_EQ(10, test_num);
+  release_stream.EndProcessing();
+  EXPECT_EQ(123, test_num);
+}
+
+TEST_F(SyncPointManagerTest, NonExistentOrderNumRelease) {
+  const CommandBufferNamespace kNamespaceId =
+      gpu::CommandBufferNamespace::GPU_IO;
+  const uint64_t kBufferId1 = 0x123;
+  const uint64_t kBufferId2 = 0x234;
+
+  SyncPointStream release_stream(sync_point_manager_.get(),
+                                 kNamespaceId,
+                                 kBufferId1);
+  SyncPointStream wait_stream(sync_point_manager_.get(),
+                              kNamespaceId,
+                              kBufferId2);
+
+  // Assign Release stream 1, 4 and assign Wait stream 2, 3.

[piman, 2015/10/01 17:58:16]

nit: maybe comment that order [3] waits on fence (1), that is supposed to be
released by [1] but doesn't?

[David Yen, 2015/10/01 18:12:00]

What this test is really testing is releases and order numbers that do not
exist, from the release stream's point of view all it knows is that the release
did not exist in 1 and it has an unprocessed order of 4. It *could* have an
order number of 2 which releases it, but doesn't know since the wait stream had
processed order number 2.

Maybe a comment that explains order [3] waits on fence (1) that is supposed to
be released by [2] but doesn't exist on that stream?

+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+  wait_stream.AllocateOrderNum(sync_point_manager_.get());
+  release_stream.AllocateOrderNum(sync_point_manager_.get());
+
+  // Have wait with order number 3 to wait on release.
+  wait_stream.BeginProcessing();
+  ASSERT_EQ(2u, wait_stream.order_data->current_order_num());
+  wait_stream.EndProcessing();
+  wait_stream.BeginProcessing();
+  ASSERT_EQ(3u, wait_stream.order_data->current_order_num());
+  int test_num = 10;
+  const bool valid_wait =
+      wait_stream.client->Wait(
+          release_stream.client->client_state(),
+          1,
+          base::Bind(&SyncPointManagerTest::SetIntegerFunction,
+                     &test_num,
+                     123));
+  EXPECT_TRUE(valid_wait);
+  EXPECT_EQ(10, test_num);
+
+  // Release stream should know it should release fence sync by order 3,
+  // so going through order number 1 should not release it yet.
+  release_stream.BeginProcessing();
+  ASSERT_EQ(1u, release_stream.order_data->current_order_num());
+  release_stream.EndProcessing();
+  EXPECT_FALSE(release_stream.client->client_state()->IsFenceSyncReleased(1));
+  EXPECT_EQ(10, test_num);
+
+  // Beginning order number 4 should immediately release 3.

[piman, 2015/10/01 17:58:16]

I think I'd like a similar test where there is no 4. I don't think it works
(there's nothing to finally cancel 1), but it needs to. Maybe when
processed_num_ becomes equal to unprocessed_num_, we need to release all
OrderFence?

[David Yen, 2015/10/01 18:12:00]

If there is no #4, then this test is the same as the previous case where the
release was expected to be in #1 but it didn't exist. This test is really
testing that the release should have been after 1 but before 3 (non existent
order number in the release stream).

Basically, the EndProcessing of #1 will release the fence. I can add a test for
this though, since it does seem a little bit different from the previous test
although internally it is handled the same.

[David Yen, 2015/10/01 18:25:43]

I've now added a new test "NonExistentRelease2" which is the same as this test
but removes the order number 4. I've also tried to add better descriptions of
how these tests work.

+  release_stream.BeginProcessing();
+  ASSERT_EQ(4u, release_stream.order_data->current_order_num());
+  EXPECT_TRUE(release_stream.client->client_state()->IsFenceSyncReleased(1));
+  EXPECT_EQ(123, test_num);
+}
+
+}  // namespace gpu
+