Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16064 )
Change subject: [util] add BlockingQueueTest.MultiThreadPerf test scenario ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/16064/1/src/kudu/util/blocking_queue-test.cc File src/kudu/util/blocking_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/16064/1/src/kudu/util/blocking_queue-test.cc@45 PS1, Line 45: DEFINE_uint32(num_blocking_writers, 3, : "number of threads calling BlockingQueue::BlockingPut()"); : DEFINE_uint32(num_non_blocking_writers, 2, : "number of threads calling BlockingQueue::Put()"); : DEFINE_uint32(num_blocking_readers, 5, : "number of threads calling BlockingQueue::BlockingGet()"); : DEFINE_uint32(runtime_sec, 5, "duration of the test (seconds)"); : DEFINE_uint32(queue_capacity, 64, "capacity of the queue (number of elements)"); Defining flags that will only be used in tests looks unnecessary. static constexpr/const variables in class will be better. http://gerrit.cloudera.org:8080/#/c/16064/1/src/kudu/util/blocking_queue-test.cc@463 PS1, Line 463: LOG(INFO) << Substitute( > nit: any reason to not log about the non-blocking Put throughput too? +1 http://gerrit.cloudera.org:8080/#/c/16064/1/src/kudu/util/blocking_queue-test.cc@468 PS1, Line 468: const size_t num_blocking_writers_; : const size_t num_non_blocking_writers_; : const size_t num_blocking_readers_; : const MonoDelta runtime_; : BlockingQueue<uint64_t> queue_; : vector<thread> threads_; : CountDownLatch barrier_; These variables can be private. Similarly except ctor and Run(), other functions can be private as well. -- To view, visit http://gerrit.cloudera.org:8080/16064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If52e4e15d3c72ad3479334f0f3fa8bb10b8c50c6 Gerrit-Change-Number: 16064 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 11 Jun 2020 17:18:10 +0000 Gerrit-HasComments: Yes
