From 10d03060acc6c1460e85004829d4fc51392ae961 Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 13:41:06 -0800 Subject: [PATCH 1/6] Add test case to return zero on a closed channel --- paddle/framework/channel_test.cc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index 444d68498c967..f38086b461093 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -60,6 +60,30 @@ TEST(Channel, SufficientBufferSizeDoesntBlock) { delete ch; } +TEST(Channel, SufficientBufferSizeDoesntBlock) { + const size_t buffer_size = 10; + auto ch = MakeChannel(buffer_size); + std::thread t([&]() { + // Try to write more than buffer size. + size_t out; + for (size_t i = 0; i < 12; ++i) { + ch->Receive(&out) + if (i < buffer_size) + EXPECT_EQ(out, i); // should block after 10 iterations + else + EXPECT_EQ(out, 0U); // after close Channel is called, expected value is 0 + } + }); + + for (size_t i = 0; i < buffer_size; ++i) { + EXPECT_EQ(ch->Send(&i), true); // sending should not block + } + + CloseChannel(ch); + t.join(); + delete ch; +} + TEST(Channel, ConcurrentSendNonConcurrentReceiveWithSufficientBufferSize) { const size_t buffer_size = 10; auto ch = MakeChannel(buffer_size); From 1891245027ea8bf677874469ebd2afe89aa298bc Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 14:26:02 -0800 Subject: [PATCH 2/6] Rename method --- paddle/framework/channel_test.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index f38086b461093..46cbbc7ddc1f9 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -60,7 +60,9 @@ TEST(Channel, SufficientBufferSizeDoesntBlock) { delete ch; } -TEST(Channel, SufficientBufferSizeDoesntBlock) { +// This test tests that CloseChannel returns a value of zero +// immediately to all receivers that are trying to receive from the channel. +TEST(Channel, ReceiverGetsZeroOnClosedChannel) { const size_t buffer_size = 10; auto ch = MakeChannel(buffer_size); std::thread t([&]() { From e611309c8a1a7f56b07cc42940097c07ccc21b99 Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 15:12:34 -0800 Subject: [PATCH 3/6] Fix test --- paddle/framework/channel_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index 46cbbc7ddc1f9..1b8b2e659fcad 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -69,7 +69,7 @@ TEST(Channel, ReceiverGetsZeroOnClosedChannel) { // Try to write more than buffer size. size_t out; for (size_t i = 0; i < 12; ++i) { - ch->Receive(&out) + ch->Receive(&out); if (i < buffer_size) EXPECT_EQ(out, i); // should block after 10 iterations else From 46acca0dc7b320d5c38c0b5bb32ec45c422590ea Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 16:18:34 -0800 Subject: [PATCH 4/6] Move receiving in same thread --- paddle/framework/channel_test.cc | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index 1b8b2e659fcad..39c79aeec260c 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -65,24 +65,24 @@ TEST(Channel, SufficientBufferSizeDoesntBlock) { TEST(Channel, ReceiverGetsZeroOnClosedChannel) { const size_t buffer_size = 10; auto ch = MakeChannel(buffer_size); - std::thread t([&]() { - // Try to write more than buffer size. - size_t out; - for (size_t i = 0; i < 12; ++i) { - ch->Receive(&out); - if (i < buffer_size) - EXPECT_EQ(out, i); // should block after 10 iterations - else - EXPECT_EQ(out, 0U); // after close Channel is called, expected value is 0 - } - }); - for (size_t i = 0; i < buffer_size; ++i) { + for (size_t i = 1; i <= buffer_size; ++i) { EXPECT_EQ(ch->Send(&i), true); // sending should not block } CloseChannel(ch); - t.join(); + + // Now try receiving for more number of times than buffer size + // after channel is closed + int out; + for (size_t i = 1; i <= 12; ++i) { + ch->Receive(&out); + if (i <= buffer_size) + EXPECT_EQ(out, i); // same value as was written by senders + else + EXPECT_EQ(out, 0U); // 0 after all elements are emptied from a closed channel + } + delete ch; } From 1861b36546d317018ab5862cdb24f02c6db41a07 Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 17:59:34 -0800 Subject: [PATCH 5/6] Fixed based on review comments --- paddle/framework/channel_test.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index 39c79aeec260c..1ffbc1c20402e 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -62,7 +62,7 @@ TEST(Channel, SufficientBufferSizeDoesntBlock) { // This test tests that CloseChannel returns a value of zero // immediately to all receivers that are trying to receive from the channel. -TEST(Channel, ReceiverGetsZeroOnClosedChannel) { +TEST(Channel, ReceiverGetsZeroOnClosedBufferedChannel) { const size_t buffer_size = 10; auto ch = MakeChannel(buffer_size); @@ -70,19 +70,24 @@ TEST(Channel, ReceiverGetsZeroOnClosedChannel) { EXPECT_EQ(ch->Send(&i), true); // sending should not block } + for (size_t i = 1; i <= 5; ++i) { + int out; + EXPECT_EQ(ch->Receive(&out), true); + EXPECT_EQ(out, i); + } CloseChannel(ch); // Now try receiving for more number of times than buffer size // after channel is closed - int out; - for (size_t i = 1; i <= 12; ++i) { + + for (size_t i = 6; i <= 12; ++i) { + int out; ch->Receive(&out); if (i <= buffer_size) EXPECT_EQ(out, i); // same value as was written by senders else EXPECT_EQ(out, 0U); // 0 after all elements are emptied from a closed channel } - delete ch; } @@ -145,6 +150,7 @@ TEST(Channel, BufferedChannelCloseUnblocksReceiversTest) { int data; // All reads should return false EXPECT_EQ(ch->Receive(&data), false); + EXPECT_EQ(data, 0); *p = true; }, &thread_ended[i]); @@ -240,6 +246,7 @@ TEST(Channel, UnbufferedChannelCloseUnblocksReceiversTest) { [&](bool *p) { int data; EXPECT_EQ(ch->Receive(&data), false); + EXPECT_EQ(data, 0); *p = true; }, &thread_ended[i]); From e06ee275db7681c49318626741df234bc7360f3b Mon Sep 17 00:00:00 2001 From: Kavya Srinet Date: Mon, 5 Feb 2018 18:10:57 -0800 Subject: [PATCH 6/6] review comments --- paddle/framework/channel_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paddle/framework/channel_test.cc b/paddle/framework/channel_test.cc index 1ffbc1c20402e..f415e0e264cdb 100644 --- a/paddle/framework/channel_test.cc +++ b/paddle/framework/channel_test.cc @@ -70,7 +70,7 @@ TEST(Channel, ReceiverGetsZeroOnClosedBufferedChannel) { EXPECT_EQ(ch->Send(&i), true); // sending should not block } - for (size_t i = 1; i <= 5; ++i) { + for (size_t i = 1; i < buffer_size/2; ++i) { int out; EXPECT_EQ(ch->Receive(&out), true); EXPECT_EQ(out, i); @@ -80,7 +80,7 @@ TEST(Channel, ReceiverGetsZeroOnClosedBufferedChannel) { // Now try receiving for more number of times than buffer size // after channel is closed - for (size_t i = 6; i <= 12; ++i) { + for (size_t i = buffer_size/2; i <= 12; ++i) { int out; ch->Receive(&out); if (i <= buffer_size)